Settlement Locking 2nd Approach #1820
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HenriqueNogara
force-pushed
the
settlement-small-improvements
branch
from
302aabd to
6d1de4d
Compare
HenriqueNogara
changed the title
HenriqueNogara
force-pushed
the
locking-2nd-approach
branch
from
efa3519 to
021cb98
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new workflow for settlement locking and extends the settlement pallet with additional API endpoints, error variants, and runtime/storage configurations to support lock-based instruction processing. Key changes include:
- Adding new extrinsics and RPC methods (lock_instruction_weight, instruction_asset_count) along with corresponding runtime API definitions.
- Updating error variants and arithmetic operations (using saturating operations) to improve reliability.
- Modifying benchmarks and tests to cover the new settlement locking workflow and updating runtime constants (e.g. MaximumLockPeriod).
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rpc/src/settlement.rs | Adds new RPC methods and adjusts API calls for lock_instruction_weight and instruction_asset_count. |
| rpc/runtime-api/src/settlement.rs | Updates API signatures and documentation to expose new runtime methods. |
| primitives/src/settlement.rs | Adjusts derive macros and adds new settlement variants (LockedForExecution, SettleAfterLock). |
| pallets/sto/src/lib.rs | Removes unused WeightMeter parameter in asset transfers. |
| pallets/settlement/src/benchmarking.rs | Updates benchmarks to test the new locking flow and changes asset count ranges. |
| pallets/runtime/tests/* | Updates tests to reflect new error variants and locking behavior. |
| pallets/runtime/* | Adds MaximumLockPeriod constant and integrates new runtime APIs. |
| pallets/nft/src/lib.rs & pallets/asset/src/lib.rs | Replaces direct arithmetic with saturating methods and adds simplified transfer functions. |
Comments suppressed due to low confidence (1)
pallets/runtime/common/src/runtime.rs:1164
- [nitpick] The function name 'get_execute_instruction_info' implies that it returns execute instruction details, but it calls 'manual_execution_weight'. Consider ensuring that the naming of the API aligns with its functionality or updating it for clarity.
fn get_execute_instruction_info(instruction_id: InstructionId) -> Option<ExecuteInstructionInfo> { Settlement::manual_execution_weight(instruction_id) }
| @@ -468,8 +468,7 @@ benchmarks! { | |||
| }: _(alice.origin, InstructionId(1), receipt_details, portfolios) | |||
|
|
|||
| execute_manual_instruction { | |||
There was a problem hiding this comment.
[nitpick] The change to begin the test range at 0 (instead of 1) for fungible assets may be intentional to test edge-case scenarios. Consider adding a brief comment to clarify that this edge-case is deliberate.
Suggested change
| execute_manual_instruction { | |
| execute_manual_instruction { | |
| // Start the range at 0 to deliberately test edge-case scenarios for fungible assets. |
Copilot uses AI. Check for mistakes.
HenriqueNogara
changed the title
|
Merged into PR #1818 for easier review. |
Neopallium
pushed a commit
that referenced
this pull request
May 6, 2025
* Add lock instruction; Allow manual execution for locked instructions; Add simplified transfer * Add lock_instruction common benchmark * Improve benchmarks * Add unit tests * Add sample weights * Add rpc calls * Remove useless test * Fix benchmarks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
changelog
new features
SettleAfterLockinstructions;new external API
MaximumLockPeriodassociated type to the settlement pallet;LockedTimestamp;lock_instruction;lock_instruction_weight,instruction_asset_count;new events
InstructionLockedvariant to the settlement pallet;other
LockTimestampNotFound,ExceededMaximumLockingPeriod,FailedAssetTransferringConditions;