diff --git a/Cargo.lock b/Cargo.lock index 9f186480..a37eba52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7178,9 +7178,18 @@ dependencies = [ "frame-support", "frame-system", "log", + "pallet-assets", + "pallet-assets-holder", "pallet-balances 40.0.1", + "pallet-preimage", + "pallet-recovery", + "pallet-reversible-transfers", + "pallet-scheduler", "pallet-timestamp", + "pallet-utility", "parity-scale-codec", + "qp-high-security", + "qp-scheduler", "scale-info", "sp-arithmetic", "sp-core", @@ -7293,6 +7302,7 @@ dependencies = [ "pallet-timestamp", "pallet-utility", "parity-scale-codec", + "qp-high-security", "qp-scheduler", "scale-info", "sp-arithmetic", @@ -8884,6 +8894,14 @@ dependencies = [ "sp-runtime", ] +[[package]] +name = "qp-high-security" +version = "0.1.0" +dependencies = [ + "parity-scale-codec", + "scale-info", +] + [[package]] name = "qp-plonky2" version = "1.1.1" @@ -9194,6 +9212,7 @@ dependencies = [ "primitive-types 0.13.1", "qp-dilithium-crypto", "qp-header", + "qp-high-security", "qp-poseidon", "qp-scheduler", "scale-info", diff --git a/pallets/multisig/Cargo.toml b/pallets/multisig/Cargo.toml index 0d768485..0fd05da0 100644 --- a/pallets/multisig/Cargo.toml +++ b/pallets/multisig/Cargo.toml @@ -18,6 +18,9 @@ frame-support.workspace = true frame-system.workspace = true log.workspace = true pallet-balances.workspace = true +pallet-reversible-transfers = { path = "../reversible-transfers", default-features = false, optional = true } +qp-high-security = { path = "../../primitives/high-security", default-features = false } +qp-scheduler = { workspace = true, optional = true } scale-info = { features = ["derive"], workspace = true } sp-arithmetic.workspace = true sp-core.workspace = true @@ -26,10 +29,20 @@ sp-runtime.workspace = true [dev-dependencies] frame-support = { workspace = true, features = ["experimental"], default-features = true } +frame-system = { workspace = true, default-features = true } +pallet-assets = { workspace = true, default-features = true } +pallet-assets-holder = { workspace = true, default-features = true } pallet-balances = { workspace = true, features = ["std"] } +pallet-preimage = { workspace = true, default-features = true } +pallet-recovery = { workspace = true, default-features = true } +pallet-reversible-transfers = { path = "../reversible-transfers", default-features = true } +pallet-scheduler = { workspace = true, default-features = true } pallet-timestamp.workspace = true +pallet-utility = { workspace = true, default-features = true } +qp-scheduler = { workspace = true, default-features = true } sp-core.workspace = true sp-io.workspace = true +sp-runtime = { workspace = true, default-features = true } [features] default = ["std"] @@ -38,6 +51,10 @@ runtime-benchmarks = [ "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", "pallet-balances/runtime-benchmarks", + "pallet-reversible-transfers", + "pallet-reversible-transfers?/runtime-benchmarks", + "qp-high-security/runtime-benchmarks", + "qp-scheduler", "sp-runtime/runtime-benchmarks", ] std = [ @@ -47,7 +64,10 @@ std = [ "frame-system/std", "log/std", "pallet-balances/std", + "pallet-reversible-transfers?/std", "pallet-timestamp/std", + "qp-high-security/std", + "qp-scheduler?/std", "scale-info/std", "sp-arithmetic/std", "sp-core/std", diff --git a/pallets/multisig/README.md b/pallets/multisig/README.md index a7853087..6bbbc7cf 100644 --- a/pallets/multisig/README.md +++ b/pallets/multisig/README.md @@ -12,20 +12,24 @@ Basic workflow for using a multisig: ```rust // 1. Create a 2-of-3 multisig (Alice creates, Bob/Charlie/Dave are signers) -Multisig::create_multisig(Origin::signed(alice), vec![bob, charlie, dave], 2); -let multisig_addr = Multisig::derive_multisig_address(&[bob, charlie, dave], 0); +Multisig::create_multisig(Origin::signed(alice), vec![bob, charlie, dave], 2, 0); +// ^ threshold ^ nonce +let multisig_addr = Multisig::derive_multisig_address(&[bob, charlie, dave], 2, 0); +// ^ threshold ^ nonce // 2. Bob proposes a transaction let call = RuntimeCall::Balances(pallet_balances::Call::transfer { dest: eve, value: 100 }); Multisig::propose(Origin::signed(bob), multisig_addr, call.encode(), expiry_block); -// 3. Charlie approves - transaction executes automatically (2/2 threshold reached) +// 3. Charlie approves (2/2 threshold reached → proposal status becomes Approved) Multisig::approve(Origin::signed(charlie), multisig_addr, proposal_id); -// ✅ Transaction executed! No separate call needed. + +// 4. Any signer executes the approved proposal +Multisig::execute(Origin::signed(charlie), multisig_addr, proposal_id); +// ✅ Transaction executed! Proposal removed from storage, deposit returned to proposer. ``` -**Key Point:** Once the threshold is reached, the transaction is **automatically executed**. -There is no separate `execute()` call exposed to users. +**Key Point:** Approval and execution are **separate**. When the threshold is reached, the proposal status becomes `Approved`; any signer must then call `execute()` to dispatch the call. ## Core Functionality @@ -35,21 +39,25 @@ Creates a new multisig account with deterministic address generation. **Required Parameters:** - `signers: Vec` - List of authorized signers (REQUIRED, 1 to MaxSigners) - `threshold: u32` - Number of approvals needed (REQUIRED, 1 ≤ threshold ≤ signers.len()) +- `nonce: u64` - User-provided nonce for address uniqueness (REQUIRED) **Validation:** - No duplicate signers - Threshold must be > 0 - Threshold cannot exceed number of signers - Signers count must be ≤ MaxSigners +- Multisig address (derived from signers+threshold+nonce) must not already exist **Important:** Signers are automatically sorted before storing and address generation. Order doesn't matter: -- `[alice, bob, charlie]` → sorted to `[alice, bob, charlie]` → `address_1` -- `[charlie, bob, alice]` → sorted to `[alice, bob, charlie]` → `address_1` (same!) -- To create multiple multisigs with same signers, the nonce provides uniqueness +- `[alice, bob, charlie]` + threshold=2 + nonce=0 → `address_1` +- `[charlie, bob, alice]` + threshold=2 + nonce=0 → `address_1` (same!) +- To create multiple multisigs with same signers, use different nonce: + - `signers=[alice, bob], threshold=2, nonce=0` → `address_A` + - `signers=[alice, bob], threshold=2, nonce=1` → `address_B` (different!) **Economic Costs:** -- **MultisigFee**: Non-refundable fee (spam prevention) → burned -- **MultisigDeposit**: Refundable deposit (storage rent) → returned when multisig dissolved +- **MultisigFee**: Non-refundable fee (spam prevention) → burned immediately +- **MultisigDeposit**: Reserved deposit (storage bond) → returned to creator when multisig dissolved ### 2. Propose Transaction Creates a new proposal for multisig execution. @@ -61,29 +69,17 @@ Creates a new proposal for multisig execution. **Validation:** - Caller must be a signer +- **High-Security Check:** If multisig is high-security, only whitelisted calls are allowed (see High-Security Integration section) - Call size must be ≤ MaxCallSize - Multisig cannot have MaxTotalProposalsInStorage or more total proposals in storage - Caller cannot exceed their per-signer proposal limit (`MaxTotalProposalsInStorage / signers_count`) - Expiry must be in the future (expiry > current_block) - Expiry must not exceed MaxExpiryDuration blocks from now (expiry ≤ current_block + MaxExpiryDuration) -**Auto-Cleanup Before Creation:** -Before creating a new proposal, the system **automatically removes all expired Active proposals** for this multisig: -- Expired proposals are identified (current_block > expiry) -- Deposits are returned to original proposers -- Storage is cleaned up -- Counters are decremented -- Events are emitted for each removed proposal - -This ensures storage is kept clean and users get their deposits back without manual intervention. +**No auto-cleanup in propose:** The pallet does **not** remove expired proposals when creating a new one. To free slots and recover deposits from expired proposals, the proposer must call `claim_deposits()` or any signer can call `remove_expired()` for individual proposals. -**Threshold=1 Auto-Execution:** -If the multisig has `threshold=1`, the proposal **executes immediately** after creation: -- Proposer's approval counts as the first (and only required) approval -- Call is dispatched automatically -- Proposal is removed from storage immediately -- Deposit is returned to proposer immediately -- No separate `approve()` call needed +**Threshold=1 behaviour:** +If the multisig has `threshold=1`, the proposal becomes **Approved** immediately after creation (proposer counts as the only required approval). The proposer (or any signer) must then call `execute()` to dispatch the call and remove the proposal. **Economic Costs:** - **ProposalFee**: Non-refundable fee (spam prevention, scaled by signer count) → burned @@ -92,8 +88,7 @@ If the multisig has `threshold=1`, the proposal **executes immediately** after c **Important:** Fee is ALWAYS paid, even if proposal expires or is cancelled. Only deposit is refundable. ### 3. Approve Transaction -Adds caller's approval to an existing proposal. **If this approval brings the total approvals -to or above the threshold, the transaction will be automatically executed and immediately removed from storage.** +Adds caller's approval to an existing proposal. **If this approval brings the total approvals to or above the threshold, the proposal status becomes `Approved`**; the call is **not** executed here—use `execute()` for that. **Required Parameters:** - `multisig_address: AccountId` - Target multisig (REQUIRED) @@ -101,18 +96,16 @@ to or above the threshold, the transaction will be automatically executed and im **Validation:** - Caller must be a signer -- Proposal must exist +- Proposal must exist and be Active - Proposal must not be expired (current_block ≤ expiry) - Caller must not have already approved -**Auto-Execution:** -When approval count reaches the threshold: -- Encoded call is executed as multisig_address origin -- Proposal **immediately removed** from storage -- ProposalDeposit **immediately returned** to proposer -- TransactionExecuted event emitted with execution result +**When threshold is reached:** +- Proposal status is set to `Approved` +- `ProposalReadyToExecute` event is emitted +- Any signer can then call `execute()` to dispatch the call -**Economic Costs:** None (deposit immediately returned on execution) +**Economic Costs:** None (deposit is returned when the proposal is executed or cancelled). ### 4. Cancel Transaction Cancels a proposal and immediately removes it from storage (proposer only). @@ -123,21 +116,39 @@ Cancels a proposal and immediately removes it from storage (proposer only). **Validation:** - Caller must be the proposer -- Proposal must exist and be Active +- Proposal must exist and be **Active or Approved** (both can be cancelled) **Economic Effects:** - Proposal **immediately removed** from storage - ProposalDeposit **immediately returned** to proposer -- Counters decremented +- Counters decremented (active_proposals, proposals_per_signer) **Economic Costs:** None (deposit immediately returned) -**Note:** ProposalFee is NOT refunded - it was burned at proposal creation. +**Note:** ProposalFee is NOT refunded (it was burned at proposal creation). + +### 5. Execute Transaction +Dispatches an **Approved** proposal. Can be called by any signer of the multisig once the approval threshold has been reached. + +**Required Parameters:** +- `multisig_address: AccountId` - Target multisig (REQUIRED) +- `proposal_id: u32` - ID (nonce) of the proposal to execute (REQUIRED) + +**Validation:** +- Caller must be a signer +- Proposal must exist and have status **Approved** +- Proposal must not be expired (current_block ≤ expiry) + +**Effects:** +- Call is decoded and dispatched with multisig_address as origin +- Proposal is removed from storage +- ProposalDeposit is returned to the proposer +- `ProposalExecuted` event is emitted -### 5. Remove Expired -Manually removes expired proposals from storage. Only signers can call this. +**Economic Costs:** Weight depends on call size (charged upfront for MaxCallSize, refunded for actual size). -**Important:** This is rarely needed because expired proposals are automatically cleaned up on any multisig activity (`propose()`, `approve()`, `cancel()`). +### 6. Remove Expired +Manually removes a single expired **Active** proposal from storage. Only signers can call this. Deposit is returned to the original proposer. **Required Parameters:** - `multisig_address: AccountId` - Target multisig (REQUIRED) @@ -148,21 +159,17 @@ Manually removes expired proposals from storage. Only signers can call this. - Proposal must exist and be Active - Must be expired (current_block > expiry) -**Note:** Executed/Cancelled proposals are automatically removed immediately, so this only applies to Active+Expired proposals. +**Note:** Executed/Cancelled proposals are removed immediately when executed/cancelled. This extrinsic only applies to **Active** proposals that are past expiry. **Economic Effects:** - ProposalDeposit returned to **original proposer** (not caller) - Proposal removed from storage -- Counters decremented +- Counters decremented (active_proposals, proposals_per_signer) **Economic Costs:** None (deposit always returned to proposer) -**Auto-Cleanup:** ALL expired proposals are automatically removed on any multisig activity (`propose()`, `approve()`, `cancel()`), making this function often unnecessary. - -### 6. Claim Deposits -Batch cleanup operation to recover all expired proposal deposits. - -**Important:** This is rarely needed because expired proposals are automatically cleaned up on any multisig activity (`propose()`, `approve()`, `cancel()`). +### 7. Claim Deposits +Batch cleanup operation to recover all caller's expired proposal deposits. **Required Parameters:** - `multisig_address: AccountId` - Target multisig (REQUIRED) @@ -172,34 +179,49 @@ Batch cleanup operation to recover all expired proposal deposits. - Only removes Active+Expired proposals (Executed/Cancelled already auto-removed) - Must be expired (current_block > expiry) +**Behavior:** +- Iterates through ALL proposals in the multisig +- Removes all that match: proposer=caller AND expired AND status=Active +- No iteration limits - cleans all in one call + **Economic Effects:** - Returns all eligible proposal deposits to caller - Removes all expired proposals from storage -- Counters decremented +- Counters decremented (active_proposals, proposals_per_signer) -**Economic Costs:** None (only returns deposits) +**Economic Costs:** +- Gas cost proportional to proposals iterated and cleaned (dynamic weight; charged upfront for worst-case, refunded for actual work) -**Auto-Cleanup:** ALL expired proposals are automatically removed on any multisig activity (`propose()`, `approve()`, `cancel()`), making this function often unnecessary. +**Note:** This is the main way to clean up a proposer's expired proposals and free per-signer quota (there is no auto-cleanup in `propose()`). -### 7. Dissolve Multisig -Permanently removes a multisig and returns the creation deposit to the original creator. +### 8. Approve Dissolve +Approve dissolving a multisig account. Requires threshold approvals to complete. **Required Parameters:** - `multisig_address: AccountId` - Target multisig (REQUIRED) **Pre-conditions:** +- Caller must be a signer - NO proposals can exist (any status) - Multisig balance MUST be zero -- Caller must be creator OR any signer -**Post-conditions:** -- MultisigDeposit returned to **original creator** (not caller) +**Approval Process:** +- Each signer calls `approve_dissolve()` +- Approvals are tracked in `DissolveApprovals` storage +- When threshold reached, multisig is automatically dissolved + +**Post-conditions (when threshold reached):** +- MultisigDeposit is **returned to creator** - Multisig removed from storage +- DissolveApprovals cleared - Cannot be used after dissolution -**Economic Costs:** None (returns MultisigDeposit) +**Economic Costs:** None (deposit returned to creator) -**Important:** MultisigFee is NEVER returned - only the MultisigDeposit. +**Important:** +- MultisigFee is NEVER returned (burned on creation) +- MultisigDeposit IS returned to the original creator +- Requires threshold approvals (not just any signer or creator) ## Use Cases @@ -242,25 +264,21 @@ matches!(call, - Spam attacks reduce circulating supply - Lower transaction costs (withdraw vs transfer) -### Deposits (Refundable, locked as storage rent) +### Deposits (Locked as storage rent) **Purpose:** Compensate for on-chain storage, incentivize cleanup - **MultisigDeposit**: - Reserved on multisig creation - - Returned when multisig dissolved (via `dissolve_multisig`) + - **Returned to creator** when multisig is dissolved (via `approve_dissolve` after threshold approvals) - Locked until no proposals exist and balance is zero - Opportunity cost incentivizes cleanup - **ProposalDeposit**: - Reserved on proposal creation - - **Auto-Returned Immediately:** - - When proposal executed (threshold reached) - - When proposal cancelled (proposer cancels) - - **Auto-Cleanup:** ALL expired proposals are automatically removed on ANY multisig activity - - Triggered by: `propose()`, `approve()`, `cancel()` - - Deposits returned to original proposers - - No manual cleanup needed for active multisigs - - **Manual Cleanup:** Only needed for inactive multisigs via `remove_expired()` or `claim_deposits()` + - **Refundable** - returned in following scenarios: + - **When proposal is executed:** Any signer calls `execute()` on an Approved proposal → deposit returned to proposer + - **When proposal is cancelled:** Proposer calls `cancel()` (Active or Approved) → deposit returned to proposer + - **Expired proposals:** No auto-cleanup in `propose()`. Proposer recovers deposits via `claim_deposits()`; any signer can remove a single expired proposal via `remove_expired()` (deposit → proposer) ### Storage Limits & Configuration **Purpose:** Prevent unbounded storage growth and resource exhaustion @@ -268,10 +286,9 @@ matches!(call, - **MaxSigners**: Maximum signers per multisig - Trade-off: Higher → more flexible governance, more computation per approval -- **MaxTotalProposalsInStorage**: Maximum total proposals (Active + Executed + Cancelled) +- **MaxTotalProposalsInStorage**: Maximum total proposals (Active + Approved; Executed/Cancelled are removed immediately) - Trade-off: Higher → more flexible, more storage risk - - Forces periodic cleanup to continue operating - - **Auto-cleanup**: Expired proposals are automatically removed when new proposals are created + - Forces periodic cleanup to continue operating (via `claim_deposits()` or `remove_expired()`) - **Per-Signer Limit**: Each signer gets `MaxTotalProposalsInStorage / signers_count` quota - Prevents single signer from monopolizing storage (filibuster protection) - Fair allocation ensures all signers can participate @@ -294,17 +311,18 @@ matches!(call, Stores multisig account data: ```rust MultisigData { - signers: BoundedVec, // List of authorized signers + creator: AccountId, // Original creator (receives deposit back on dissolve) + signers: BoundedVec, // List of authorized signers (sorted) threshold: u32, // Required approvals - nonce: u64, // Unique identifier used in address generation - deposit: Balance, // Reserved deposit (refundable) - creator: AccountId, // Who created it (receives deposit back) - last_activity: BlockNumber, // Last action timestamp (for grace period) - active_proposals: u32, // Count of open proposals (monitoring/analytics) + proposal_nonce: u32, // Counter for unique proposal IDs + deposit: Balance, // Reserved deposit (returned to creator on dissolve) + active_proposals: u32, // Count of active proposals (for limits) proposals_per_signer: BoundedBTreeMap, // Per-signer proposal count (filibuster protection) } ``` +**Note:** Address is deterministically derived from `hash(pallet_id || sorted_signers || threshold || nonce)` where nonce is user-provided at creation time. + ### Proposals: DoubleMap Stores proposal data indexed by (multisig_address, proposal_id): ```rust @@ -314,25 +332,36 @@ ProposalData { expiry: BlockNumber, // Deadline for approvals approvals: BoundedVec, // List of signers who approved deposit: Balance, // Reserved deposit (refundable) - status: ProposalStatus, // Active only (Executed/Cancelled are removed immediately) + status: ProposalStatus, // Active | Approved (Executed/Cancelled are removed immediately) +} + +enum ProposalStatus { + Active, // Collecting approvals + Approved, // Threshold reached; any signer can call execute() + // Executed and Cancelled are not stored — proposal is removed immediately } ``` -**Important:** Only **Active** proposals are stored. Executed and Cancelled proposals are **immediately removed** from storage and their deposits are returned. Historical data is available through events (see Historical Data section below). +**Important:** Only **Active** and **Approved** proposals are stored. When a proposal is executed or cancelled, it is **immediately removed** from storage and the deposit is returned. Historical data is available through events (see Historical Data section below). -### GlobalNonce: u64 -Internal counter for generating unique multisig addresses. Not exposed via API. +### DissolveApprovals: Map> +Tracks which signers have approved dissolving each multisig. +- Key: Multisig address +- Value: List of signers who approved dissolution +- Cleared when multisig is dissolved or when threshold reached ## Events - `MultisigCreated { creator, multisig_address, signers, threshold, nonce }` - `ProposalCreated { multisig_address, proposer, proposal_id }` - `ProposalApproved { multisig_address, approver, proposal_id, approvals_count }` +- `ProposalReadyToExecute { multisig_address, proposal_id, approvals_count }` — emitted when threshold is reached (approve or propose with threshold=1); proposal is Approved until someone calls `execute()` - `ProposalExecuted { multisig_address, proposal_id, proposer, call, approvers, result }` - `ProposalCancelled { multisig_address, proposer, proposal_id }` - `ProposalRemoved { multisig_address, proposal_id, proposer, removed_by }` - `DepositsClaimed { multisig_address, claimer, total_returned, proposals_removed, multisig_removed }` -- `MultisigDissolved { multisig_address, caller, deposit_returned }` +- `DissolveApproved { multisig_address, approver, approvals_count }` +- `MultisigDissolved { multisig_address, deposit_returned, approvers }` ## Errors @@ -357,7 +386,10 @@ Internal counter for generating unique multisig addresses. Not exposed via API. - `TooManyProposalsInStorage` - Multisig has MaxTotalProposalsInStorage total proposals (cleanup required to create new) - `TooManyProposalsPerSigner` - Caller has reached their per-signer proposal limit (`MaxTotalProposalsInStorage / signers_count`) - `ProposalNotExpired` - Proposal not yet expired (for remove_expired) -- `ProposalNotActive` - Proposal is not active (already executed or cancelled) +- `ProposalNotActive` - Proposal is not active or approved (already executed or cancelled) +- `ProposalNotApproved` - Proposal is not in Approved status (for `execute()`) +- `ProposalsExist` - Cannot dissolve multisig while proposals exist +- `MultisigAccountNotZero` - Cannot dissolve multisig with non-zero balance ## Important Behavior @@ -381,18 +413,21 @@ approve(multisig, 1) // Approve proposal #1 ### Signer Order Doesn't Matter Signers are **automatically sorted** before address generation and storage: - Input order is irrelevant - signers are always sorted deterministically -- Address is derived from `Hash(PalletId + sorted_signers + nonce)` -- Same signers in any order = same multisig address (with same nonce) -- To create multiple multisigs with same participants, use different creation transactions (nonce auto-increments) +- Address is derived from `Hash(PalletId + sorted_signers + threshold + nonce)` +- Same signers+threshold+nonce in any order = same multisig address +- User must provide unique nonce to create multiple multisigs with same signers **Example:** ```rust -// These create the SAME multisig address (same signers, same nonce): -create_multisig([alice, bob, charlie], 2) // → multisig_addr_1 (nonce=0) -create_multisig([charlie, bob, alice], 2) // → multisig_addr_1 (SAME! nonce would be 1 but already exists) +// These create the SAME multisig address (same signers, threshold, nonce): +create_multisig([alice, bob, charlie], 2, 0) // → multisig_addr_1 +create_multisig([charlie, bob, alice], 2, 0) // → multisig_addr_1 (SAME!) -// To create another multisig with same signers: -create_multisig([alice, bob, charlie], 2) // → multisig_addr_2 (nonce=1, different address) +// To create another multisig with same signers, use different nonce: +create_multisig([alice, bob, charlie], 2, 1) // → multisig_addr_2 (different!) + +// Different threshold = different address (even with same nonce): +create_multisig([alice, bob, charlie], 3, 0) // → multisig_addr_3 (different!) ``` ## Historical Data and Event Indexing @@ -446,12 +481,12 @@ This event structure is optimized for indexing by SubSquid and similar indexers: - Deposits (refundable) prevent storage bloat - MaxTotalProposalsInStorage caps total storage per multisig - Per-signer limits prevent single signer from monopolizing storage (filibuster protection) -- Auto-cleanup of expired proposals reduces storage pressure +- Explicit cleanup (claim_deposits, remove_expired) keeps storage under control ### Storage Cleanup -- Grace period allows proposers priority cleanup -- After grace: public cleanup incentivized -- Batch cleanup via claim_deposits for efficiency +- No auto-cleanup in `propose()` (predictable weight; proposer must free slots via cleanup) +- Manual cleanup via `remove_expired()`: any signer can remove a single expired Active proposal (deposit → proposer) +- Batch cleanup via `claim_deposits()`: proposer recovers all their expired proposal deposits at once and frees per-signer quota ### Economic Attacks - **Multisig Spam:** Costs MultisigFee (burned, reduces supply) @@ -485,15 +520,15 @@ impl pallet_multisig::Config for Runtime { // Storage limits (prevent unbounded growth) type MaxSigners = ConstU32<100>; // Max complexity - type MaxTotalProposalsInStorage = ConstU32<200>; // Total storage cap (auto-cleanup on propose) + type MaxTotalProposalsInStorage = ConstU32<200>; // Total storage cap (cleanup via claim_deposits/remove_expired) type MaxCallSize = ConstU32<10240>; // Per-proposal storage limit type MaxExpiryDuration = ConstU32<100_800>; // Max proposal lifetime (~2 weeks @ 12s) // Economic parameters (example values - adjust per runtime) - type MultisigFee = ConstU128<{ 100 * MILLI_UNIT }>; // Creation barrier - type MultisigDeposit = ConstU128<{ 500 * MILLI_UNIT }>; // Storage rent - type ProposalFee = ConstU128<{ 1000 * MILLI_UNIT }>; // Base proposal cost - type ProposalDeposit = ConstU128<{ 1000 * MILLI_UNIT }>; // Cleanup incentive + type MultisigFee = ConstU128<{ 100 * MILLI_UNIT }>; // Creation barrier (burned) + type MultisigDeposit = ConstU128<{ 500 * MILLI_UNIT }>; // Storage bond (returned to creator on dissolve) + type ProposalFee = ConstU128<{ 1000 * MILLI_UNIT }>; // Base proposal cost (burned) + type ProposalDeposit = ConstU128<{ 1000 * MILLI_UNIT }>; // Storage rent (refundable) type SignerStepFactor = Permill::from_percent(1); // Dynamic pricing (1% per signer) type PalletId = ConstPalletId(*b"py/mltsg"); @@ -507,6 +542,188 @@ impl pallet_multisig::Config for Runtime { - **Enterprise use:** Higher MaxSigners, longer MaxExpiryDuration - **Public use:** Moderate limits, shorter expiry for faster turnover +## High-Security Integration + +The multisig pallet integrates with **pallet-reversible-transfers** to support high-security multisigs with call whitelisting and delayed execution. + +### Overview + +**Standard Multisig:** +- Proposes any `RuntimeCall` +- Executes immediately on threshold +- No restrictions + +**High-Security Multisig:** +- **Whitelist enforced:** Only allowed calls can be proposed +- **Delayed execution:** Via `ReversibleTransfers::schedule_transfer()` +- **Guardian oversight:** Guardian can cancel during delay period +- **Use case:** Corporate treasury, regulated operations, high-value custody + +### ⚠️ Important: Enabling High-Security + +**Risk Window:** +When enabling high-security for an existing multisig with active proposals: +1. **Existing proposals** are NOT automatically blocked +2. **Whitelist check** only happens at proposal creation time (`propose()`) +3. **Proposals created before HS** can still be executed after HS is enabled + +**Mitigation:** +Before enabling high-security, ensure: +- ✅ All active proposals are **completed** (executed or cancelled) +- ✅ All proposals have **expired** or been **removed** +- ✅ No pending approvals exist + +**Safe workflow:** +```rust +// 1. Check for active proposals +let proposals = query_proposals(multisig_address); +assert_eq!(proposals.len(), 0, "Must cleanup proposals first"); + +// 2. Cancel or wait for expiry +for proposal_id in proposals { + Multisig::cancel(Origin::signed(proposer), multisig_address, proposal_id); + // OR: wait for expiry +} + +// 3. NOW enable high-security +ReversibleTransfers::set_high_security( + Origin::signed(multisig_address), + delay: 100_800, + guardian: guardian_account +); +``` + +**Why this design:** +- **Simplicity:** Single check point (`propose`) easier to reason about +- **Gas efficiency:** No decode overhead on every approval +- **User control:** Explicit transition management +- **Trade-off:** Performance and simplicity over defense-in-depth + +**Could be changed:** +Adding whitelist check in `approve()` (before execution) would close this window, +at the cost of: +- Higher gas on every approval for HS multisigs (~70M units for decode + check) +- More complex execution path +- Would make this a non-issue + +### How It Works + +1. **Setup:** Multisig account calls `ReversibleTransfers::set_high_security(delay, guardian)` +2. **Propose:** Only whitelisted calls allowed: + - ✅ `ReversibleTransfers::schedule_transfer` + - ✅ `ReversibleTransfers::schedule_asset_transfer` + - ✅ `ReversibleTransfers::cancel` + - ❌ All other calls → `CallNotAllowedForHighSecurityMultisig` error +3. **Approve:** Standard multisig approval process +4. **Execute:** Threshold reached → transfer scheduled with delay +5. **Guardian:** Can cancel via `ReversibleTransfers::cancel(tx_id)` during delay + +### Code Example + +```rust +// 1. Create standard 3-of-5 multisig +let multisig_addr = Multisig::create_multisig( + Origin::signed(alice), + vec![alice, bob, charlie, dave, eve], + 3, + 0 // nonce +); + +// 2. Enable high-security (via multisig proposal + approvals) +// Propose and get 3 approvals for: +ReversibleTransfers::set_high_security( + Origin::signed(multisig_addr), + delay: 100_800, // 2 weeks @ 12s blocks + guardian: guardian_account +); + +// 3. Now only whitelisted calls work +// ✅ ALLOWED: Schedule delayed transfer +Multisig::propose( + Origin::signed(alice), + multisig_addr, + RuntimeCall::ReversibleTransfers( + Call::schedule_transfer { dest: recipient, amount: 1000 } + ).encode(), + expiry +); +// → Whitelist check passes +// → Collect approvals +// → Transfer scheduled with 2-week delay +// → Guardian can cancel if suspicious + +// ❌ REJECTED: Direct transfer +Multisig::propose( + Origin::signed(alice), + multisig_addr, + RuntimeCall::Balances( + Call::transfer { dest: recipient, amount: 1000 } + ).encode(), + expiry +); +// → ERROR: CallNotAllowedForHighSecurityMultisig +// → Proposal fails immediately +``` + +### Performance Impact + +High-security multisigs have higher costs due to call validation: + +- **+1 DB read:** Check `ReversibleTransfers::HighSecurityAccounts` +- **+Decode overhead:** Variable cost based on call size (O(call_size)) +- **+Whitelist check:** ~10k units for pattern matching +- **Total overhead:** Base cost + decode cost proportional to call size + +**Dynamic weight refund:** +Normal multisigs automatically get refunded for unused high-security overhead. + +**Weight calculation:** +- `propose()` charges upfront for worst-case high-security path: `propose_high_security(call.len())`. Actual weight refunded based on path: `propose(call_size)` for normal multisig, `propose_high_security(call_size)` for HS. No cleanup in propose (no iteration/cleanup parameters). +- `execute()` charges upfront for `execute(MaxCallSize)`; actual weight refunded as `execute(actual_call_size)`. +- `claim_deposits()` charges upfront for worst-case iteration and cleanup; actual weight based on proposals iterated and cleaned (dynamic refund). + +**Security notes:** +- Call size is validated BEFORE decode to prevent DoS via oversized payloads +- Weight formula includes O(call_size) component for decode (HS path) to prevent underpayment +- Benchmarks must be regenerated after logic changes (see README / MULTISIG_REQ benchmarking section) + +See `MULTISIG_REQ.md` for detailed cost breakdown and benchmarking instructions. + +### Configuration + +```rust +impl pallet_multisig::Config for Runtime { + type HighSecurity = runtime::HighSecurityConfig; + // ... other config +} + +// Runtime implements HighSecurityInspector trait +// (trait defined in primitives/high-security crate) +pub struct HighSecurityConfig; +impl qp_high_security::HighSecurityInspector for HighSecurityConfig { + fn is_high_security(who: &AccountId) -> bool { + ReversibleTransfers::is_high_security_account(who) + } + + fn is_whitelisted(call: &RuntimeCall) -> bool { + matches!(call, + RuntimeCall::ReversibleTransfers(Call::schedule_transfer { .. }) | + RuntimeCall::ReversibleTransfers(Call::schedule_asset_transfer { .. }) | + RuntimeCall::ReversibleTransfers(Call::cancel { .. }) + ) + } + + fn guardian(who: &AccountId) -> Option { + ReversibleTransfers::get_guardian(who) + } +} +``` + +### Documentation + +- See `MULTISIG_REQ.md` for complete high-security integration requirements +- See `pallet-reversible-transfers` docs for guardian management and delay configuration + ## License MIT-0 diff --git a/pallets/multisig/src/benchmarking.rs b/pallets/multisig/src/benchmarking.rs index 03db467f..59f45386 100644 --- a/pallets/multisig/src/benchmarking.rs +++ b/pallets/multisig/src/benchmarking.rs @@ -1,11 +1,13 @@ //! Benchmarking setup for pallet-multisig use super::*; -use crate::Pallet as Multisig; +use crate::{ + BoundedApprovalsOf, BoundedCallOf, BoundedSignersOf, DissolveApprovals, MultisigDataOf, + Multisigs, Pallet as Multisig, ProposalDataOf, ProposalStatus, Proposals, +}; use alloc::vec; -use frame_benchmarking::{account as benchmark_account, v2::*, BenchmarkError}; -use frame_support::traits::{fungible::Mutate, ReservableCurrency}; -use frame_system::RawOrigin; +use frame_benchmarking::v2::*; +use frame_support::{traits::fungible::Mutate, BoundedBTreeMap}; const SEED: u32 = 0; @@ -30,538 +32,413 @@ where mod benchmarks { use super::*; use codec::Encode; + use frame_support::traits::ReservableCurrency; + use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; + use qp_high_security::HighSecurityInspector; + // ---------- Reusable setup helpers (keep benchmark bodies focused on what we measure) + // ---------- + + /// Funded caller + signers (sorted). Caller is first in the list. + fn setup_funded_signer_set( + signer_count: u32, + ) -> (T::AccountId, Vec) + where + BalanceOf2: From, + { + let caller: T::AccountId = whitelisted_caller(); + fund_account::(&caller, BalanceOf2::::from(100_000u128)); + let mut signers = vec![caller.clone()]; + for i in 0..signer_count.saturating_sub(1) { + let s: T::AccountId = account("signer", i, SEED); + fund_account::(&s, BalanceOf2::::from(100_000u128)); + signers.push(s); + } + signers.sort(); + (caller, signers) + } + + /// Funded caller + signers matching genesis (signer1, signer2). Multisig address is in + /// ReversibleTransfers::initial_high_security_accounts when runtime-benchmarks. + fn setup_funded_signer_set_hs( + ) -> (T::AccountId, Vec) + where + BalanceOf2: From, + { + let caller: T::AccountId = whitelisted_caller(); + let signer1: T::AccountId = account("signer1", 0, SEED); + let signer2: T::AccountId = account("signer2", 1, SEED); + fund_account::(&caller, BalanceOf2::::from(100_000u128)); + fund_account::(&signer1, BalanceOf2::::from(100_000u128)); + fund_account::(&signer2, BalanceOf2::::from(100_000u128)); + let mut signers = vec![caller.clone(), signer1, signer2]; + signers.sort(); + (caller, signers) + } + + /// Insert multisig into storage (bypasses create_multisig). Returns multisig address. + fn insert_multisig( + caller: &T::AccountId, + signers: &[T::AccountId], + threshold: u32, + nonce: u64, + proposal_nonce: u32, + active_proposals: u32, + ) -> T::AccountId { + let multisig_address = Multisig::::derive_multisig_address(signers, threshold, nonce); + let bounded_signers: BoundedSignersOf = signers.to_vec().try_into().unwrap(); + let data = MultisigDataOf:: { + creator: caller.clone(), + signers: bounded_signers, + threshold, + proposal_nonce, + deposit: T::MultisigDeposit::get(), + active_proposals, + proposals_per_signer: BoundedBTreeMap::new(), + }; + Multisigs::::insert(&multisig_address, data); + multisig_address + } + + fn set_block(n: u32) + where + BlockNumberFor: From, + { + frame_system::Pallet::::set_block_number(n.into()); + } + + /// Returns a Vec of MaxSigners account IDs for worst-case approvals decode cost. + fn approvals_max() -> Vec { + (0..T::MaxSigners::get()).map(|i| account("approval", i, SEED)).collect() + } + + /// Insert a single proposal into storage. `approvals` = list of account ids that have approved. + fn insert_proposal( + multisig_address: &T::AccountId, + proposal_id: u32, + proposer: &T::AccountId, + call_size: u32, + expiry: BlockNumberFor, + approvals: &[T::AccountId], + status: ProposalStatus, + deposit: crate::BalanceOf, + ) { + let system_call = frame_system::Call::::remark { remark: vec![1u8; call_size as usize] }; + let encoded = ::RuntimeCall::from(system_call).encode(); + let bounded_call: BoundedCallOf = encoded.try_into().unwrap(); + let bounded_approvals: BoundedApprovalsOf = approvals.to_vec().try_into().unwrap(); + let proposal_data = ProposalDataOf:: { + proposer: proposer.clone(), + call: bounded_call, + expiry, + approvals: bounded_approvals, + deposit, + status, + }; + Proposals::::insert(multisig_address, proposal_id, proposal_data); + } + + /// Benchmark `create_multisig` extrinsic. + /// Parameter: s = signers count #[benchmark] - fn create_multisig() -> Result<(), BenchmarkError> { + fn create_multisig(s: Linear<2, { T::MaxSigners::get() }>) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); // Fund the caller with enough balance for deposit fund_account::(&caller, BalanceOf2::::from(10000u128)); // Create signers (including caller) - let signer1: T::AccountId = benchmark_account("signer1", 0, SEED); - let signer2: T::AccountId = benchmark_account("signer2", 1, SEED); - let signers = vec![caller.clone(), signer1, signer2]; + let mut signers = vec![caller.clone()]; + for n in 0..s.saturating_sub(1) { + let signer: T::AccountId = account("signer", n, SEED); + signers.push(signer); + } let threshold = 2u32; + let nonce = 0u64; #[extrinsic_call] - _(RawOrigin::Signed(caller.clone()), signers.clone(), threshold); + _(RawOrigin::Signed(caller.clone()), signers.clone(), threshold, nonce); // Verify the multisig was created // Note: signers are sorted internally, so we must sort for address derivation let mut sorted_signers = signers.clone(); sorted_signers.sort(); - let multisig_address = Multisig::::derive_multisig_address(&sorted_signers, 0); + let multisig_address = + Multisig::::derive_multisig_address(&sorted_signers, threshold, nonce); assert!(Multisigs::::contains_key(multisig_address)); Ok(()) } + /// Benchmark `propose` extrinsic (non-HS path). + /// Uses different signers than propose_high_security so the multisig address is NOT in + /// HighSecurityAccounts (dev genesis records whitelisted_caller+signer1+signer2). No decode, no + /// whitelist. Parameter: c = call size #[benchmark] fn propose( c: Linear<0, { T::MaxCallSize::get().saturating_sub(100) }>, - e: Linear<0, { T::MaxTotalProposalsInStorage::get() }>, // expired proposals to cleanup ) -> Result<(), BenchmarkError> { - // Setup: Create a multisig first - let caller: T::AccountId = whitelisted_caller(); - fund_account::(&caller, BalanceOf2::::from(100000u128)); - - let signer1: T::AccountId = benchmark_account("signer1", 0, SEED); - let signer2: T::AccountId = benchmark_account("signer2", 1, SEED); - fund_account::(&signer1, BalanceOf2::::from(100000u128)); - fund_account::(&signer2, BalanceOf2::::from(100000u128)); - - let mut signers = vec![caller.clone(), signer1.clone(), signer2.clone()]; + // Uses account("signer", 0/1) so multisig address differs from genesis (signer1/signer2). + let (caller, signers) = setup_funded_signer_set::(3); let threshold = 2u32; - signers.sort(); - - // Create multisig directly in storage - let multisig_address = Multisig::::derive_multisig_address(&signers, 0); - let bounded_signers: BoundedSignersOf = signers.clone().try_into().unwrap(); - let multisig_data = MultisigDataOf:: { - signers: bounded_signers, - threshold, - nonce: 0, - proposal_nonce: e, // We'll insert e expired proposals - creator: caller.clone(), - deposit: T::MultisigDeposit::get(), - last_activity: frame_system::Pallet::::block_number(), - active_proposals: e, - proposals_per_signer: BoundedBTreeMap::new(), - }; - Multisigs::::insert(&multisig_address, multisig_data); - - // Insert e expired proposals (worst case for auto-cleanup) - let expired_block = 10u32.into(); - for i in 0..e { - let system_call = frame_system::Call::::remark { remark: vec![i as u8; 10] }; - let call = ::RuntimeCall::from(system_call); - let encoded_call = call.encode(); - let bounded_call: BoundedCallOf = encoded_call.try_into().unwrap(); - let bounded_approvals: BoundedApprovalsOf = vec![caller.clone()].try_into().unwrap(); - - let proposal_data = ProposalDataOf:: { - proposer: caller.clone(), - call: bounded_call, - expiry: expired_block, - approvals: bounded_approvals, - deposit: 10u32.into(), - status: ProposalStatus::Active, - }; - Proposals::::insert(&multisig_address, i, proposal_data); - } - - // Move past expiry so proposals are expired - frame_system::Pallet::::set_block_number(100u32.into()); - - // Create a new proposal (will auto-cleanup all e expired proposals) - let system_call = frame_system::Call::::remark { remark: vec![99u8; c as usize] }; - let call = ::RuntimeCall::from(system_call); - let encoded_call = call.encode(); + let multisig_address = insert_multisig::(&caller, &signers, threshold, 0, 0, 0); + assert!( + !T::HighSecurity::is_high_security(&multisig_address), + "propose must hit non-HS path" + ); + set_block::(100); + + let new_call = frame_system::Call::::remark { remark: vec![99u8; c as usize] }; + let encoded_call = ::RuntimeCall::from(new_call).encode(); let expiry = frame_system::Pallet::::block_number() + 1000u32.into(); #[extrinsic_call] _(RawOrigin::Signed(caller.clone()), multisig_address.clone(), encoded_call, expiry); - // Verify new proposal was created and expired ones were cleaned let multisig = Multisigs::::get(&multisig_address).unwrap(); - assert_eq!(multisig.active_proposals, 1); // Only new proposal remains - + assert_eq!(multisig.active_proposals, 1); Ok(()) } + /// Benchmark `propose` for high-security multisigs. + /// Uses signer1/signer2 so multisig address matches genesis (ReversibleTransfers:: + /// initial_high_security_accounts). HighSecurityAccounts::contains_key reads from trie. #[benchmark] - fn approve( + fn propose_high_security( c: Linear<0, { T::MaxCallSize::get().saturating_sub(100) }>, - e: Linear<0, { T::MaxTotalProposalsInStorage::get() }>, // expired proposals to cleanup ) -> Result<(), BenchmarkError> { - // Setup: Create multisig and proposal directly in storage - // Threshold is 3, so adding one more approval won't trigger execution - let caller: T::AccountId = whitelisted_caller(); - fund_account::(&caller, BalanceOf2::::from(100000u128)); - - let signer1: T::AccountId = benchmark_account("signer1", 0, SEED); - let signer2: T::AccountId = benchmark_account("signer2", 1, SEED); - let signer3: T::AccountId = benchmark_account("signer3", 2, SEED); - fund_account::(&signer1, BalanceOf2::::from(100000u128)); - fund_account::(&signer2, BalanceOf2::::from(100000u128)); - fund_account::(&signer3, BalanceOf2::::from(100000u128)); - - let mut signers = vec![caller.clone(), signer1.clone(), signer2.clone(), signer3.clone()]; - let threshold = 3u32; // Need 3 approvals - - // Sort signers to match create_multisig behavior - signers.sort(); - - // Directly insert multisig into storage - let multisig_address = Multisig::::derive_multisig_address(&signers, 0); - let bounded_signers: BoundedSignersOf = signers.clone().try_into().unwrap(); - let multisig_data = MultisigDataOf:: { - signers: bounded_signers, - threshold, - nonce: 0, - proposal_nonce: e + 1, // We'll insert e expired proposals + 1 active - creator: caller.clone(), - deposit: T::MultisigDeposit::get(), - last_activity: frame_system::Pallet::::block_number(), - active_proposals: e + 1, - proposals_per_signer: BoundedBTreeMap::new(), - }; - Multisigs::::insert(&multisig_address, multisig_data); + let (caller, signers) = setup_funded_signer_set_hs::(); + let threshold = 2u32; + let multisig_address = insert_multisig::(&caller, &signers, threshold, 0, 0, 0); + assert!( + T::HighSecurity::is_high_security(&multisig_address), + "propose_high_security must hit HS path" + ); + set_block::(100); + + let new_call = frame_system::Call::::remark { remark: vec![99u8; c as usize] }; + let encoded_call = ::RuntimeCall::from(new_call).encode(); + let expiry = frame_system::Pallet::::block_number() + 1000u32.into(); - // Insert e expired proposals (worst case for auto-cleanup) - let expired_block = 10u32.into(); - for i in 0..e { - let system_call = frame_system::Call::::remark { remark: vec![i as u8; 10] }; - let call = ::RuntimeCall::from(system_call); - let encoded_call = call.encode(); - let bounded_call: BoundedCallOf = encoded_call.try_into().unwrap(); - let bounded_approvals: BoundedApprovalsOf = vec![caller.clone()].try_into().unwrap(); - - let proposal_data = ProposalDataOf:: { - proposer: caller.clone(), - call: bounded_call, - expiry: expired_block, - approvals: bounded_approvals, - deposit: 10u32.into(), - status: ProposalStatus::Active, - }; - Proposals::::insert(&multisig_address, i, proposal_data); - } + #[extrinsic_call] + propose(RawOrigin::Signed(caller.clone()), multisig_address.clone(), encoded_call, expiry); - // Move past expiry so proposals are expired - frame_system::Pallet::::set_block_number(100u32.into()); + let multisig = Multisigs::::get(&multisig_address).unwrap(); + assert_eq!(multisig.active_proposals, 1); + Ok(()) + } - // Directly insert active proposal into storage with 1 approval - // Create a remark call where the remark itself is c bytes - let system_call = frame_system::Call::::remark { remark: vec![1u8; c as usize] }; - let call = ::RuntimeCall::from(system_call); - let encoded_call = call.encode(); + /// Benchmark `approve` extrinsic (without execution). Uses MaxSigners for worst-case approvals + /// decode. Threshold = MaxSigners, 99 approvals pre-stored, approver adds 100th. + /// Parameter: c = call size (stored proposal call) + #[benchmark] + fn approve( + c: Linear<0, { T::MaxCallSize::get().saturating_sub(100) }>, + ) -> Result<(), BenchmarkError> { + let max_s = T::MaxSigners::get(); + let (caller, signers) = setup_funded_signer_set::(max_s); + let threshold = max_s; + let multisig_address = insert_multisig::(&caller, &signers, threshold, 0, 1, 1); + set_block::(100); let expiry = frame_system::Pallet::::block_number() + 1000u32.into(); - let bounded_call: BoundedCallOf = encoded_call.clone().try_into().unwrap(); - let bounded_approvals: BoundedApprovalsOf = vec![caller.clone()].try_into().unwrap(); - - let proposal_data = ProposalDataOf:: { - proposer: caller.clone(), - call: bounded_call, + // Worst-case approvals decode: threshold-1 approvals (99 for MaxSigners=100) + let approvals: Vec<_> = signers[0..threshold as usize - 1].to_vec(); + insert_proposal::( + &multisig_address, + 0, + &caller, + c, expiry, - approvals: bounded_approvals, - deposit: 10u32.into(), - status: ProposalStatus::Active, - }; - - let proposal_id = e; // Active proposal after expired ones - Proposals::::insert(&multisig_address, proposal_id, proposal_data); + &approvals, + ProposalStatus::Active, + 10u32.into(), + ); + let approver = signers[threshold as usize - 1].clone(); #[extrinsic_call] - _(RawOrigin::Signed(signer1.clone()), multisig_address.clone(), proposal_id); - - // Verify approval was added (now 2/3, not executed yet) - let proposal = Proposals::::get(&multisig_address, proposal_id).unwrap(); - assert!(proposal.approvals.contains(&signer1)); - assert_eq!(proposal.approvals.len(), 2); + _(RawOrigin::Signed(approver), multisig_address.clone(), 0u32); + let proposal = Proposals::::get(&multisig_address, 0).unwrap(); + assert_eq!(proposal.approvals.len(), threshold as usize); Ok(()) } + /// Benchmark `execute` extrinsic (dispatches an Approved proposal). + /// Uses MaxSigners approvals for worst-case decode. Parameter: c = call size #[benchmark] - fn approve_and_execute( + fn execute( c: Linear<0, { T::MaxCallSize::get().saturating_sub(100) }>, ) -> Result<(), BenchmarkError> { - // Benchmarks approve() when it triggers auto-execution (threshold reached) - let caller: T::AccountId = whitelisted_caller(); - fund_account::(&caller, BalanceOf2::::from(10000u128)); - - let signer1: T::AccountId = benchmark_account("signer1", 0, SEED); - let signer2: T::AccountId = benchmark_account("signer2", 1, SEED); - fund_account::(&signer1, BalanceOf2::::from(10000u128)); - fund_account::(&signer2, BalanceOf2::::from(10000u128)); - - let mut signers = vec![caller.clone(), signer1.clone(), signer2.clone()]; - let threshold = 2u32; - - // Sort signers to match create_multisig behavior - signers.sort(); - - // Directly insert multisig into storage - let multisig_address = Multisig::::derive_multisig_address(&signers, 0); - let bounded_signers: BoundedSignersOf = signers.clone().try_into().unwrap(); - let multisig_data = MultisigDataOf:: { - signers: bounded_signers, - threshold, - nonce: 0, - proposal_nonce: 1, // We'll insert proposal with id 0 - creator: caller.clone(), - deposit: T::MultisigDeposit::get(), - last_activity: frame_system::Pallet::::block_number(), - active_proposals: 1, - proposals_per_signer: BoundedBTreeMap::new(), - }; - Multisigs::::insert(&multisig_address, multisig_data); - - // Directly insert proposal with 1 approval (caller already approved) - // signer2 will approve and trigger execution - // Create a remark call where the remark itself is c bytes - let system_call = frame_system::Call::::remark { remark: vec![1u8; c as usize] }; - let call = ::RuntimeCall::from(system_call); - let encoded_call = call.encode(); + let max_s = T::MaxSigners::get(); + let (caller, signers) = setup_funded_signer_set::(max_s); + let threshold = max_s; + let multisig_address = insert_multisig::(&caller, &signers, threshold, 0, 1, 1); + set_block::(100); let expiry = frame_system::Pallet::::block_number() + 1000u32.into(); - let bounded_call: BoundedCallOf = encoded_call.clone().try_into().unwrap(); - // Only 1 approval so far - let bounded_approvals: BoundedApprovalsOf = vec![caller.clone()].try_into().unwrap(); - - let proposal_data = ProposalDataOf:: { - proposer: caller.clone(), - call: bounded_call, + // Worst-case approvals decode: MaxSigners approvals (Approved) + insert_proposal::( + &multisig_address, + 0, + &caller, + c, expiry, - approvals: bounded_approvals, - deposit: 10u32.into(), - status: ProposalStatus::Active, - }; - - let proposal_id = 0u32; - Proposals::::insert(&multisig_address, proposal_id, proposal_data); + &signers, + ProposalStatus::Approved, + 10u32.into(), + ); + let executor = signers[0].clone(); - // signer2 approves, reaching threshold (2/2), triggering auto-execution #[extrinsic_call] - approve(RawOrigin::Signed(signer2.clone()), multisig_address.clone(), proposal_id); - - // Verify proposal was removed from storage (auto-deleted after execution) - assert!(!Proposals::::contains_key(&multisig_address, proposal_id)); + _(RawOrigin::Signed(executor), multisig_address.clone(), 0u32); + assert!(!Proposals::::contains_key(&multisig_address, 0)); Ok(()) } + /// Benchmark `cancel` extrinsic. Uses MaxSigners approvals for worst-case decode. + /// Parameter: c = stored proposal call size #[benchmark] fn cancel( c: Linear<0, { T::MaxCallSize::get().saturating_sub(100) }>, - e: Linear<0, { T::MaxTotalProposalsInStorage::get() }>, // expired proposals to cleanup ) -> Result<(), BenchmarkError> { - // Setup: Create multisig and proposal directly in storage - let caller: T::AccountId = whitelisted_caller(); - fund_account::(&caller, BalanceOf2::::from(100000u128)); - - let signer1: T::AccountId = benchmark_account("signer1", 0, SEED); - let signer2: T::AccountId = benchmark_account("signer2", 1, SEED); - fund_account::(&signer1, BalanceOf2::::from(100000u128)); - fund_account::(&signer2, BalanceOf2::::from(100000u128)); - - let mut signers = vec![caller.clone(), signer1.clone(), signer2.clone()]; + let (caller, signers) = setup_funded_signer_set::(3); let threshold = 2u32; - - // Sort signers to match create_multisig behavior - signers.sort(); - - // Directly insert multisig into storage - let multisig_address = Multisig::::derive_multisig_address(&signers, 0); - let bounded_signers: BoundedSignersOf = signers.clone().try_into().unwrap(); - let multisig_data = MultisigDataOf:: { - signers: bounded_signers, - threshold, - nonce: 0, - proposal_nonce: e + 1, // We'll insert e expired proposals + 1 active - creator: caller.clone(), - deposit: T::MultisigDeposit::get(), - last_activity: frame_system::Pallet::::block_number(), - active_proposals: e + 1, - proposals_per_signer: BoundedBTreeMap::new(), - }; - Multisigs::::insert(&multisig_address, multisig_data); - - // Insert e expired proposals (worst case for auto-cleanup) - let expired_block = 10u32.into(); - for i in 0..e { - let system_call = frame_system::Call::::remark { remark: vec![i as u8; 10] }; - let call = ::RuntimeCall::from(system_call); - let encoded_call = call.encode(); - let bounded_call: BoundedCallOf = encoded_call.try_into().unwrap(); - let bounded_approvals: BoundedApprovalsOf = vec![caller.clone()].try_into().unwrap(); - - let proposal_data = ProposalDataOf:: { - proposer: caller.clone(), - call: bounded_call, - expiry: expired_block, - approvals: bounded_approvals, - deposit: 10u32.into(), - status: ProposalStatus::Active, - }; - Proposals::::insert(&multisig_address, i, proposal_data); - } - - // Move past expiry so proposals are expired - frame_system::Pallet::::set_block_number(100u32.into()); - - // Directly insert active proposal into storage - // Create a remark call where the remark itself is c bytes - let system_call = frame_system::Call::::remark { remark: vec![1u8; c as usize] }; - let call = ::RuntimeCall::from(system_call); - let encoded_call = call.encode(); + let multisig_address = insert_multisig::(&caller, &signers, threshold, 0, 1, 1); + set_block::(100); let expiry = frame_system::Pallet::::block_number() + 1000u32.into(); - let bounded_call: BoundedCallOf = encoded_call.clone().try_into().unwrap(); - let bounded_approvals: BoundedApprovalsOf = vec![caller.clone()].try_into().unwrap(); - - let proposal_data = ProposalDataOf:: { - proposer: caller.clone(), - call: bounded_call, + let approvals = approvals_max::(); + insert_proposal::( + &multisig_address, + 0, + &caller, + c, expiry, - approvals: bounded_approvals, - deposit: 10u32.into(), - status: ProposalStatus::Active, - }; - - let proposal_id = e; // Active proposal after expired ones - Proposals::::insert(&multisig_address, proposal_id, proposal_data); + &approvals, + ProposalStatus::Active, + T::ProposalDeposit::get(), + ); + ::Currency::reserve(&caller, T::ProposalDeposit::get()).unwrap(); #[extrinsic_call] - _(RawOrigin::Signed(caller.clone()), multisig_address.clone(), proposal_id); - - // Verify proposal was removed from storage (auto-deleted after cancellation) - assert!(!Proposals::::contains_key(&multisig_address, proposal_id)); + _(RawOrigin::Signed(caller.clone()), multisig_address.clone(), 0u32); + assert!(!Proposals::::contains_key(&multisig_address, 0)); Ok(()) } + /// Benchmark `remove_expired` extrinsic. Uses MaxSigners approvals for worst-case decode. + /// Parameter: c = stored proposal call size #[benchmark] - fn remove_expired() -> Result<(), BenchmarkError> { - // Setup: Create multisig and expired proposal directly in storage - let caller: T::AccountId = whitelisted_caller(); - fund_account::(&caller, BalanceOf2::::from(10000u128)); - - let signer1: T::AccountId = benchmark_account("signer1", 0, SEED); - let signer2: T::AccountId = benchmark_account("signer2", 1, SEED); - fund_account::(&signer1, BalanceOf2::::from(10000u128)); - fund_account::(&signer2, BalanceOf2::::from(10000u128)); - - let mut signers = vec![caller.clone(), signer1.clone(), signer2.clone()]; + fn remove_expired( + c: Linear<0, { T::MaxCallSize::get().saturating_sub(100) }>, + ) -> Result<(), BenchmarkError> { + let (caller, signers) = setup_funded_signer_set::(3); let threshold = 2u32; - - // Sort signers to match create_multisig behavior - signers.sort(); - - // Directly insert multisig into storage - let multisig_address = Multisig::::derive_multisig_address(&signers, 0); - let bounded_signers: BoundedSignersOf = signers.clone().try_into().unwrap(); - let multisig_data = MultisigDataOf:: { - signers: bounded_signers, - threshold, - nonce: 0, - proposal_nonce: 1, // We'll insert proposal with id 0 - creator: caller.clone(), - deposit: T::MultisigDeposit::get(), - last_activity: 1u32.into(), - active_proposals: 1, - proposals_per_signer: BoundedBTreeMap::new(), - }; - Multisigs::::insert(&multisig_address, multisig_data); - - // Create proposal with expired timestamp - let system_call = frame_system::Call::::remark { remark: vec![1u8; 32] }; - let call = ::RuntimeCall::from(system_call); - let encoded_call = call.encode(); - let expiry = 10u32.into(); // Already expired - let bounded_call: BoundedCallOf = encoded_call.clone().try_into().unwrap(); - let bounded_approvals: BoundedApprovalsOf = vec![caller.clone()].try_into().unwrap(); - - let proposal_data = ProposalDataOf:: { - proposer: caller.clone(), - call: bounded_call, + let multisig_address = insert_multisig::(&caller, &signers, threshold, 0, 1, 1); + let expiry = 10u32.into(); + let approvals = approvals_max::(); + insert_proposal::( + &multisig_address, + 0, + &caller, + c, expiry, - approvals: bounded_approvals, - deposit: 10u32.into(), - status: ProposalStatus::Active, - }; - - let proposal_id = 0u32; - Proposals::::insert(&multisig_address, proposal_id, proposal_data); - - // Move past expiry - frame_system::Pallet::::set_block_number(100u32.into()); + &approvals, + ProposalStatus::Active, + 10u32.into(), + ); + set_block::(100); - // Call as signer (caller is one of signers) #[extrinsic_call] - _(RawOrigin::Signed(caller.clone()), multisig_address.clone(), proposal_id); - - // Verify proposal was removed - assert!(!Proposals::::contains_key(&multisig_address, proposal_id)); + _(RawOrigin::Signed(caller.clone()), multisig_address.clone(), 0u32); + assert!(!Proposals::::contains_key(&multisig_address, 0)); Ok(()) } + /// Benchmark `claim_deposits` extrinsic. Uses MaxSigners approvals per proposal for worst-case + /// decode. Parameters: i = iterated proposals, r = removed (cleaned) proposals, + /// c = average stored call size (affects iteration cost) #[benchmark] fn claim_deposits( - p: Linear<1, { T::MaxTotalProposalsInStorage::get() }>, /* number of expired proposals - * to cleanup */ + i: Linear<1, { T::MaxTotalProposalsInStorage::get() }>, + r: Linear<1, { T::MaxTotalProposalsInStorage::get() }>, + c: Linear<0, { T::MaxCallSize::get().saturating_sub(100) }>, ) -> Result<(), BenchmarkError> { - // Setup: Create multisig with multiple expired proposals directly in storage - let caller: T::AccountId = whitelisted_caller(); - fund_account::(&caller, BalanceOf2::::from(100000u128)); + let cleaned_target = (r as u32).min(i); + let total_proposals = i; - let signer1: T::AccountId = benchmark_account("signer1", 0, SEED); - let signer2: T::AccountId = benchmark_account("signer2", 1, SEED); - fund_account::(&signer1, BalanceOf2::::from(10000u128)); - fund_account::(&signer2, BalanceOf2::::from(10000u128)); - - let mut signers = vec![caller.clone(), signer1.clone(), signer2.clone()]; + let (caller, signers) = setup_funded_signer_set::(3); let threshold = 2u32; + let multisig_address = + insert_multisig::(&caller, &signers, threshold, 0, total_proposals, total_proposals); - // Sort signers to match create_multisig behavior - signers.sort(); - - // Directly insert multisig into storage - let multisig_address = Multisig::::derive_multisig_address(&signers, 0); - let bounded_signers: BoundedSignersOf = signers.clone().try_into().unwrap(); - let multisig_data = MultisigDataOf:: { - signers: bounded_signers, - threshold, - nonce: 0, - proposal_nonce: p, // We'll insert p proposals with ids 0..p-1 - creator: caller.clone(), - deposit: T::MultisigDeposit::get(), - last_activity: 1u32.into(), - active_proposals: p, - proposals_per_signer: BoundedBTreeMap::new(), - }; - Multisigs::::insert(&multisig_address, multisig_data); - - // Create multiple expired proposals directly in storage - let expiry = 10u32.into(); // Already expired - - for i in 0..p { - let system_call = frame_system::Call::::remark { remark: vec![i as u8; 32] }; - let call = ::RuntimeCall::from(system_call); - let encoded_call = call.encode(); - let bounded_call: BoundedCallOf = encoded_call.clone().try_into().unwrap(); - let bounded_approvals: BoundedApprovalsOf = vec![caller.clone()].try_into().unwrap(); - - let proposal_data = ProposalDataOf:: { - proposer: caller.clone(), - call: bounded_call, + let approvals = approvals_max::(); + let expired_block = 10u32.into(); + let future_block = 999999u32.into(); + for idx in 0..total_proposals { + let expiry = if idx < cleaned_target { expired_block } else { future_block }; + insert_proposal::( + &multisig_address, + idx, + &caller, + c, expiry, - approvals: bounded_approvals, - deposit: 10u32.into(), - status: ProposalStatus::Active, - }; - - Proposals::::insert(&multisig_address, i, proposal_data); + &approvals, + ProposalStatus::Active, + 10u32.into(), + ); } - // Move past expiry - frame_system::Pallet::::set_block_number(100u32.into()); + set_block::(100); #[extrinsic_call] _(RawOrigin::Signed(caller.clone()), multisig_address.clone()); - // Verify all expired proposals were cleaned up - assert_eq!(Proposals::::iter_key_prefix(&multisig_address).count(), 0); - + let remaining = Proposals::::iter_key_prefix(&multisig_address).count() as u32; + assert_eq!(remaining, total_proposals - cleaned_target); Ok(()) } + /// Benchmark `approve_dissolve` when threshold is NOT reached. + /// Just adds an approval to DissolveApprovals (cheap path). #[benchmark] - fn dissolve_multisig() -> Result<(), BenchmarkError> { - // Setup: Create a clean multisig (no proposals, zero balance) - let caller: T::AccountId = whitelisted_caller(); - fund_account::(&caller, BalanceOf2::::from(10000u128)); + fn approve_dissolve() -> Result<(), BenchmarkError> { + let (caller, signers) = setup_funded_signer_set::(3); + let threshold = 3u32; // Need 3 approvals, we add 1st + let deposit = T::MultisigDeposit::get(); + ::Currency::reserve(&caller, deposit)?; - let signer1: T::AccountId = benchmark_account("signer1", 0, SEED); - let signer2: T::AccountId = benchmark_account("signer2", 1, SEED); + let multisig_address = insert_multisig::(&caller, &signers, threshold, 0, 0, 0); + // No pre-inserted approvals - caller adds first approval (threshold not reached) - let mut signers = vec![caller.clone(), signer1.clone(), signer2.clone()]; - let threshold = 2u32; + #[extrinsic_call] + approve_dissolve(RawOrigin::Signed(caller.clone()), multisig_address.clone()); - // Sort signers to match create_multisig behavior - signers.sort(); + assert!(Multisigs::::contains_key(&multisig_address)); + assert!(DissolveApprovals::::get(&multisig_address).unwrap().len() == 1); + Ok(()) + } - // Directly insert multisig into storage - let multisig_address = Multisig::::derive_multisig_address(&signers, 0); - let bounded_signers: BoundedSignersOf = signers.clone().try_into().unwrap(); + /// Benchmark `approve_dissolve` when threshold IS reached (dissolves multisig). + #[benchmark] + fn approve_dissolve_threshold_reached() -> Result<(), BenchmarkError> { + let (caller, signers) = setup_funded_signer_set::(3); + let threshold = 2u32; let deposit = T::MultisigDeposit::get(); + ::Currency::reserve(&caller, deposit)?; - // Reserve deposit from caller - T::Currency::reserve(&caller, deposit)?; - - let multisig_data = MultisigDataOf:: { - signers: bounded_signers, - threshold, - nonce: 0, - proposal_nonce: 0, - creator: caller.clone(), - deposit, - last_activity: frame_system::Pallet::::block_number(), - active_proposals: 0, // No proposals - proposals_per_signer: BoundedBTreeMap::new(), - }; - Multisigs::::insert(&multisig_address, multisig_data); - - // Ensure multisig address has zero balance (required for dissolution) - // Don't fund it at all + let multisig_address = insert_multisig::(&caller, &signers, threshold, 0, 0, 0); + // Pre-insert one approval from a signer that is NOT the caller (avoid AlreadyApproved). + let first_approval = signers.iter().find(|s| *s != &caller).unwrap().clone(); + let mut approvals = BoundedApprovalsOf::::default(); + approvals.try_push(first_approval).unwrap(); + DissolveApprovals::::insert(&multisig_address, approvals); #[extrinsic_call] - _(RawOrigin::Signed(caller.clone()), multisig_address.clone()); + approve_dissolve(RawOrigin::Signed(caller.clone()), multisig_address.clone()); - // Verify multisig was removed assert!(!Multisigs::::contains_key(&multisig_address)); - Ok(()) } diff --git a/pallets/multisig/src/lib.rs b/pallets/multisig/src/lib.rs index 3befe0fc..e8c65568 100644 --- a/pallets/multisig/src/lib.rs +++ b/pallets/multisig/src/lib.rs @@ -5,15 +5,20 @@ //! //! ## Features //! -//! - Create multisig addresses with configurable thresholds +//! - Create multisig addresses with deterministic generation (signers + threshold + user-provided +//! nonce) //! - Propose transactions for multisig approval //! - Approve proposed transactions -//! - Execute transactions once threshold is reached +//! - Execute transactions once threshold is reached (automatic) +//! - Auto-cleanup of proposer's expired proposals on propose() +//! - Per-signer proposal limits for filibuster protection //! //! ## Data Structures //! -//! - **Multisig**: Contains signers, threshold, and global nonce -//! - **Proposal**: Contains transaction data, proposer, expiry, and approvals +//! - **MultisigData**: Contains signers, threshold, proposal counter, deposit, and per-signer +//! tracking +//! - **ProposalData**: Contains transaction data, proposer, expiry, approvals, deposit, and status +//! - **DissolveApprovals**: Tracks threshold-based approvals for multisig dissolution #![cfg_attr(not(feature = "std"), no_std)] @@ -40,46 +45,38 @@ use sp_runtime::RuntimeDebug; /// Multisig account data #[derive(Encode, Decode, MaxEncodedLen, Clone, TypeInfo, RuntimeDebug, PartialEq, Eq)] -pub struct MultisigData -{ +pub struct MultisigData { + /// Account that created this multisig (receives deposit back on dissolve) + pub creator: AccountId, /// List of signers who can approve transactions pub signers: BoundedSigners, /// Number of approvals required to execute a transaction pub threshold: u32, - /// Global unique identifier for this multisig (for address derivation) - pub nonce: u64, - /// Proposal counter for unique proposal hashes + /// Proposal counter for unique proposal IDs pub proposal_nonce: u32, - /// Account that created this multisig - pub creator: AccountId, - /// Deposit reserved by the creator + /// Deposit reserved by the creator (returned on dissolve) pub deposit: Balance, - /// Last block when this multisig was used - pub last_activity: BlockNumber, - /// Number of currently active (non-executed/non-cancelled) proposals + /// Number of active proposals (for global limit checking) pub active_proposals: u32, - /// Counter of proposals in storage per signer (for filibuster protection) + /// Per-signer proposal count (for filibuster protection) + /// Maps AccountId -> number of active proposals pub proposals_per_signer: BoundedProposalsPerSigner, } impl< - BlockNumber: Default, AccountId: Default, BoundedSigners: Default, Balance: Default, BoundedProposalsPerSigner: Default, - > Default - for MultisigData + > Default for MultisigData { fn default() -> Self { Self { + creator: Default::default(), signers: Default::default(), threshold: 1, - nonce: 0, proposal_nonce: 0, - creator: Default::default(), deposit: Default::default(), - last_activity: Default::default(), active_proposals: 0, proposals_per_signer: Default::default(), } @@ -91,6 +88,8 @@ impl< pub enum ProposalStatus { /// Proposal is active and awaiting approvals Active, + /// Proposal has reached threshold and is ready to execute + Approved, /// Proposal was executed successfully Executed, /// Proposal was cancelled by proposer @@ -125,13 +124,15 @@ pub mod pallet { use codec::Encode; use frame_support::{ dispatch::{ - DispatchResult, DispatchResultWithPostInfo, GetDispatchInfo, Pays, PostDispatchInfo, + DispatchErrorWithPostInfo, DispatchResult, DispatchResultWithPostInfo, GetDispatchInfo, + Pays, PostDispatchInfo, }, pallet_prelude::*, traits::{Currency, ReservableCurrency}, PalletId, }; use frame_system::pallet_prelude::*; + use qp_high_security::HighSecurityInspector; use sp_arithmetic::traits::Saturating; use sp_runtime::{ traits::{Dispatchable, Hash, TrailingZeroInput}, @@ -205,6 +206,12 @@ pub mod pallet { /// Weight information for extrinsics type WeightInfo: WeightInfo; + + /// Interface to check if an account is in high-security mode + type HighSecurity: qp_high_security::HighSecurityInspector< + Self::AccountId, + ::RuntimeCall, + >; } /// Type alias for bounded signers vector @@ -218,13 +225,12 @@ pub mod pallet { /// Type alias for bounded call data pub type BoundedCallOf = BoundedVec::MaxCallSize>; - /// Type alias for bounded proposals per signer map + /// Type alias for per-signer proposal counts pub type BoundedProposalsPerSignerOf = BoundedBTreeMap<::AccountId, u32, ::MaxSigners>; /// Type alias for MultisigData with proper bounds pub type MultisigDataOf = MultisigData< - BlockNumberFor, ::AccountId, BoundedSignersOf, BalanceOf, @@ -240,11 +246,7 @@ pub mod pallet { BoundedApprovalsOf, >; - /// Global nonce for generating unique multisig addresses - #[pallet::storage] - pub type GlobalNonce = StorageValue<_, u64, ValueQuery>; - - /// Multisigs stored by their generated address + /// Multisigs stored by their deterministic address #[pallet::storage] #[pallet::getter(fn multisigs)] pub type Multisigs = @@ -263,6 +265,12 @@ pub mod pallet { OptionQuery, >; + /// Dissolve approvals: tracks which signers approved dissolving the multisig + /// Maps multisig_address -> Vec + #[pallet::storage] + pub type DissolveApprovals = + StorageMap<_, Blake2_128Concat, T::AccountId, BoundedApprovalsOf, OptionQuery>; + #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { @@ -284,6 +292,12 @@ pub mod pallet { proposal_id: u32, approvals_count: u32, }, + /// A proposal has reached threshold and is ready to execute + ProposalReadyToExecute { + multisig_address: T::AccountId, + proposal_id: u32, + approvals_count: u32, + }, /// A proposal has been executed /// Contains all data needed for indexing by SubSquid ProposalExecuted { @@ -315,11 +329,17 @@ pub mod pallet { proposals_removed: u32, multisig_removed: bool, }, - /// A multisig account was dissolved and deposit returned + /// A signer approved dissolving the multisig + DissolveApproved { + multisig_address: T::AccountId, + approver: T::AccountId, + approvals_count: u32, + }, + /// A multisig account was dissolved (threshold reached) MultisigDissolved { multisig_address: T::AccountId, - caller: T::AccountId, - deposit_returned: BalanceOf, + deposit_returned: T::AccountId, // Creator who receives the deposit back + approvers: Vec, }, } @@ -371,28 +391,40 @@ pub mod pallet { ProposalNotExpired, /// Proposal is not active (already executed or cancelled) ProposalNotActive, + /// Proposal has not been approved yet (threshold not reached) + ProposalNotApproved, /// Cannot dissolve multisig with existing proposals (clear them first) ProposalsExist, /// Multisig account must have zero balance before dissolution MultisigAccountNotZero, + /// Call is not allowed for high-security multisig + CallNotAllowedForHighSecurityMultisig, } #[pallet::call] impl Pallet { - /// Create a new multisig account + /// Create a new multisig account with deterministic address /// /// Parameters: /// - `signers`: List of accounts that can sign for this multisig /// - `threshold`: Number of approvals required to execute transactions + /// - `nonce`: User-provided nonce for address uniqueness /// - /// The multisig address is derived from a hash of all signers + global nonce. - /// The creator must pay a non-refundable fee (burned). + /// The multisig address is deterministically derived from: + /// hash(pallet_id || sorted_signers || threshold || nonce) + /// + /// Signers are automatically sorted before hashing, so order doesn't matter. + /// + /// Economic costs: + /// - MultisigFee: burned immediately (spam prevention) + /// - MultisigDeposit: reserved until dissolution, then returned to creator (storage bond) #[pallet::call_index(0)] - #[pallet::weight(::WeightInfo::create_multisig())] + #[pallet::weight(::WeightInfo::create_multisig(signers.len() as u32))] pub fn create_multisig( origin: OriginFor, signers: Vec, threshold: u32, + nonce: u64, ) -> DispatchResult { let creator = ensure_signed(origin)?; @@ -402,8 +434,7 @@ pub mod pallet { ensure!(threshold <= signers.len() as u32, Error::::ThresholdTooHigh); ensure!(signers.len() <= T::MaxSigners::get() as usize, Error::::TooManySigners); - // Sort signers for deterministic address generation - // (order shouldn't matter - nonce provides uniqueness) + // Sort signers for duplicate check and storage let mut sorted_signers = signers.clone(); sorted_signers.sort(); @@ -412,12 +443,10 @@ pub mod pallet { ensure!(sorted_signers[i] != sorted_signers[i - 1], Error::::DuplicateSigner); } - // Get and increment global nonce - let nonce = GlobalNonce::::get(); - GlobalNonce::::put(nonce.saturating_add(1)); - - // Generate multisig address from hash of (sorted_signers, nonce) - let multisig_address = Self::derive_multisig_address(&sorted_signers, nonce); + // Generate deterministic multisig address + // Note: derive_multisig_address() will sort internally, but we already have sorted + // for duplicate check, so we pass sorted to avoid double sorting + let multisig_address = Self::derive_multisig_address(&sorted_signers, threshold, nonce); // Ensure multisig doesn't already exist ensure!( @@ -443,22 +472,17 @@ pub mod pallet { let bounded_signers: BoundedSignersOf = sorted_signers.try_into().map_err(|_| Error::::TooManySigners)?; - // Get current block for last_activity - let current_block = frame_system::Pallet::::block_number(); - // Store multisig data Multisigs::::insert( &multisig_address, MultisigDataOf:: { + creator: creator.clone(), signers: bounded_signers.clone(), threshold, - nonce, proposal_nonce: 0, - creator: creator.clone(), deposit, - last_activity: current_block, active_proposals: 0, - proposals_per_signer: Default::default(), + proposals_per_signer: BoundedProposalsPerSignerOf::::default(), }, ); @@ -485,43 +509,75 @@ pub mod pallet { /// - A deposit (refundable - returned immediately on execution/cancellation) /// - A fee (non-refundable, burned immediately) /// - /// **Auto-cleanup:** Before creating a new proposal, ALL expired proposals are - /// automatically removed and deposits returned to original proposers. This is the primary - /// cleanup mechanism. + /// **Auto-cleanup:** Before creating a new proposal, ALL proposer's expired + /// proposals are automatically removed. This is the primary cleanup mechanism. /// /// **For threshold=1:** If the multisig threshold is 1, the proposal executes immediately. + /// + /// **Weight:** Charged upfront for worst-case (high-security path with decode). + /// Refunded to actual cost on success based on whether HS path was taken. #[pallet::call_index(1)] - #[pallet::weight(::WeightInfo::propose( - call.len() as u32, - T::MaxTotalProposalsInStorage::get() - ))] + #[pallet::weight(::WeightInfo::propose_high_security(call.len() as u32))] + #[allow(clippy::useless_conversion)] pub fn propose( origin: OriginFor, multisig_address: T::AccountId, call: Vec, expiry: BlockNumberFor, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let proposer = ensure_signed(origin)?; - // Check if proposer is a signer - let multisig_data = - Multisigs::::get(&multisig_address).ok_or(Error::::MultisigNotFound)?; - ensure!(multisig_data.signers.contains(&proposer), Error::::NotASigner); + // CRITICAL: Check call size FIRST, before any heavy operations (especially decode) + // This prevents DoS via oversized payloads that would be decoded before size validation + let call_size = call.len() as u32; + if call_size > T::MaxCallSize::get() { + return Self::err_with_weight(Error::::CallTooLarge, 0); + } - // Auto-cleanup expired proposals before creating new one - // This is the primary cleanup mechanism for active multisigs - Self::auto_cleanup_expired_proposals(&multisig_address, &proposer); + // Check if proposer is a signer (1 read: Multisigs) + let multisig_data = Multisigs::::get(&multisig_address).ok_or_else(|| { + DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(T::DbWeight::get().reads(1)), + pays_fee: Pays::Yes, + }, + error: Error::::MultisigNotFound.into(), + } + })?; + if !multisig_data.signers.contains(&proposer) { + return Self::err_with_weight(Error::::NotASigner, 1); + } + + // High-security check: if multisig is high-security, only whitelisted calls allowed + // Size already validated above, so decode is now safe + // (2 reads: Multisigs + HighSecurityAccounts) + let is_high_security = T::HighSecurity::is_high_security(&multisig_address); + if is_high_security { + let decoded_call = + ::RuntimeCall::decode(&mut &call[..]).map_err(|_| { + DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(T::DbWeight::get().reads(2)), + pays_fee: Pays::Yes, + }, + error: Error::::InvalidCall.into(), + } + })?; + if !T::HighSecurity::is_whitelisted(&decoded_call) { + return Self::err_with_weight( + Error::::CallNotAllowedForHighSecurityMultisig, + 2, + ); + } + } - // Reload multisig data after potential cleanup - let multisig_data = - Multisigs::::get(&multisig_address).ok_or(Error::::MultisigNotFound)?; let current_block = frame_system::Pallet::::block_number(); // Get signers count (used for multiple checks below) let signers_count = multisig_data.signers.len() as u32; - // Check total proposals in storage limit (Active + Executed + Cancelled) - // This incentivizes cleanup and prevents unbounded storage growth + // Check total proposals in storage limit + // Users must call claim_deposits() or remove_expired() to free space let total_proposals_in_storage = Proposals::::iter_prefix(&multisig_address).count() as u32; ensure!( @@ -530,16 +586,15 @@ pub mod pallet { ); // Check per-signer proposal limit (filibuster protection) - // Each signer can have at most (MaxTotal / NumSigners) proposals in storage - // This prevents a single signer from monopolizing the proposal queue - // Use saturating_div to handle edge cases (division by 0, etc.) and ensure at least 1 - let max_per_signer = T::MaxTotalProposalsInStorage::get() - .checked_div(signers_count) - .unwrap_or(1) // If division fails (shouldn't happen), allow at least 1 - .max(1); // Ensure minimum of 1 proposal per signer - let proposer_count = + // Each signer can have max (TotalLimit / SignersCount) proposals + let max_proposals_per_signer = + T::MaxTotalProposalsInStorage::get().saturating_div(signers_count); + let proposer_current_count = multisig_data.proposals_per_signer.get(&proposer).copied().unwrap_or(0); - ensure!(proposer_count < max_per_signer, Error::::TooManyProposalsPerSigner); + ensure!( + proposer_current_count < max_proposals_per_signer, + Error::::TooManyProposalsPerSigner + ); // Check call size ensure!(call.len() as u32 <= T::MaxCallSize::get(), Error::::CallTooLarge); @@ -576,14 +631,7 @@ pub mod pallet { T::Currency::reserve(&proposer, deposit) .map_err(|_| Error::::InsufficientBalance)?; - // Update multisig last_activity - Multisigs::::mutate(&multisig_address, |maybe_multisig| { - if let Some(multisig) = maybe_multisig { - multisig.last_activity = current_block; - } - }); - - // Convert to bounded vec + // Convert to bounded vec (call_size already computed and validated above) let bounded_call: BoundedCallOf = call.try_into().map_err(|_| Error::::CallTooLarge)?; @@ -614,17 +662,14 @@ pub mod pallet { // Store proposal with nonce as key (simple and efficient) Proposals::::insert(&multisig_address, proposal_id, proposal); - // Increment active proposals counter and per-signer counter - Multisigs::::mutate(&multisig_address, |maybe_multisig| { - if let Some(multisig) = maybe_multisig { - multisig.active_proposals = multisig.active_proposals.saturating_add(1); - - // Update per-signer counter for filibuster protection - let current_count = - multisig.proposals_per_signer.get(&proposer).copied().unwrap_or(0); - let _ = multisig + // Increment proposal counters + Multisigs::::mutate(&multisig_address, |maybe_data| { + if let Some(ref mut data) = maybe_data { + data.active_proposals = data.active_proposals.saturating_add(1); + let count = data.proposals_per_signer.get(&proposer).copied().unwrap_or(0); + let _ = data .proposals_per_signer - .try_insert(proposer.clone(), current_count.saturating_add(1)); + .try_insert(proposer.clone(), count.saturating_add(1)); } }); @@ -638,34 +683,40 @@ pub mod pallet { // Check if threshold is reached immediately (threshold=1 case) // Proposer is already counted as first approval if 1 >= multisig_data.threshold { - // Threshold reached - execute immediately - // Need to get proposal again since we inserted it - let proposal = Proposals::::get(&multisig_address, proposal_id) - .ok_or(Error::::ProposalNotFound)?; - Self::do_execute(multisig_address, proposal_id, proposal)?; + Proposals::::mutate(&multisig_address, proposal_id, |maybe_proposal| { + if let Some(ref mut p) = maybe_proposal { + p.status = ProposalStatus::Approved; + } + }); + Self::deposit_event(Event::ProposalReadyToExecute { + multisig_address: multisig_address.clone(), + proposal_id, + approvals_count: 1, + }); } - Ok(()) + // Refund weight: HS path was charged upfront, refund if non-HS + let actual_weight = if is_high_security { + ::WeightInfo::propose_high_security(call_size) + } else { + ::WeightInfo::propose(call_size) + }; + + Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes }) } /// Approve a proposed transaction /// /// If this approval brings the total approvals to or above the threshold, - /// the transaction will be automatically executed. - /// - /// **Auto-cleanup:** Before processing the approval, ALL expired proposals are - /// automatically removed and deposits returned to original proposers. + /// the proposal status changes to `Approved` and can be executed via `execute()`. /// /// Parameters: /// - `multisig_address`: The multisig account /// - `proposal_id`: ID (nonce) of the proposal to approve /// - /// Weight: Charges for MAX call size and MAX expired proposals, refunds based on actual + /// Weight: Charges for MAX call size, refunds based on actual #[pallet::call_index(2)] - #[pallet::weight(::WeightInfo::approve( - T::MaxCallSize::get(), - T::MaxTotalProposalsInStorage::get() - ))] + #[pallet::weight(::WeightInfo::approve(T::MaxCallSize::get()))] #[allow(clippy::useless_conversion)] pub fn approve( origin: OriginFor, @@ -674,29 +725,46 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let approver = ensure_signed(origin)?; - // Check if approver is a signer - let multisig_data = Self::ensure_is_signer(&multisig_address, &approver)?; - - // Auto-cleanup expired proposals on any multisig activity - // Returns count of proposals in storage (which determines iteration cost) - let iterated_count = Self::auto_cleanup_expired_proposals(&multisig_address, &approver); + // Check if approver is a signer (1 read: Multisigs) + let multisig_data = Multisigs::::get(&multisig_address).ok_or_else(|| { + DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(T::DbWeight::get().reads(1)), + pays_fee: Pays::Yes, + }, + error: Error::::MultisigNotFound.into(), + } + })?; + if !multisig_data.signers.contains(&approver) { + return Self::err_with_weight(Error::::NotASigner, 1); + } - // Get proposal - let mut proposal = Proposals::::get(&multisig_address, proposal_id) - .ok_or(Error::::ProposalNotFound)?; + // Get proposal (2 reads: Multisigs + Proposals) + let mut proposal = + Proposals::::get(&multisig_address, proposal_id).ok_or_else(|| { + DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(T::DbWeight::get().reads(2)), + pays_fee: Pays::Yes, + }, + error: Error::::ProposalNotFound.into(), + } + })?; - // Calculate actual weight based on real call size and actual storage size - // We charge for worst-case (e=Max), but refund based on actual storage size + // Calculate actual weight based on real call size let actual_call_size = proposal.call.len() as u32; - let actual_weight = - ::WeightInfo::approve(actual_call_size, iterated_count); + let actual_weight = ::WeightInfo::approve(actual_call_size); - // Check if not expired + // Check if not expired (2 reads already performed) let current_block = frame_system::Pallet::::block_number(); - ensure!(current_block <= proposal.expiry, Error::::ProposalExpired); + if current_block > proposal.expiry { + return Self::err_with_weight(Error::::ProposalExpired, 2); + } - // Check if already approved - ensure!(!proposal.approvals.contains(&approver), Error::::AlreadyApproved); + // Check if already approved (2 reads already performed) + if proposal.approvals.contains(&approver) { + return Self::err_with_weight(Error::::AlreadyApproved, 2); + } // Add approval proposal @@ -706,6 +774,14 @@ pub mod pallet { let approvals_count = proposal.approvals.len() as u32; + // Check if threshold is reached - if so, mark as Approved + if approvals_count >= multisig_data.threshold { + proposal.status = ProposalStatus::Approved; + } + + // Save proposal + Proposals::::insert(&multisig_address, proposal_id, &proposal); + // Emit approval event Self::deposit_event(Event::ProposalApproved { multisig_address: multisig_address.clone(), @@ -714,19 +790,12 @@ pub mod pallet { approvals_count, }); - // Check if threshold is reached - if so, execute immediately - if approvals_count >= multisig_data.threshold { - // Execute the transaction - Self::do_execute(multisig_address, proposal_id, proposal)?; - } else { - // Not ready yet, just save the proposal - Proposals::::insert(&multisig_address, proposal_id, proposal); - - // Update multisig last_activity - Multisigs::::mutate(&multisig_address, |maybe_multisig| { - if let Some(multisig) = maybe_multisig { - multisig.last_activity = frame_system::Pallet::::block_number(); - } + // Emit ready-to-execute event if threshold just reached + if proposal.status == ProposalStatus::Approved { + Self::deposit_event(Event::ProposalReadyToExecute { + multisig_address, + proposal_id, + approvals_count, }); } @@ -736,19 +805,11 @@ pub mod pallet { /// Cancel a proposed transaction (only by proposer) /// - /// **Auto-cleanup:** Before processing the cancellation, ALL expired proposals are - /// automatically removed and deposits returned to original proposers. - /// /// Parameters: /// - `multisig_address`: The multisig account /// - `proposal_id`: ID (nonce) of the proposal to cancel - /// - /// Weight: Charges for MAX call size and MAX expired proposals, refunds based on actual #[pallet::call_index(3)] - #[pallet::weight(::WeightInfo::cancel( - T::MaxCallSize::get(), - T::MaxTotalProposalsInStorage::get() - ))] + #[pallet::weight(::WeightInfo::cancel(T::MaxCallSize::get()))] #[allow(clippy::useless_conversion)] pub fn cancel( origin: OriginFor, @@ -757,25 +818,31 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let canceller = ensure_signed(origin)?; - // Auto-cleanup expired proposals on any multisig activity - // Returns count of proposals in storage (which determines iteration cost) - let iterated_count = - Self::auto_cleanup_expired_proposals(&multisig_address, &canceller); - - // Get proposal - let proposal = Proposals::::get(&multisig_address, proposal_id) - .ok_or(Error::::ProposalNotFound)?; + // Get proposal (1 read: Proposals) + let proposal = + Proposals::::get(&multisig_address, proposal_id).ok_or_else(|| { + DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(T::DbWeight::get().reads(1)), + pays_fee: Pays::Yes, + }, + error: Error::::ProposalNotFound.into(), + } + })?; - // Calculate actual weight based on real call size and actual storage size - // We charge for worst-case (e=Max), but refund based on actual storage size - let actual_call_size = proposal.call.len() as u32; - let actual_weight = ::WeightInfo::cancel(actual_call_size, iterated_count); + // Check if caller is the proposer (1 read already performed) + if canceller != proposal.proposer { + return Self::err_with_weight(Error::::NotProposer, 1); + } - // Check if caller is the proposer - ensure!(canceller == proposal.proposer, Error::::NotProposer); + // Check if proposal is cancellable (Active or Approved) + if proposal.status != ProposalStatus::Active && + proposal.status != ProposalStatus::Approved + { + return Self::err_with_weight(Error::::ProposalNotActive, 1); + } - // Check if proposal is still active - ensure!(proposal.status == ProposalStatus::Active, Error::::ProposalNotActive); + let call_size = proposal.call.len() as u32; // Remove proposal from storage and return deposit immediately Self::remove_proposal_and_return_deposit( @@ -792,7 +859,7 @@ pub mod pallet { proposal_id, }); - // Return actual weight (refund overpayment) + let actual_weight = ::WeightInfo::cancel(call_size); Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes }) } @@ -805,7 +872,7 @@ pub mod pallet { /// The deposit is always returned to the original proposer, not the caller. /// This allows any signer to help clean up storage even if proposer is inactive. #[pallet::call_index(4)] - #[pallet::weight(::WeightInfo::remove_expired())] + #[pallet::weight(::WeightInfo::remove_expired(T::MaxCallSize::get()))] pub fn remove_expired( origin: OriginFor, multisig_address: T::AccountId, @@ -859,96 +926,149 @@ pub mod pallet { /// Returns all proposal deposits to the proposer in a single transaction. #[pallet::call_index(5)] #[pallet::weight(::WeightInfo::claim_deposits( - T::MaxTotalProposalsInStorage::get() - ))] + T::MaxTotalProposalsInStorage::get(), // Worst-case iterated + T::MaxTotalProposalsInStorage::get().saturating_div(2), // Worst-case cleaned + T::MaxCallSize::get() // Worst-case avg call size + ))] + #[allow(clippy::useless_conversion)] pub fn claim_deposits( origin: OriginFor, multisig_address: T::AccountId, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let caller = ensure_signed(origin)?; - let current_block = frame_system::Pallet::::block_number(); - - let mut total_returned = BalanceOf::::zero(); - let mut removed_count = 0u32; - - // Iterate through all proposals for this multisig - // Only Active+Expired proposals exist (Executed/Cancelled are auto-removed) - let proposals_to_remove: Vec<(u32, ProposalDataOf)> = - Proposals::::iter_prefix(&multisig_address) - .filter(|(_, proposal)| { - // Only proposals where caller is proposer - if proposal.proposer != caller { - return false; - } - - // Only Active proposals can exist (Executed/Cancelled auto-removed) - // Must be expired to remove - proposal.status == ProposalStatus::Active && current_block > proposal.expiry - }) - .collect(); - - // Remove proposals and return deposits - for (id, proposal) in proposals_to_remove { - total_returned = total_returned.saturating_add(proposal.deposit); - removed_count = removed_count.saturating_add(1); + // Cleanup ALL caller's expired proposals + // Returns: (cleaned_count, total_proposals_iterated, total_call_bytes) + let (cleaned, total_proposals_iterated, total_call_bytes) = + Self::cleanup_proposer_expired(&multisig_address, &caller, &caller); - // Remove from storage and return deposit - Self::remove_proposal_and_return_deposit( - &multisig_address, - id, - &proposal.proposer, - proposal.deposit, - ); - - // Emit event for each removed proposal - Self::deposit_event(Event::ProposalRemoved { - multisig_address: multisig_address.clone(), - proposal_id: id, - proposer: caller.clone(), - removed_by: caller.clone(), - }); - } + let deposit_per_proposal = T::ProposalDeposit::get(); + let total_returned = deposit_per_proposal.saturating_mul(cleaned.into()); // Emit summary event Self::deposit_event(Event::DepositsClaimed { multisig_address: multisig_address.clone(), claimer: caller, total_returned, - proposals_removed: removed_count, - multisig_removed: false, // Multisig is never auto-removed now + proposals_removed: cleaned, + multisig_removed: false, }); - Ok(()) + // Average call size over iterated proposals (for weight) + let avg_call_size = if total_proposals_iterated > 0 { + total_call_bytes / total_proposals_iterated + } else { + 0 + }; + + let actual_weight = ::WeightInfo::claim_deposits( + total_proposals_iterated, + cleaned, + avg_call_size, + ); + Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes }) } - /// Dissolve (remove) a multisig and recover the creation deposit. + /// Execute an approved proposal + /// + /// Can be called by any signer of the multisig once the proposal has reached + /// the approval threshold (status = Approved). The proposal must not be expired. + /// + /// On execution: + /// - The call is decoded and dispatched as the multisig account + /// - Proposal is removed from storage + /// - Deposit is returned to the proposer + /// + /// Parameters: + /// - `multisig_address`: The multisig account + /// - `proposal_id`: ID (nonce) of the proposal to execute + #[pallet::call_index(7)] + #[pallet::weight(::WeightInfo::execute(T::MaxCallSize::get()))] + #[allow(clippy::useless_conversion)] + pub fn execute( + origin: OriginFor, + multisig_address: T::AccountId, + proposal_id: u32, + ) -> DispatchResultWithPostInfo { + let executor = ensure_signed(origin)?; + + // Check if executor is a signer (1 read: Multisigs) + let multisig_data = Multisigs::::get(&multisig_address).ok_or_else(|| { + DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(T::DbWeight::get().reads(1)), + pays_fee: Pays::Yes, + }, + error: Error::::MultisigNotFound.into(), + } + })?; + if !multisig_data.signers.contains(&executor) { + return Self::err_with_weight(Error::::NotASigner, 1); + } + + // Get proposal (2 reads: Multisigs + Proposals) + let proposal = + Proposals::::get(&multisig_address, proposal_id).ok_or_else(|| { + DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(T::DbWeight::get().reads(2)), + pays_fee: Pays::Yes, + }, + error: Error::::ProposalNotFound.into(), + } + })?; + + // Must be Approved status + if proposal.status != ProposalStatus::Approved { + return Self::err_with_weight(Error::::ProposalNotApproved, 2); + } + + // Must not be expired + let current_block = frame_system::Pallet::::block_number(); + if current_block > proposal.expiry { + return Self::err_with_weight(Error::::ProposalExpired, 2); + } + + // Calculate actual weight based on real call size + let actual_call_size = proposal.call.len() as u32; + let actual_weight = ::WeightInfo::execute(actual_call_size); + + // Execute the proposal + Self::do_execute(multisig_address, proposal_id, proposal)?; + + Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes }) + } + + /// Approve dissolving a multisig account + /// + /// Signers call this to approve dissolving the multisig. + /// When threshold is reached, the multisig is automatically dissolved. /// /// Requirements: - /// - No proposals exist (active, executed, or cancelled) - must be fully cleaned up. - /// - Multisig account balance must be zero. - /// - Can be called by the creator OR any signer. + /// - Caller must be a signer + /// - No proposals exist (active, executed, or cancelled) - must be fully cleaned up + /// - Multisig account balance must be zero /// - /// The deposit is ALWAYS returned to the original `creator` stored in `MultisigData`. + /// When threshold is reached: + /// - Deposit is returned to creator + /// - Multisig storage is removed #[pallet::call_index(6)] - #[pallet::weight(::WeightInfo::dissolve_multisig())] - pub fn dissolve_multisig( + #[pallet::weight(::WeightInfo::approve_dissolve_threshold_reached())] + #[allow(clippy::useless_conversion)] + pub fn approve_dissolve( origin: OriginFor, multisig_address: T::AccountId, - ) -> DispatchResult { - let caller = ensure_signed(origin)?; + ) -> DispatchResultWithPostInfo { + let approver = ensure_signed(origin)?; // 1. Get multisig data let multisig_data = Multisigs::::get(&multisig_address).ok_or(Error::::MultisigNotFound)?; - // 2. Check permissions: Creator OR Any Signer - let is_signer = multisig_data.signers.contains(&caller); - let is_creator = multisig_data.creator == caller; - ensure!(is_signer || is_creator, Error::::NotASigner); + // 2. Check permissions: Must be a signer + ensure!(multisig_data.signers.contains(&approver), Error::::NotASigner); // 3. Check if account is clean (no proposals at all) - // iter_prefix is efficient enough here as we just need to check if ANY exist if Proposals::::iter_prefix(&multisig_address).next().is_some() { return Err(Error::::ProposalsExist.into()); } @@ -957,27 +1077,87 @@ pub mod pallet { let balance = T::Currency::total_balance(&multisig_address); ensure!(balance.is_zero(), Error::::MultisigAccountNotZero); - // 5. Return deposit to creator - T::Currency::unreserve(&multisig_data.creator, multisig_data.deposit); + // 5. Get or create approval list + let mut approvals = DissolveApprovals::::get(&multisig_address).unwrap_or_default(); - // 6. Remove multisig from storage - Multisigs::::remove(&multisig_address); + // 6. Check if already approved + ensure!(!approvals.contains(&approver), Error::::AlreadyApproved); - // 7. Emit event - Self::deposit_event(Event::MultisigDissolved { - multisig_address, - caller, - deposit_returned: multisig_data.deposit, + // 7. Add approval + approvals.try_push(approver.clone()).map_err(|_| Error::::TooManySigners)?; + + let approvals_count = approvals.len() as u32; + + // 8. Emit approval event + Self::deposit_event(Event::DissolveApproved { + multisig_address: multisig_address.clone(), + approver: approver.clone(), + approvals_count, }); - Ok(()) + // 9. Check if threshold reached + let threshold_reached = approvals_count >= multisig_data.threshold; + if threshold_reached { + // Threshold reached - dissolve multisig + let deposit = multisig_data.deposit; + let creator = multisig_data.creator.clone(); + + // Remove multisig from storage + Multisigs::::remove(&multisig_address); + DissolveApprovals::::remove(&multisig_address); + + // Return deposit to creator + T::Currency::unreserve(&creator, deposit); + + // Emit dissolved event + Self::deposit_event(Event::MultisigDissolved { + multisig_address, + deposit_returned: creator, + approvers: approvals.to_vec(), + }); + } else { + // Not ready yet, save approvals + DissolveApprovals::::insert(&multisig_address, approvals); + } + + let actual_weight = if threshold_reached { + ::WeightInfo::approve_dissolve_threshold_reached() + } else { + ::WeightInfo::approve_dissolve() + }; + Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes }) } } impl Pallet { - /// Derive a multisig address from signers and nonce - pub fn derive_multisig_address(signers: &[T::AccountId], nonce: u64) -> T::AccountId { - // Create a unique identifier from pallet id + signers + nonce. + /// Return an error with actual weight consumed instead of charging full upfront weight. + /// Use for early exits where minimal work was performed. + fn err_with_weight(error: Error, reads: u64) -> DispatchResultWithPostInfo { + Err(DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(T::DbWeight::get().reads(reads)), + pays_fee: Pays::Yes, + }, + error: error.into(), + }) + } + + /// Derive a deterministic multisig address from signers, threshold, and nonce + /// + /// The address is computed as: hash(pallet_id || sorted_signers || threshold || nonce) + /// Signers are automatically sorted internally for deterministic results. + /// This allows users to pre-compute the address before creating the multisig. + pub fn derive_multisig_address( + signers: &[T::AccountId], + threshold: u32, + nonce: u64, + ) -> T::AccountId { + // Sort signers for deterministic address generation + // User doesn't need to worry about order + let mut sorted_signers = signers.to_vec(); + sorted_signers.sort(); + + // Create a unique identifier from pallet id + sorted signers + threshold + nonce. // // IMPORTANT: // - Do NOT `Decode` directly from a finite byte-slice and then "fallback" to a constant @@ -987,7 +1167,8 @@ pub mod pallet { let pallet_id = T::PalletId::get(); let mut data = Vec::new(); data.extend_from_slice(&pallet_id.0); - data.extend_from_slice(&signers.encode()); + data.extend_from_slice(&sorted_signers.encode()); + data.extend_from_slice(&threshold.encode()); data.extend_from_slice(&nonce.encode()); // Hash the data and map it deterministically into an AccountId. @@ -1017,64 +1198,66 @@ pub mod pallet { Ok(multisig_data) } - /// Auto-cleanup expired proposals at the start of any multisig activity - /// This is the primary cleanup mechanism for active multisigs - /// Returns deposits to original proposers and emits cleanup events - fn auto_cleanup_expired_proposals( + /// Cleanup ALL expired proposals for a specific proposer + /// + /// Iterates through all proposals in the multisig and removes expired ones + /// belonging to the specified proposer. + /// + /// Returns: (cleaned_count, total_proposals_iterated) + /// - cleaned_count: number of proposals actually removed + /// - total_proposals_iterated: total proposals that existed before cleanup (for weight + /// calculation) + /// Returns: (cleaned_count, total_proposals_iterated, total_call_bytes) + /// - total_call_bytes: sum of proposal.call.len() over iterated proposals (for weight) + fn cleanup_proposer_expired( multisig_address: &T::AccountId, + proposer: &T::AccountId, caller: &T::AccountId, - ) -> u32 { + ) -> (u32, u32, u32) { let current_block = frame_system::Pallet::::block_number(); - let mut iterated_count = 0u32; - let mut expired_proposals: Vec<(u32, T::AccountId, BalanceOf)> = Vec::new(); - - // Iterate through all proposals to count them AND identify expired ones - for (id, proposal) in Proposals::::iter_prefix(multisig_address) { - iterated_count += 1; - if proposal.status == ProposalStatus::Active && current_block > proposal.expiry { - expired_proposals.push((id, proposal.proposer, proposal.deposit)); - } - } + let mut total_iterated = 0u32; + let mut total_call_bytes = 0u32; + + // Collect expired proposals to remove + // IMPORTANT: We count ALL proposals during iteration (for weight calculation) + let expired_proposals: Vec<(u32, T::AccountId, BalanceOf)> = + Proposals::::iter_prefix(multisig_address) + .filter_map(|(proposal_id, proposal)| { + total_iterated += 1; // Count every proposal we iterate through + total_call_bytes += proposal.call.len() as u32; + + // Only proposer's expired active proposals + if proposal.proposer == *proposer && + proposal.status == ProposalStatus::Active && + current_block > proposal.expiry + { + Some((proposal_id, proposal.proposer, proposal.deposit)) + } else { + None + } + }) + .collect(); + + let cleaned = expired_proposals.len() as u32; - // Remove expired proposals and return deposits - for (id, expired_proposer, deposit) in expired_proposals.iter() { + // Remove proposals and emit events + for (proposal_id, expired_proposer, deposit) in expired_proposals { Self::remove_proposal_and_return_deposit( multisig_address, - *id, - expired_proposer, - *deposit, + proposal_id, + &expired_proposer, + deposit, ); - // Emit event for each removed proposal Self::deposit_event(Event::ProposalRemoved { multisig_address: multisig_address.clone(), - proposal_id: *id, - proposer: expired_proposer.clone(), + proposal_id, + proposer: expired_proposer, removed_by: caller.clone(), }); } - // Return total number of proposals iterated (not cleaned) - // This reflects the actual storage read cost - iterated_count - } - - /// Decrement proposal counters (active_proposals and per-signer counter) - /// Used when removing proposals from storage - fn decrement_proposal_counters(multisig_address: &T::AccountId, proposer: &T::AccountId) { - Multisigs::::mutate(multisig_address, |maybe_multisig| { - if let Some(multisig) = maybe_multisig { - multisig.active_proposals = multisig.active_proposals.saturating_sub(1); - - // Decrement per-signer counter - if let Some(count) = multisig.proposals_per_signer.get_mut(proposer) { - *count = count.saturating_sub(1); - if *count == 0 { - multisig.proposals_per_signer.remove(proposer); - } - } - } - }); + (cleaned, total_iterated, total_call_bytes) } /// Remove a proposal from storage and return deposit to proposer @@ -1088,11 +1271,22 @@ pub mod pallet { // Remove from storage Proposals::::remove(multisig_address, proposal_id); + // Decrement proposal counters + Multisigs::::mutate(multisig_address, |maybe_data| { + if let Some(ref mut data) = maybe_data { + data.active_proposals = data.active_proposals.saturating_sub(1); + if let Some(count) = data.proposals_per_signer.get_mut(proposer) { + *count = count.saturating_sub(1); + // Remove entry if count reaches 0 to save storage + if *count == 0 { + data.proposals_per_signer.remove(proposer); + } + } + } + }); + // Return deposit to proposer T::Currency::unreserve(proposer, deposit); - - // Decrement counters - Self::decrement_proposal_counters(multisig_address, proposer); } /// Internal function to execute a proposal @@ -1122,13 +1316,6 @@ pub mod pallet { proposal.deposit, ); - // EFFECTS: Update multisig last_activity BEFORE external interaction - Multisigs::::mutate(&multisig_address, |maybe_multisig| { - if let Some(multisig) = maybe_multisig { - multisig.last_activity = frame_system::Pallet::::block_number(); - } - }); - // INTERACTIONS: NOW execute the call as the multisig account // Proposal already removed, so reentrancy cannot affect storage let result = diff --git a/pallets/multisig/src/migration.rs b/pallets/multisig/src/migration.rs new file mode 100644 index 00000000..e69de29b diff --git a/pallets/multisig/src/mock.rs b/pallets/multisig/src/mock.rs index 38f241d6..514925b4 100644 --- a/pallets/multisig/src/mock.rs +++ b/pallets/multisig/src/mock.rs @@ -1,89 +1,121 @@ -//! Mock runtime for testing pallet-multisig +//! Mock runtime for testing pallet-multisig. +//! Single mock used for both unit tests and benchmark tests; implements +//! `pallet_reversible_transfers::Config` so that benchmark test suite compiles and runs. + +use core::{cell::RefCell, marker::PhantomData}; use crate as pallet_multisig; use frame_support::{ - parameter_types, - traits::{ConstU32, Everything}, + derive_impl, ord_parameter_types, parameter_types, + traits::{ConstU32, EitherOfDiverse, EqualPrivilegeOnly, Time}, PalletId, }; -use sp_core::{crypto::AccountId32, H256}; -use sp_runtime::{ - traits::{BlakeTwo256, IdentityLookup}, - BuildStorage, Permill, -}; +use frame_system::{limits::BlockWeights, EnsureRoot, EnsureSignedBy}; +use qp_scheduler::BlockNumberOrTimestamp; +use sp_core::ConstU128; +use sp_runtime::{BuildStorage, Perbill, Permill, Weight}; type Block = frame_system::mocking::MockBlock; -type Balance = u128; - -// Configure a mock runtime to test the pallet. -frame_support::construct_runtime!( - pub enum Test - { - System: frame_system, - Balances: pallet_balances, - Multisig: pallet_multisig, +pub type Balance = u128; +pub type AccountId = sp_core::crypto::AccountId32; + +// account_id from u64 (first 8 bytes = id.to_le_bytes()) — same as in tests +pub fn account_id(id: u64) -> AccountId { + let mut data = [0u8; 32]; + data[0..8].copy_from_slice(&id.to_le_bytes()); + AccountId::new(data) +} + +#[frame_support::runtime] +mod runtime { + use super::*; + + #[runtime::runtime] + #[runtime::derive( + RuntimeCall, + RuntimeEvent, + RuntimeError, + RuntimeOrigin, + RuntimeFreezeReason, + RuntimeHoldReason, + RuntimeSlashReason, + RuntimeLockId, + RuntimeTask + )] + pub struct Test; + + #[runtime::pallet_index(0)] + pub type System = frame_system::Pallet; + + #[runtime::pallet_index(1)] + pub type Balances = pallet_balances::Pallet; + + #[runtime::pallet_index(2)] + pub type Multisig = pallet_multisig::Pallet; + + #[runtime::pallet_index(3)] + pub type Preimage = pallet_preimage::Pallet; + + #[runtime::pallet_index(4)] + pub type Scheduler = pallet_scheduler::Pallet; + + #[runtime::pallet_index(5)] + pub type Recovery = pallet_recovery::Pallet; + + #[runtime::pallet_index(6)] + pub type Utility = pallet_utility::Pallet; + + #[runtime::pallet_index(7)] + pub type Assets = pallet_assets::Pallet; + + #[runtime::pallet_index(8)] + pub type AssetsHolder = pallet_assets_holder::Pallet; + + #[runtime::pallet_index(9)] + pub type ReversibleTransfers = pallet_reversible_transfers::Pallet; +} + +impl TryFrom for pallet_balances::Call { + type Error = (); + fn try_from(call: RuntimeCall) -> Result { + match call { + RuntimeCall::Balances(c) => Ok(c), + _ => Err(()), + } } -); +} -parameter_types! { - pub const BlockHashCount: u64 = 250; +impl TryFrom for pallet_assets::Call { + type Error = (); + fn try_from(call: RuntimeCall) -> Result { + match call { + RuntimeCall::Assets(c) => Ok(c), + _ => Err(()), + } + } } +#[derive_impl(frame_system::config_preludes::TestDefaultConfig)] impl frame_system::Config for Test { - type RuntimeEvent = RuntimeEvent; - type BaseCallFilter = Everything; type Block = Block; - type BlockWeights = (); - type BlockLength = (); - type DbWeight = (); - type RuntimeOrigin = RuntimeOrigin; - type RuntimeCall = RuntimeCall; - type Nonce = u64; - type Hash = H256; - type Hashing = BlakeTwo256; - type AccountId = AccountId32; - type Lookup = IdentityLookup; - type BlockHashCount = BlockHashCount; - type Version = (); - type PalletInfo = PalletInfo; + type AccountId = AccountId; + type Lookup = sp_runtime::traits::IdentityLookup; type AccountData = pallet_balances::AccountData; - type OnNewAccount = (); - type OnKilledAccount = (); - type SystemWeightInfo = (); - type SS58Prefix = (); - type OnSetCode = (); - type MaxConsumers = ConstU32<16>; - type RuntimeTask = (); - type SingleBlockMigrations = (); - type MultiBlockMigrator = (); - type PreInherents = (); - type PostInherents = (); - type PostTransactions = (); - type ExtensionsWeightInfo = (); } parameter_types! { - pub const ExistentialDeposit: Balance = 1; - pub const MaxLocks: u32 = 50; - pub const MaxReserves: u32 = 50; - pub const MaxFreezes: u32 = 50; - pub const MintingAccount: AccountId32 = AccountId32::new([99u8; 32]); + pub MintingAccount: AccountId = AccountId::new([1u8; 32]); } +#[derive_impl(pallet_balances::config_preludes::TestDefaultConfig)] impl pallet_balances::Config for Test { - type WeightInfo = (); type Balance = Balance; type DustRemoval = (); - type ExistentialDeposit = ExistentialDeposit; - type AccountStore = System; - type MaxLocks = MaxLocks; - type MaxReserves = MaxReserves; - type ReserveIdentifier = [u8; 8]; + type ExistentialDeposit = ConstU128<1>; + type AccountStore = frame_system::Pallet; + type WeightInfo = (); type RuntimeHoldReason = RuntimeHoldReason; - type RuntimeFreezeReason = RuntimeFreezeReason; - type FreezeIdentifier = (); - type MaxFreezes = MaxFreezes; - type DoneSlashHandler = (); + type MaxFreezes = MaxReversibleTransfers; type MintingAccount = MintingAccount; } @@ -92,12 +124,12 @@ parameter_types! { pub const MaxSignersParam: u32 = 10; pub const MaxTotalProposalsInStorageParam: u32 = 20; pub const MaxCallSizeParam: u32 = 1024; - pub const MultisigFeeParam: Balance = 1000; // Non-refundable fee - pub const MultisigDepositParam: Balance = 500; // Refundable deposit + pub const MultisigFeeParam: Balance = 1000; + pub const MultisigDepositParam: Balance = 500; pub const ProposalDepositParam: Balance = 100; - pub const ProposalFeeParam: Balance = 1000; // Non-refundable fee - pub const SignerStepFactorParam: Permill = Permill::from_parts(10_000); // 1% - pub const MaxExpiryDurationParam: u64 = 10000; // 10000 blocks for testing (enough for all test scenarios) + pub const ProposalFeeParam: Balance = 1000; + pub const SignerStepFactorParam: Permill = Permill::from_parts(10_000); + pub const MaxExpiryDurationParam: u64 = 10000; } impl pallet_multisig::Config for Test { @@ -114,30 +146,177 @@ impl pallet_multisig::Config for Test { type MaxExpiryDuration = MaxExpiryDurationParam; type PalletId = MultisigPalletId; type WeightInfo = (); + type HighSecurity = crate::tests::MockHighSecurity; } -// Helper to create AccountId32 from u64 -pub fn account_id(id: u64) -> AccountId32 { - let mut data = [0u8; 32]; - data[0..8].copy_from_slice(&id.to_le_bytes()); - AccountId32::new(data) +type Moment = u64; + +thread_local! { + static MOCKED_TIME: RefCell = const { RefCell::new(69420) }; +} + +pub struct MockTimestamp(PhantomData); + +impl MockTimestamp +where + T::Moment: From, +{ + pub fn set_timestamp(now: Moment) { + MOCKED_TIME.with(|v| *v.borrow_mut() = now); + } +} + +impl Time for MockTimestamp { + type Moment = Moment; + fn now() -> Self::Moment { + MOCKED_TIME.with(|v| *v.borrow()) + } +} + +parameter_types! { + pub const ReversibleTransfersPalletIdValue: PalletId = PalletId(*b"rtpallet"); + pub const DefaultDelay: BlockNumberOrTimestamp = + BlockNumberOrTimestamp::BlockNumber(10); + pub const MinDelayPeriodBlocks: u64 = 2; + pub const MinDelayPeriodMoment: u64 = 2000; + pub const MaxReversibleTransfers: u32 = 100; + pub const MaxInterceptorAccounts: u32 = 10; + pub const HighSecurityVolumeFee: Permill = Permill::from_percent(1); +} + +impl pallet_reversible_transfers::Config for Test { + type SchedulerOrigin = OriginCaller; + type RuntimeHoldReason = RuntimeHoldReason; + type Scheduler = Scheduler; + type BlockNumberProvider = System; + type MaxPendingPerAccount = MaxReversibleTransfers; + type DefaultDelay = DefaultDelay; + type MinDelayPeriodBlocks = MinDelayPeriodBlocks; + type MinDelayPeriodMoment = MinDelayPeriodMoment; + type PalletId = ReversibleTransfersPalletIdValue; + type Preimages = Preimage; + type WeightInfo = (); + type Moment = Moment; + type TimeProvider = MockTimestamp; + type MaxInterceptorAccounts = MaxInterceptorAccounts; + type VolumeFee = HighSecurityVolumeFee; +} + +parameter_types! { + pub const AssetDeposit: Balance = 0; + pub const AssetAccountDeposit: Balance = 0; + pub const AssetsStringLimit: u32 = 50; + pub const MetadataDepositBase: Balance = 0; + pub const MetadataDepositPerByte: Balance = 0; +} + +impl pallet_assets::Config for Test { + type Balance = Balance; + type RuntimeEvent = RuntimeEvent; + type AssetId = u32; + type AssetIdParameter = codec::Compact; + type Currency = Balances; + type CreateOrigin = + frame_support::traits::AsEnsureOriginWithArg>; + type ForceOrigin = frame_system::EnsureRoot; + type AssetDeposit = AssetDeposit; + type MetadataDepositBase = MetadataDepositBase; + type MetadataDepositPerByte = MetadataDepositPerByte; + type ApprovalDeposit = sp_core::ConstU128<0>; + type StringLimit = AssetsStringLimit; + type Freezer = (); + type Extra = (); + type WeightInfo = (); + type CallbackHandle = pallet_assets::AutoIncAssetId; + type AssetAccountDeposit = AssetAccountDeposit; + type RemoveItemsLimit = frame_support::traits::ConstU32<1000>; + type Holder = pallet_assets_holder::Pallet; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); +} + +impl pallet_assets_holder::Config for Test { + type RuntimeEvent = RuntimeEvent; + type RuntimeHoldReason = RuntimeHoldReason; +} + +parameter_types! { + pub const ConfigDepositBase: Balance = 1; + pub const FriendDepositFactor: Balance = 1; + pub const MaxFriends: u32 = 9; + pub const RecoveryDeposit: Balance = 1; +} + +impl pallet_recovery::Config for Test { + type WeightInfo = (); + type RuntimeCall = RuntimeCall; + type RuntimeEvent = RuntimeEvent; + type Currency = Balances; + type ConfigDepositBase = ConfigDepositBase; + type FriendDepositFactor = FriendDepositFactor; + type MaxFriends = MaxFriends; + type RecoveryDeposit = RecoveryDeposit; + type BlockNumberProvider = System; +} + +impl pallet_preimage::Config for Test { + type WeightInfo = (); + type Currency = (); + type ManagerOrigin = EnsureRoot; + type Consideration = (); + type RuntimeEvent = RuntimeEvent; +} + +parameter_types! { + pub storage MaximumSchedulerWeight: Weight = + Perbill::from_percent(80) * BlockWeights::default().max_block; + pub const TimestampBucketSize: u64 = 1000; +} + +ord_parameter_types! { + pub const One: AccountId = AccountId::new([1u8; 32]); +} + +impl pallet_scheduler::Config for Test { + type RuntimeOrigin = RuntimeOrigin; + type PalletsOrigin = OriginCaller; + type RuntimeCall = RuntimeCall; + type MaximumWeight = MaximumSchedulerWeight; + type ScheduleOrigin = EitherOfDiverse, EnsureSignedBy>; + type OriginPrivilegeCmp = EqualPrivilegeOnly; + type MaxScheduledPerBlock = ConstU32<10>; + type WeightInfo = (); + type Preimages = Preimage; + type Moment = Moment; + type TimeProvider = MockTimestamp; + type TimestampBucketSize = TimestampBucketSize; +} + +impl pallet_utility::Config for Test { + type RuntimeEvent = RuntimeEvent; + type RuntimeCall = RuntimeCall; + type PalletsOrigin = OriginCaller; + type WeightInfo = (); } -// Build genesis storage according to the mock runtime. pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); pallet_balances::GenesisConfig:: { balances: vec![ - (account_id(1), 100000), // Alice - (account_id(2), 200000), // Bob - (account_id(3), 300000), // Charlie - (account_id(4), 400000), // Dave - (account_id(5), 500000), // Eve + (account_id(1), 100_000), + (account_id(2), 200_000), + (account_id(3), 300_000), + (account_id(4), 400_000), + (account_id(5), 500_000), ], } .assimilate_storage(&mut t) .unwrap(); + pallet_reversible_transfers::GenesisConfig:: { initial_high_security_accounts: vec![] } + .assimilate_storage(&mut t) + .unwrap(); + t.into() } diff --git a/pallets/multisig/src/tests.rs b/pallets/multisig/src/tests.rs index 277672ac..444661eb 100644 --- a/pallets/multisig/src/tests.rs +++ b/pallets/multisig/src/tests.rs @@ -1,9 +1,34 @@ //! Unit tests for pallet-multisig -use crate::{mock::*, Error, Event, GlobalNonce, Multisigs, ProposalStatus, Proposals}; +use crate::{mock::*, Error, Event, Multisigs, ProposalStatus, Proposals}; use codec::Encode; use frame_support::{assert_noop, assert_ok, traits::fungible::Mutate}; +use qp_high_security::HighSecurityInspector; use sp_core::crypto::AccountId32; +use sp_runtime::DispatchError; + +/// Mock implementation for HighSecurityInspector +pub struct MockHighSecurity; +impl HighSecurityInspector for MockHighSecurity { + fn is_high_security(who: &AccountId32) -> bool { + // For testing, account 100 is high security + who == &account_id(100) + } + fn is_whitelisted(call: &RuntimeCall) -> bool { + // For testing, only remarks with "safe" are whitelisted + match call { + RuntimeCall::System(frame_system::Call::remark { remark }) => remark == b"safe", + _ => false, + } + } + fn guardian(who: &AccountId32) -> Option { + if who == &account_id(100) { + Some(account_id(200)) // Guardian is account 200 + } else { + None + } + } +} /// Helper function to get Alice's account ID fn alice() -> AccountId32 { @@ -38,6 +63,18 @@ fn get_last_proposal_id(multisig_address: &AccountId32) -> u32 { multisig.proposal_nonce.saturating_sub(1) } +/// Assert that a DispatchResultWithPostInfo is Err with the expected error variant, +/// ignoring the PostDispatchInfo (actual_weight). +fn assert_err_ignore_postinfo( + result: sp_runtime::DispatchResultWithInfo, + expected: DispatchError, +) { + match result { + Err(err) => assert_eq!(err.error, expected), + Ok(_) => panic!("Expected Err({:?}), got Ok", expected), + } +} + // ==================== MULTISIG CREATION TESTS ==================== #[test] @@ -61,6 +98,7 @@ fn create_multisig_works() { RuntimeOrigin::signed(creator.clone()), signers.clone(), threshold, + 0, // nonce )); // Check balances @@ -69,19 +107,13 @@ fn create_multisig_works() { assert_eq!(Balances::free_balance(creator.clone()), initial_balance - fee - deposit); // Check that multisig was created - let global_nonce = GlobalNonce::::get(); - assert_eq!(global_nonce, 1); - // Get multisig address - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); // Check storage let multisig_data = Multisigs::::get(&multisig_address).unwrap(); assert_eq!(multisig_data.threshold, threshold); - assert_eq!(multisig_data.nonce, 0); assert_eq!(multisig_data.signers.to_vec(), signers); - assert_eq!(multisig_data.active_proposals, 0); - assert_eq!(multisig_data.creator, creator.clone()); assert_eq!(multisig_data.deposit, deposit); // Check that event was emitted @@ -100,7 +132,12 @@ fn create_multisig_fails_with_threshold_zero() { let threshold = 0; assert_noop!( - Multisig::create_multisig(RuntimeOrigin::signed(creator.clone()), signers, threshold,), + Multisig::create_multisig( + RuntimeOrigin::signed(creator.clone()), + signers, + threshold, + 0 + ), Error::::ThresholdZero ); }); @@ -114,7 +151,12 @@ fn create_multisig_fails_with_empty_signers() { let threshold = 1; assert_noop!( - Multisig::create_multisig(RuntimeOrigin::signed(creator.clone()), signers, threshold,), + Multisig::create_multisig( + RuntimeOrigin::signed(creator.clone()), + signers, + threshold, + 0 + ), Error::::NotEnoughSigners ); }); @@ -128,7 +170,12 @@ fn create_multisig_fails_with_threshold_too_high() { let threshold = 3; // More than number of signers assert_noop!( - Multisig::create_multisig(RuntimeOrigin::signed(creator.clone()), signers, threshold,), + Multisig::create_multisig( + RuntimeOrigin::signed(creator.clone()), + signers, + threshold, + 0 + ), Error::::ThresholdTooHigh ); }); @@ -142,7 +189,12 @@ fn create_multisig_fails_with_duplicate_signers() { let threshold = 2; assert_noop!( - Multisig::create_multisig(RuntimeOrigin::signed(creator.clone()), signers, threshold,), + Multisig::create_multisig( + RuntimeOrigin::signed(creator.clone()), + signers, + threshold, + 0 + ), Error::::DuplicateSigner ); }); @@ -155,20 +207,25 @@ fn create_multiple_multisigs_increments_nonce() { let signers1 = vec![bob(), charlie()]; let signers2 = vec![bob(), dave()]; + // Create first multisig with nonce=0 assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers1.clone(), - 2 + 2, + 0 // nonce )); + + // Create second multisig with nonce=1 assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers2.clone(), - 2 + 2, + 1 // nonce - user must provide different nonce )); - // Check both multisigs exist - let multisig1 = Multisig::derive_multisig_address(&signers1, 0); - let multisig2 = Multisig::derive_multisig_address(&signers2, 1); + // Check both multisigs exist with their respective nonces + let multisig1 = Multisig::derive_multisig_address(&signers1, 2, 0); + let multisig2 = Multisig::derive_multisig_address(&signers2, 2, 1); assert!(Multisigs::::contains_key(multisig1)); assert!(Multisigs::::contains_key(multisig2)); @@ -187,10 +244,11 @@ fn propose_works() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); // Propose a transaction let proposer = bob(); @@ -233,16 +291,17 @@ fn propose_fails_if_not_signer() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); // Try to propose as non-signer let call = make_call(vec![1, 2, 3]); - assert_noop!( + assert_err_ignore_postinfo( Multisig::propose(RuntimeOrigin::signed(dave()), multisig_address.clone(), call, 1000), - Error::::NotASigner + Error::::NotASigner.into(), ); }); } @@ -256,13 +315,15 @@ fn approve_works() { let creator = alice(); let signers = vec![bob(), charlie(), dave()]; + let threshold = 3; assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 3 + threshold, + 0 )); // Need 3 approvals - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, threshold, 0); let call = make_call(vec![1, 2, 3]); assert_ok!(Multisig::propose( @@ -298,7 +359,7 @@ fn approve_works() { } #[test] -fn approve_auto_executes_when_threshold_reached() { +fn approve_sets_approved_when_threshold_reached() { new_test_ext().execute_with(|| { System::set_block_number(1); @@ -307,10 +368,11 @@ fn approve_auto_executes_when_threshold_reached() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); let call = make_call(vec![1, 2, 3]); assert_ok!(Multisig::propose( @@ -322,20 +384,44 @@ fn approve_auto_executes_when_threshold_reached() { let proposal_id = get_last_proposal_id(&multisig_address); - // Charlie approves - threshold reached (2/2), auto-executes and removes + // Charlie approves - threshold reached (2/2), status becomes Approved assert_ok!(Multisig::approve( RuntimeOrigin::signed(charlie()), multisig_address.clone(), proposal_id )); - // Check that proposal was executed and immediately removed from storage + // Proposal should still exist with Approved status + let proposal = crate::Proposals::::get(&multisig_address, proposal_id).unwrap(); + assert_eq!(proposal.status, ProposalStatus::Approved); + + // Deposit should still be reserved (not returned until execute) + assert!(Balances::reserved_balance(bob()) > 0); + + // Check ProposalReadyToExecute event + System::assert_has_event( + Event::ProposalReadyToExecute { + multisig_address: multisig_address.clone(), + proposal_id, + approvals_count: 2, + } + .into(), + ); + + // Now any signer can execute + assert_ok!(Multisig::execute( + RuntimeOrigin::signed(charlie()), + multisig_address.clone(), + proposal_id + )); + + // Now proposal is removed assert!(crate::Proposals::::get(&multisig_address, proposal_id).is_none()); - // Deposit should be returned immediately - assert_eq!(Balances::reserved_balance(bob()), 0); // No longer reserved + // Deposit returned + assert_eq!(Balances::reserved_balance(bob()), 0); - // Check event was emitted + // Check execution event System::assert_has_event( Event::ProposalExecuted { multisig_address, @@ -362,10 +448,11 @@ fn cancel_works() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); let proposer = bob(); let call = make_call(vec![1, 2, 3]); @@ -408,10 +495,11 @@ fn cancel_fails_if_already_executed() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); let call = make_call(vec![1, 2, 3]); assert_ok!(Multisig::propose( @@ -423,17 +511,24 @@ fn cancel_fails_if_already_executed() { let proposal_id = get_last_proposal_id(&multisig_address); - // Approve to execute (auto-executes and removes proposal) + // Approve (reaches threshold → Approved) assert_ok!(Multisig::approve( RuntimeOrigin::signed(charlie()), multisig_address.clone(), proposal_id )); + // Execute (removes proposal from storage) + assert_ok!(Multisig::execute( + RuntimeOrigin::signed(charlie()), + multisig_address.clone(), + proposal_id + )); + // Try to cancel executed proposal (already removed, so ProposalNotFound) - assert_noop!( + assert_err_ignore_postinfo( Multisig::cancel(RuntimeOrigin::signed(bob()), multisig_address.clone(), proposal_id), - Error::::ProposalNotFound + Error::::ProposalNotFound.into(), ); }); } @@ -450,10 +545,11 @@ fn remove_expired_works_after_grace_period() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); let call = make_call(vec![1, 2, 3]); let expiry = 100; @@ -485,7 +581,7 @@ fn remove_expired_works_after_grace_period() { } #[test] -fn executed_proposals_auto_removed() { +fn executed_proposals_removed_from_storage() { new_test_ext().execute_with(|| { System::set_block_number(1); @@ -494,10 +590,11 @@ fn executed_proposals_auto_removed() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); let call = make_call(vec![1, 2, 3]); assert_ok!(Multisig::propose( @@ -509,20 +606,27 @@ fn executed_proposals_auto_removed() { let proposal_id = get_last_proposal_id(&multisig_address); - // Execute - should auto-remove proposal and return deposit + // Approve → Approved assert_ok!(Multisig::approve( RuntimeOrigin::signed(charlie()), multisig_address.clone(), proposal_id )); - // Proposal should be immediately removed + // Execute → removed from storage, deposit returned + assert_ok!(Multisig::execute( + RuntimeOrigin::signed(bob()), + multisig_address.clone(), + proposal_id + )); + + // Proposal should be removed assert!(crate::Proposals::::get(&multisig_address, proposal_id).is_none()); - // Deposit should be immediately returned + // Deposit should be returned assert_eq!(Balances::reserved_balance(bob()), 0); - // Trying to remove again should fail (already removed) + // Trying to remove again should fail assert_noop!( Multisig::remove_expired( RuntimeOrigin::signed(charlie()), @@ -544,10 +648,11 @@ fn remove_expired_fails_for_non_signer() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); let call = make_call(vec![1, 2, 3]); let expiry = 1000; @@ -592,10 +697,11 @@ fn claim_deposits_works() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); // Bob creates 3 proposals for i in 0..3 { @@ -643,10 +749,11 @@ fn claim_deposits_works() { fn derive_multisig_address_is_deterministic() { new_test_ext().execute_with(|| { let signers = vec![bob(), charlie(), dave()]; + let threshold = 2; let nonce = 42; - let address1 = Multisig::derive_multisig_address(&signers, nonce); - let address2 = Multisig::derive_multisig_address(&signers, nonce); + let address1 = Multisig::derive_multisig_address(&signers, threshold, nonce); + let address2 = Multisig::derive_multisig_address(&signers, threshold, nonce); assert_eq!(address1, address2); }); @@ -656,9 +763,10 @@ fn derive_multisig_address_is_deterministic() { fn derive_multisig_address_different_for_different_nonce() { new_test_ext().execute_with(|| { let signers = vec![bob(), charlie(), dave()]; + let threshold = 2; - let address1 = Multisig::derive_multisig_address(&signers, 0); - let address2 = Multisig::derive_multisig_address(&signers, 1); + let address1 = Multisig::derive_multisig_address(&signers, threshold, 0); + let address2 = Multisig::derive_multisig_address(&signers, threshold, 1); assert_ne!(address1, address2); }); @@ -668,9 +776,14 @@ fn derive_multisig_address_different_for_different_nonce() { fn is_signer_works() { new_test_ext().execute_with(|| { let signers = vec![bob(), charlie()]; - assert_ok!(Multisig::create_multisig(RuntimeOrigin::signed(alice()), signers.clone(), 2)); + assert_ok!(Multisig::create_multisig( + RuntimeOrigin::signed(alice()), + signers.clone(), + 2, + 0 + )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); assert!(Multisig::is_signer(&multisig_address, &bob())); assert!(Multisig::is_signer(&multisig_address, &charlie())); @@ -688,9 +801,10 @@ fn too_many_proposals_in_storage_fails() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); // MaxTotal = 20, 2 signers = 10 each // Executed/Cancelled proposals are auto-removed, so only Active count toward storage @@ -738,13 +852,15 @@ fn only_active_proposals_remain_in_storage() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); - // Test that only Active proposals remain in storage (Executed/Cancelled auto-removed) + // Test that only Active/Approved proposals remain in storage + // (Executed/Cancelled are removed) - // Bob creates 10, executes 5, cancels 1 - only 4 active remain + // Bob creates 10, approves+executes 5, cancels 1 - only 4 active remain for i in 0..10 { let call = make_call(vec![i as u8]); assert_ok!(Multisig::propose( @@ -756,11 +872,18 @@ fn only_active_proposals_remain_in_storage() { if i < 5 { let id = get_last_proposal_id(&multisig_address); + // Approve → Approved assert_ok!(Multisig::approve( RuntimeOrigin::signed(charlie()), multisig_address.clone(), id )); + // Execute → removed + assert_ok!(Multisig::execute( + RuntimeOrigin::signed(charlie()), + multisig_address.clone(), + id + )); } else if i == 5 { let id = get_last_proposal_id(&multisig_address); assert_ok!(Multisig::cancel( @@ -781,9 +904,9 @@ fn only_active_proposals_remain_in_storage() { 2000 )); } - // Bob: 10 Active (at per-signer limit) + // Bob: 10 Active (at per-signer limit: 20 total / 2 signers = 10 per signer) - // Bob cannot create 11th + // Bob cannot create 11th (exceeds per-signer limit) assert_noop!( Multisig::propose( RuntimeOrigin::signed(bob()), @@ -797,7 +920,7 @@ fn only_active_proposals_remain_in_storage() { } #[test] -fn auto_cleanup_allows_new_proposals() { +fn per_signer_limit_blocks_new_proposals_until_cleanup() { new_test_ext().execute_with(|| { System::set_block_number(1); @@ -806,9 +929,10 @@ fn auto_cleanup_allows_new_proposals() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); // Bob creates 10 proposals, all expire at block 100 (at per-signer limit) for i in 0..10 { @@ -819,7 +943,7 @@ fn auto_cleanup_allows_new_proposals() { 100 )); } - // Bob: 10 Active (at per-signer limit) + // Bob: 10 Active (at per-signer limit: 20 total / 2 signers = 10 per signer) // Bob cannot create more (at limit) assert_noop!( @@ -835,7 +959,24 @@ fn auto_cleanup_allows_new_proposals() { // Move past expiry System::set_block_number(101); - // Now Bob can create new - propose() auto-cleans expired + // propose() no longer auto-cleans, so Bob is still blocked + assert_noop!( + Multisig::propose( + RuntimeOrigin::signed(bob()), + multisig_address.clone(), + make_call(vec![99]), + 200 + ), + Error::::TooManyProposalsPerSigner + ); + + // Bob must explicitly claim deposits to free space + assert_ok!(Multisig::claim_deposits( + RuntimeOrigin::signed(bob()), + multisig_address.clone(), + )); + + // Now Bob can create new assert_ok!(Multisig::propose( RuntimeOrigin::signed(bob()), multisig_address.clone(), @@ -843,9 +984,9 @@ fn auto_cleanup_allows_new_proposals() { 200 )); - // Verify old proposals were removed + // Verify: old expired removed by claim_deposits, plus the new one let count = crate::Proposals::::iter_prefix(&multisig_address).count(); - assert_eq!(count, 1); // Only the new one remains + assert_eq!(count, 1); }); } @@ -859,10 +1000,11 @@ fn propose_fails_with_expiry_in_past() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); let call = make_call(vec![1, 2, 3]); @@ -908,10 +1050,11 @@ fn propose_fails_with_expiry_too_far() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); let call = make_call(vec![1, 2, 3]); @@ -972,10 +1115,11 @@ fn propose_charges_correct_fee_with_signer_factor() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); let proposer = bob(); let call = make_call(vec![1, 2, 3]); @@ -1010,30 +1154,37 @@ fn dissolve_multisig_works() { let creator = alice(); let signers = vec![bob(), charlie()]; let deposit = 500; - let fee = 1000; - let initial_balance = Balances::free_balance(creator.clone()); // Create assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, // threshold + 0 // nonce )); assert_eq!(Balances::reserved_balance(creator.clone()), deposit); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); - // Try to dissolve immediately (success) - assert_ok!(Multisig::dissolve_multisig( - RuntimeOrigin::signed(creator.clone()), + // Approve dissolve by Bob (1st approval) + assert_ok!(Multisig::approve_dissolve( + RuntimeOrigin::signed(bob()), multisig_address.clone() )); - // Check cleanup + // Still exists (threshold not reached) + assert!(Multisigs::::contains_key(&multisig_address)); + + // Approve dissolve by Charlie (2nd approval - threshold reached!) + assert_ok!(Multisig::approve_dissolve( + RuntimeOrigin::signed(charlie()), + multisig_address.clone() + )); + + // Check cleanup - multisig removed assert!(!Multisigs::::contains_key(&multisig_address)); + // Deposit was returned to creator (unreserved) assert_eq!(Balances::reserved_balance(creator.clone()), 0); - // Balance returned (minus burned fee) - assert_eq!(Balances::free_balance(creator.clone()), initial_balance - fee); }); } @@ -1046,9 +1197,10 @@ fn dissolve_multisig_fails_with_proposals() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, // threshold + 0 // nonce )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); // Create proposal let call = make_call(vec![1]); @@ -1059,12 +1211,9 @@ fn dissolve_multisig_fails_with_proposals() { 100 )); - // Try to dissolve + // Try to approve dissolve - should fail because proposals exist assert_noop!( - Multisig::dissolve_multisig( - RuntimeOrigin::signed(creator.clone()), - multisig_address.clone() - ), + Multisig::approve_dissolve(RuntimeOrigin::signed(bob()), multisig_address.clone()), Error::::ProposalsExist ); }); @@ -1079,9 +1228,10 @@ fn per_signer_proposal_limit_enforced() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - 2 + 2, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); // MaxTotalProposalsInStorage = 20 // With 2 signers, each can have max 20/2 = 10 proposals @@ -1119,7 +1269,7 @@ fn per_signer_proposal_limit_enforced() { } #[test] -fn propose_with_threshold_one_executes_immediately() { +fn propose_with_threshold_one_sets_approved() { new_test_ext().execute_with(|| { System::set_block_number(1); @@ -1131,17 +1281,18 @@ fn propose_with_threshold_one_executes_immediately() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - threshold + threshold, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, threshold, 0); // Fund multisig account for balance transfer as Mutate<_>>::mint_into(&multisig_address, 50000).unwrap(); let initial_dave_balance = Balances::free_balance(dave()); - // Alice proposes a transfer - should execute immediately since threshold=1 + // Alice proposes a transfer - threshold=1, so immediately Approved let transfer_call = RuntimeCall::Balances(pallet_balances::Call::transfer_keep_alive { dest: dave(), value: 1000, @@ -1154,34 +1305,39 @@ fn propose_with_threshold_one_executes_immediately() { 100 )); - let proposal_id = 0; // First proposal + let proposal_id = 0; - // Verify the proposal was executed immediately (should NOT exist anymore) - assert!(Proposals::::get(&multisig_address, proposal_id).is_none()); + // Proposal should be Approved (not executed yet) + let proposal = Proposals::::get(&multisig_address, proposal_id).unwrap(); + assert_eq!(proposal.status, ProposalStatus::Approved); - // Verify the transfer actually happened - assert_eq!(Balances::free_balance(dave()), initial_dave_balance + 1000); + // Transfer hasn't happened yet + assert_eq!(Balances::free_balance(dave()), initial_dave_balance); - // Verify ProposalExecuted event was emitted + // Check ProposalReadyToExecute event System::assert_has_event( - Event::ProposalExecuted { + Event::ProposalReadyToExecute { multisig_address: multisig_address.clone(), proposal_id, - proposer: alice(), - call: transfer_call.encode(), - approvers: vec![alice()], - result: Ok(()), + approvals_count: 1, } .into(), ); - // Verify deposit was returned to Alice (execution removes proposal) + // Any signer can now execute + assert_ok!(Multisig::execute( + RuntimeOrigin::signed(bob()), + multisig_address.clone(), + proposal_id + )); + + // Now the transfer happened + assert_eq!(Balances::free_balance(dave()), initial_dave_balance + 1000); + + // Proposal removed, deposit returned + assert!(Proposals::::get(&multisig_address, proposal_id).is_none()); let alice_reserved = Balances::reserved_balance(alice()); assert_eq!(alice_reserved, 500); // Only MultisigDeposit, no ProposalDeposit - - // Verify active_proposals counter was decremented back to 0 - let multisig_data = Multisigs::::get(&multisig_address).unwrap(); - assert_eq!(multisig_data.active_proposals, 0); }); } @@ -1198,10 +1354,11 @@ fn propose_with_threshold_two_waits_for_approval() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - threshold + threshold, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); // Fund multisig account as Mutate<_>>::mint_into(&multisig_address, 50000).unwrap(); @@ -1231,23 +1388,35 @@ fn propose_with_threshold_two_waits_for_approval() { // Verify the transfer did NOT happen yet assert_eq!(Balances::free_balance(dave()), initial_dave_balance); - // Bob approves - NOW it should execute (threshold=2 reached) + // Bob approves - threshold=2 reached → Approved assert_ok!(Multisig::approve( RuntimeOrigin::signed(bob()), multisig_address.clone(), proposal_id )); - // Now proposal should be executed and removed - assert!(Proposals::::get(&multisig_address, proposal_id).is_none()); + // Proposal should be Approved but NOT removed + let proposal = Proposals::::get(&multisig_address, proposal_id).unwrap(); + assert_eq!(proposal.status, ProposalStatus::Approved); + + // Transfer NOT yet happened + assert_eq!(Balances::free_balance(dave()), initial_dave_balance); - // Verify the transfer happened + // Now execute + assert_ok!(Multisig::execute( + RuntimeOrigin::signed(charlie()), + multisig_address.clone(), + proposal_id + )); + + // Now proposal removed and transfer happened + assert!(Proposals::::get(&multisig_address, proposal_id).is_none()); assert_eq!(Balances::free_balance(dave()), initial_dave_balance + 1000); }); } #[test] -fn auto_cleanup_on_approve_and_cancel() { +fn no_auto_cleanup_on_propose_approve_cancel() { new_test_ext().execute_with(|| { System::set_block_number(1); @@ -1259,10 +1428,11 @@ fn auto_cleanup_on_approve_and_cancel() { assert_ok!(Multisig::create_multisig( RuntimeOrigin::signed(creator.clone()), signers.clone(), - threshold + threshold, + 0 )); - let multisig_address = Multisig::derive_multisig_address(&signers, 0); + let multisig_address = Multisig::derive_multisig_address(&signers, 3, 0); // Create two proposals assert_ok!(Multisig::propose( @@ -1286,40 +1456,106 @@ fn auto_cleanup_on_approve_and_cancel() { // Move time forward past first proposal expiry System::set_block_number(101); - // Charlie approves proposal #1 (should trigger auto-cleanup of proposal #0) - // Note: Bob is the proposer of #1, so Charlie must approve + // approve() does NOT auto-cleanup assert_ok!(Multisig::approve( RuntimeOrigin::signed(charlie()), multisig_address.clone(), 1 )); + assert!(Proposals::::get(&multisig_address, 0).is_some()); // expired but still there - // Verify proposal #0 was auto-cleaned - assert!(Proposals::::get(&multisig_address, 0).is_none()); - // Proposal #1 still exists (not expired, waiting for approval) - assert!(Proposals::::get(&multisig_address, 1).is_some()); - - // Create another proposal that will expire + // propose() does NOT auto-cleanup either assert_ok!(Multisig::propose( RuntimeOrigin::signed(alice()), multisig_address.clone(), make_call(vec![3]), - 150 // expires at block 150 + 150 )); + // Proposal #0 still exists - not auto-cleaned + assert!(Proposals::::get(&multisig_address, 0).is_some()); + assert!(Proposals::::get(&multisig_address, 1).is_some()); + assert!(Proposals::::get(&multisig_address, 2).is_some()); - // Move time forward past proposal #2 expiry + // cancel() does NOT auto-cleanup System::set_block_number(151); - - // Charlie cancels proposal #1 (should trigger auto-cleanup of proposal #2) assert_ok!(Multisig::cancel(RuntimeOrigin::signed(bob()), multisig_address.clone(), 1)); + assert!(Proposals::::get(&multisig_address, 1).is_none()); // cancelled + assert!(Proposals::::get(&multisig_address, 0).is_some()); // expired, still there + assert!(Proposals::::get(&multisig_address, 2).is_some()); // expired, still there - // Verify proposal #2 was auto-cleaned + // Only explicit cleanup works: claim_deposits or remove_expired + assert_ok!(Multisig::claim_deposits( + RuntimeOrigin::signed(alice()), + multisig_address.clone(), + )); + // Alice's expired proposals (#0, #2) now cleaned + assert!(Proposals::::get(&multisig_address, 0).is_none()); assert!(Proposals::::get(&multisig_address, 2).is_none()); - // Proposal #1 was cancelled - assert!(Proposals::::get(&multisig_address, 1).is_none()); + }); +} - // Verify active_proposals counter is correct (should be 0) - let multisig_data = Multisigs::::get(&multisig_address).unwrap(); - assert_eq!(multisig_data.active_proposals, 0); +// ==================== HIGH SECURITY TESTS ==================== + +#[test] +fn high_security_propose_fails_for_non_whitelisted_call() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + + // Create a multisig with account_id(100) as one of signers + // We'll manually insert it as high-security multisig + let multisig_address = account_id(100); + let signers = vec![alice(), bob()]; + + Multisigs::::insert( + &multisig_address, + crate::MultisigData { + creator: alice(), + signers: signers.try_into().unwrap(), + threshold: 2, + proposal_nonce: 0, + deposit: 500, + active_proposals: 0, + proposals_per_signer: Default::default(), + }, + ); + + // Try to propose a non-whitelisted call (remark without "safe") + let call = make_call(b"unsafe".to_vec()); + assert_err_ignore_postinfo( + Multisig::propose(RuntimeOrigin::signed(alice()), multisig_address.clone(), call, 1000), + Error::::CallNotAllowedForHighSecurityMultisig.into(), + ); + + // Try to propose a whitelisted call (remark with "safe") - should work + let call = make_call(b"safe".to_vec()); + assert_ok!(Multisig::propose( + RuntimeOrigin::signed(alice()), + multisig_address.clone(), + call, + 1000 + )); + }); +} + +#[test] +fn normal_multisig_allows_any_call() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + + // Create a normal multisig (not high-security) + let signers = vec![alice(), bob(), charlie()]; + let threshold = 2; + assert_ok!(Multisig::create_multisig( + RuntimeOrigin::signed(alice()), + signers.clone(), + threshold, + 0 // nonce + )); + + let multisig_address = Multisig::derive_multisig_address(&signers, 2, 0); + + // Any call should work for normal multisig + let call = make_call(b"anything".to_vec()); + assert_ok!(Multisig::propose(RuntimeOrigin::signed(alice()), multisig_address, call, 1000)); }); } diff --git a/pallets/multisig/src/weights.rs b/pallets/multisig/src/weights.rs index 65a25230..e5732fe6 100644 --- a/pallets/multisig/src/weights.rs +++ b/pallets/multisig/src/weights.rs @@ -19,7 +19,7 @@ //! Autogenerated weights for `pallet_multisig` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 49.1.0 -//! DATE: 2026-01-28, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2026-02-11, STEPS: `20`, REPEAT: `50`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` //! HOSTNAME: `coldbook.local`, CPU: `` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024` @@ -28,13 +28,20 @@ // ./target/release/quantus-node // benchmark // pallet -// --chain=dev -// --pallet=pallet_multisig -// --extrinsic=* -// --steps=50 -// --repeat=20 -// --output=./pallets/multisig/src/weights.rs -// --template=./.maintain/frame-weight-template.hbs +// --chain +// dev +// --pallet +// pallet_multisig +// --extrinsic +// * +// --steps +// 20 +// --repeat +// 50 +// --output +// pallets/multisig/src/weights.rs +// --template +// .maintain/frame-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -47,297 +54,363 @@ use core::marker::PhantomData; /// Weight functions needed for `pallet_multisig`. pub trait WeightInfo { - fn create_multisig() -> Weight; - fn propose(c: u32, e: u32, ) -> Weight; - fn approve(c: u32, e: u32, ) -> Weight; - fn approve_and_execute(c: u32, ) -> Weight; - fn cancel(c: u32, e: u32, ) -> Weight; - fn remove_expired() -> Weight; - fn claim_deposits(p: u32, ) -> Weight; - fn dissolve_multisig() -> Weight; + fn create_multisig(s: u32, ) -> Weight; + fn propose(c: u32, ) -> Weight; + fn propose_high_security(c: u32, ) -> Weight; + fn approve(c: u32, ) -> Weight; + fn execute(c: u32, ) -> Weight; + fn cancel(c: u32, ) -> Weight; + fn remove_expired(c: u32, ) -> Weight; + fn claim_deposits(i: u32, r: u32, c: u32, ) -> Weight; + fn approve_dissolve() -> Weight; + fn approve_dissolve_threshold_reached() -> Weight; } /// Weights for `pallet_multisig` using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - /// Storage: `Multisig::GlobalNonce` (r:1 w:1) - /// Proof: `Multisig::GlobalNonce` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`) /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) - fn create_multisig() -> Weight { + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// The range of component `s` is `[2, 100]`. + fn create_multisig(s: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `152` - // Estimated: `10389` - // Minimum execution time: 192_000_000 picoseconds. - Weight::from_parts(195_000_000, 10389) - .saturating_add(T::DbWeight::get().reads(2_u64)) - .saturating_add(T::DbWeight::get().writes(2_u64)) + // Estimated: `10377` + // Minimum execution time: 189_000_000 picoseconds. + Weight::from_parts(120_513_891, 10377) + // Standard Error: 35_600 + .saturating_add(Weight::from_parts(4_800_379, 0).saturating_mul(s.into())) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) } /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) - /// Storage: `Multisig::Proposals` (r:201 w:201) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// Storage: `ReversibleTransfers::HighSecurityAccounts` (r:1 w:0) + /// Proof: `ReversibleTransfers::HighSecurityAccounts` (`max_values`: None, `max_size`: Some(89), added: 2564, mode: `MaxEncodedLen`) + /// Storage: `Multisig::Proposals` (r:1 w:1) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// The range of component `c` is `[0, 10140]`. - /// The range of component `e` is `[0, 200]`. - fn propose(_c: u32, e: u32, ) -> Weight { + fn propose(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `458 + e * (215 ±0)` - // Estimated: `17022 + e * (16032 ±0)` - // Minimum execution time: 40_000_000 picoseconds. - Weight::from_parts(140_354_473, 17022) - // Standard Error: 30_916 - .saturating_add(Weight::from_parts(14_183_732, 0).saturating_mul(e.into())) - .saturating_add(T::DbWeight::get().reads(2_u64)) - .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(e.into()))) + // Measured: `838` + // Estimated: `17022` + // Minimum execution time: 36_000_000 picoseconds. + Weight::from_parts(37_772_595, 17022) + // Standard Error: 7 + .saturating_add(Weight::from_parts(172, 0).saturating_mul(c.into())) + .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) - .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(e.into()))) - .saturating_add(Weight::from_parts(0, 16032).saturating_mul(e.into())) } /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) - /// Storage: `Multisig::Proposals` (r:202 w:201) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// Storage: `ReversibleTransfers::HighSecurityAccounts` (r:1 w:0) + /// Proof: `ReversibleTransfers::HighSecurityAccounts` (`max_values`: None, `max_size`: Some(89), added: 2564, mode: `MaxEncodedLen`) + /// Storage: `Multisig::Proposals` (r:1 w:1) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// The range of component `c` is `[0, 10140]`. - /// The range of component `e` is `[0, 200]`. - fn approve(_c: u32, e: u32, ) -> Weight { + fn propose_high_security(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `657 + c * (1 ±0) + e * (215 ±0)` - // Estimated: `33054 + e * (16032 ±0)` - // Minimum execution time: 23_000_000 picoseconds. - Weight::from_parts(31_012_674, 33054) - // Standard Error: 25_877 - .saturating_add(Weight::from_parts(13_708_908, 0).saturating_mul(e.into())) + // Measured: `838` + // Estimated: `17022` + // Minimum execution time: 36_000_000 picoseconds. + Weight::from_parts(38_070_055, 17022) + // Standard Error: 57 + .saturating_add(Weight::from_parts(672, 0).saturating_mul(c.into())) .saturating_add(T::DbWeight::get().reads(3_u64)) - .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(e.into()))) .saturating_add(T::DbWeight::get().writes(2_u64)) - .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(e.into()))) - .saturating_add(Weight::from_parts(0, 16032).saturating_mul(e.into())) + } + /// Storage: `Multisig::Multisigs` (r:1 w:0) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// Storage: `Multisig::Proposals` (r:1 w:1) + /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) + /// The range of component `c` is `[0, 10140]`. + fn approve(_c: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `6964 + c * (1 ±0)` + // Estimated: `17022` + // Minimum execution time: 17_000_000 picoseconds. + Weight::from_parts(33_776_726, 17022) + .saturating_add(T::DbWeight::get().reads(2_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) } /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) - /// Storage: `Multisig::Proposals` (r:2 w:1) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// Storage: `Multisig::Proposals` (r:1 w:1) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// The range of component `c` is `[0, 10140]`. - fn approve_and_execute(c: u32, ) -> Weight { + fn execute(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `790 + c * (1 ±0)` - // Estimated: `33054` - // Minimum execution time: 29_000_000 picoseconds. - Weight::from_parts(29_907_548, 33054) - // Standard Error: 17 - .saturating_add(Weight::from_parts(782, 0).saturating_mul(c.into())) - .saturating_add(T::DbWeight::get().reads(3_u64)) + // Measured: `6996 + c * (1 ±0)` + // Estimated: `17022` + // Minimum execution time: 25_000_000 picoseconds. + Weight::from_parts(43_059_043, 17022) + // Standard Error: 99 + .saturating_add(Weight::from_parts(890, 0).saturating_mul(c.into())) + .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } - /// Storage: `Multisig::Proposals` (r:202 w:201) + /// Storage: `Multisig::Proposals` (r:1 w:1) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) /// The range of component `c` is `[0, 10140]`. - /// The range of component `e` is `[0, 200]`. - fn cancel(c: u32, e: u32, ) -> Weight { + fn cancel(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `625 + c * (1 ±0) + e * (215 ±0)` - // Estimated: `33054 + e * (16032 ±0)` - // Minimum execution time: 27_000_000 picoseconds. - Weight::from_parts(22_414_315, 33054) - // Standard Error: 576 - .saturating_add(Weight::from_parts(1_526, 0).saturating_mul(c.into())) - // Standard Error: 29_178 - .saturating_add(Weight::from_parts(13_866_655, 0).saturating_mul(e.into())) - .saturating_add(T::DbWeight::get().reads(3_u64)) - .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(e.into()))) + // Measured: `3891 + c * (1 ±0)` + // Estimated: `17022` + // Minimum execution time: 22_000_000 picoseconds. + Weight::from_parts(25_331_594, 17022) + // Standard Error: 38 + .saturating_add(Weight::from_parts(451, 0).saturating_mul(c.into())) + .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) - .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(e.into()))) - .saturating_add(Weight::from_parts(0, 16032).saturating_mul(e.into())) } /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) /// Storage: `Multisig::Proposals` (r:1 w:1) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) - fn remove_expired() -> Weight { + /// The range of component `c` is `[0, 10140]`. + fn remove_expired(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `764` + // Measured: `3891 + c * (1 ±0)` // Estimated: `17022` // Minimum execution time: 21_000_000 picoseconds. - Weight::from_parts(23_000_000, 17022) + Weight::from_parts(25_274_827, 17022) + // Standard Error: 49 + .saturating_add(Weight::from_parts(425, 0).saturating_mul(c.into())) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } /// Storage: `Multisig::Proposals` (r:201 w:200) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) - /// The range of component `p` is `[1, 200]`. - fn claim_deposits(p: u32, ) -> Weight { + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// The range of component `i` is `[1, 200]`. + /// The range of component `r` is `[1, 200]`. + /// The range of component `c` is `[0, 10140]`. + fn claim_deposits(i: u32, r: u32, _c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `625 + p * (237 ±0)` - // Estimated: `17022 + p * (16032 ±0)` - // Minimum execution time: 23_000_000 picoseconds. - Weight::from_parts(28_491_742, 17022) - // Standard Error: 16_103 - .saturating_add(Weight::from_parts(13_535_595, 0).saturating_mul(p.into())) + // Measured: `3897 + c * (1 ±0) + i * (115 ±0)` + // Estimated: `17022 + i * (16032 ±0)` + // Minimum execution time: 27_000_000 picoseconds. + Weight::from_parts(30_000_000, 17022) + // Standard Error: 102_732 + .saturating_add(Weight::from_parts(11_666_897, 0).saturating_mul(i.into())) + // Standard Error: 102_732 + .saturating_add(Weight::from_parts(5_671_417, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(2_u64)) - .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(p.into()))) + .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(i.into()))) + .saturating_add(T::DbWeight::get().writes(2_u64)) + .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(i.into()))) + .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(r.into()))) + .saturating_add(Weight::from_parts(0, 16032).saturating_mul(i.into())) + } + /// Storage: `Multisig::Multisigs` (r:1 w:0) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// Storage: `Multisig::Proposals` (r:1 w:0) + /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) + /// Storage: `System::Account` (r:1 w:0) + /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) + /// Storage: `Multisig::DissolveApprovals` (r:1 w:1) + /// Proof: `Multisig::DissolveApprovals` (`max_values`: None, `max_size`: Some(3250), added: 5725, mode: `MaxEncodedLen`) + fn approve_dissolve() -> Weight { + // Proof Size summary in bytes: + // Measured: `526` + // Estimated: `17022` + // Minimum execution time: 13_000_000 picoseconds. + Weight::from_parts(16_000_000, 17022) + .saturating_add(T::DbWeight::get().reads(4_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) - .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(p.into()))) - .saturating_add(Weight::from_parts(0, 16032).saturating_mul(p.into())) } /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) /// Storage: `Multisig::Proposals` (r:1 w:0) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// Storage: `System::Account` (r:1 w:0) /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) - fn dissolve_multisig() -> Weight { + /// Storage: `Multisig::DissolveApprovals` (r:1 w:1) + /// Proof: `Multisig::DissolveApprovals` (`max_values`: None, `max_size`: Some(3250), added: 5725, mode: `MaxEncodedLen`) + fn approve_dissolve_threshold_reached() -> Weight { // Proof Size summary in bytes: - // Measured: `538` + // Measured: `703` // Estimated: `17022` - // Minimum execution time: 20_000_000 picoseconds. - Weight::from_parts(30_000_000, 17022) - .saturating_add(T::DbWeight::get().reads(3_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) + // Minimum execution time: 25_000_000 picoseconds. + Weight::from_parts(28_000_000, 17022) + .saturating_add(T::DbWeight::get().reads(4_u64)) + .saturating_add(T::DbWeight::get().writes(2_u64)) } } // For backwards compatibility and tests. impl WeightInfo for () { - /// Storage: `Multisig::GlobalNonce` (r:1 w:1) - /// Proof: `Multisig::GlobalNonce` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`) /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) - fn create_multisig() -> Weight { + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// The range of component `s` is `[2, 100]`. + fn create_multisig(s: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `152` - // Estimated: `10389` - // Minimum execution time: 192_000_000 picoseconds. - Weight::from_parts(195_000_000, 10389) - .saturating_add(RocksDbWeight::get().reads(2_u64)) - .saturating_add(RocksDbWeight::get().writes(2_u64)) + // Estimated: `10377` + // Minimum execution time: 189_000_000 picoseconds. + Weight::from_parts(120_513_891, 10377) + // Standard Error: 35_600 + .saturating_add(Weight::from_parts(4_800_379, 0).saturating_mul(s.into())) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) } /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) - /// Storage: `Multisig::Proposals` (r:201 w:201) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// Storage: `ReversibleTransfers::HighSecurityAccounts` (r:1 w:0) + /// Proof: `ReversibleTransfers::HighSecurityAccounts` (`max_values`: None, `max_size`: Some(89), added: 2564, mode: `MaxEncodedLen`) + /// Storage: `Multisig::Proposals` (r:1 w:1) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// The range of component `c` is `[0, 10140]`. - /// The range of component `e` is `[0, 200]`. - fn propose(_c: u32, e: u32, ) -> Weight { + fn propose(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `458 + e * (215 ±0)` - // Estimated: `17022 + e * (16032 ±0)` - // Minimum execution time: 40_000_000 picoseconds. - Weight::from_parts(140_354_473, 17022) - // Standard Error: 30_916 - .saturating_add(Weight::from_parts(14_183_732, 0).saturating_mul(e.into())) - .saturating_add(RocksDbWeight::get().reads(2_u64)) - .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(e.into()))) + // Measured: `838` + // Estimated: `17022` + // Minimum execution time: 36_000_000 picoseconds. + Weight::from_parts(37_772_595, 17022) + // Standard Error: 7 + .saturating_add(Weight::from_parts(172, 0).saturating_mul(c.into())) + .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) - .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(e.into()))) - .saturating_add(Weight::from_parts(0, 16032).saturating_mul(e.into())) } /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) - /// Storage: `Multisig::Proposals` (r:202 w:201) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// Storage: `ReversibleTransfers::HighSecurityAccounts` (r:1 w:0) + /// Proof: `ReversibleTransfers::HighSecurityAccounts` (`max_values`: None, `max_size`: Some(89), added: 2564, mode: `MaxEncodedLen`) + /// Storage: `Multisig::Proposals` (r:1 w:1) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// The range of component `c` is `[0, 10140]`. - /// The range of component `e` is `[0, 200]`. - fn approve(_c: u32, e: u32, ) -> Weight { + fn propose_high_security(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `657 + c * (1 ±0) + e * (215 ±0)` - // Estimated: `33054 + e * (16032 ±0)` - // Minimum execution time: 23_000_000 picoseconds. - Weight::from_parts(31_012_674, 33054) - // Standard Error: 25_877 - .saturating_add(Weight::from_parts(13_708_908, 0).saturating_mul(e.into())) + // Measured: `838` + // Estimated: `17022` + // Minimum execution time: 36_000_000 picoseconds. + Weight::from_parts(38_070_055, 17022) + // Standard Error: 57 + .saturating_add(Weight::from_parts(672, 0).saturating_mul(c.into())) .saturating_add(RocksDbWeight::get().reads(3_u64)) - .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(e.into()))) .saturating_add(RocksDbWeight::get().writes(2_u64)) - .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(e.into()))) - .saturating_add(Weight::from_parts(0, 16032).saturating_mul(e.into())) + } + /// Storage: `Multisig::Multisigs` (r:1 w:0) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// Storage: `Multisig::Proposals` (r:1 w:1) + /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) + /// The range of component `c` is `[0, 10140]`. + fn approve(_c: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `6964 + c * (1 ±0)` + // Estimated: `17022` + // Minimum execution time: 17_000_000 picoseconds. + Weight::from_parts(33_776_726, 17022) + .saturating_add(RocksDbWeight::get().reads(2_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) } /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) - /// Storage: `Multisig::Proposals` (r:2 w:1) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// Storage: `Multisig::Proposals` (r:1 w:1) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// The range of component `c` is `[0, 10140]`. - fn approve_and_execute(c: u32, ) -> Weight { + fn execute(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `790 + c * (1 ±0)` - // Estimated: `33054` - // Minimum execution time: 29_000_000 picoseconds. - Weight::from_parts(29_907_548, 33054) - // Standard Error: 17 - .saturating_add(Weight::from_parts(782, 0).saturating_mul(c.into())) - .saturating_add(RocksDbWeight::get().reads(3_u64)) + // Measured: `6996 + c * (1 ±0)` + // Estimated: `17022` + // Minimum execution time: 25_000_000 picoseconds. + Weight::from_parts(43_059_043, 17022) + // Standard Error: 99 + .saturating_add(Weight::from_parts(890, 0).saturating_mul(c.into())) + .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } - /// Storage: `Multisig::Proposals` (r:202 w:201) + /// Storage: `Multisig::Proposals` (r:1 w:1) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) /// The range of component `c` is `[0, 10140]`. - /// The range of component `e` is `[0, 200]`. - fn cancel(c: u32, e: u32, ) -> Weight { + fn cancel(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `625 + c * (1 ±0) + e * (215 ±0)` - // Estimated: `33054 + e * (16032 ±0)` - // Minimum execution time: 27_000_000 picoseconds. - Weight::from_parts(22_414_315, 33054) - // Standard Error: 576 - .saturating_add(Weight::from_parts(1_526, 0).saturating_mul(c.into())) - // Standard Error: 29_178 - .saturating_add(Weight::from_parts(13_866_655, 0).saturating_mul(e.into())) - .saturating_add(RocksDbWeight::get().reads(3_u64)) - .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(e.into()))) + // Measured: `3891 + c * (1 ±0)` + // Estimated: `17022` + // Minimum execution time: 22_000_000 picoseconds. + Weight::from_parts(25_331_594, 17022) + // Standard Error: 38 + .saturating_add(Weight::from_parts(451, 0).saturating_mul(c.into())) + .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) - .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(e.into()))) - .saturating_add(Weight::from_parts(0, 16032).saturating_mul(e.into())) } /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) /// Storage: `Multisig::Proposals` (r:1 w:1) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) - fn remove_expired() -> Weight { + /// The range of component `c` is `[0, 10140]`. + fn remove_expired(c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `764` + // Measured: `3891 + c * (1 ±0)` // Estimated: `17022` // Minimum execution time: 21_000_000 picoseconds. - Weight::from_parts(23_000_000, 17022) + Weight::from_parts(25_274_827, 17022) + // Standard Error: 49 + .saturating_add(Weight::from_parts(425, 0).saturating_mul(c.into())) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } /// Storage: `Multisig::Proposals` (r:201 w:200) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) - /// The range of component `p` is `[1, 200]`. - fn claim_deposits(p: u32, ) -> Weight { + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// The range of component `i` is `[1, 200]`. + /// The range of component `r` is `[1, 200]`. + /// The range of component `c` is `[0, 10140]`. + fn claim_deposits(i: u32, r: u32, _c: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `625 + p * (237 ±0)` - // Estimated: `17022 + p * (16032 ±0)` - // Minimum execution time: 23_000_000 picoseconds. - Weight::from_parts(28_491_742, 17022) - // Standard Error: 16_103 - .saturating_add(Weight::from_parts(13_535_595, 0).saturating_mul(p.into())) + // Measured: `3897 + c * (1 ±0) + i * (115 ±0)` + // Estimated: `17022 + i * (16032 ±0)` + // Minimum execution time: 27_000_000 picoseconds. + Weight::from_parts(30_000_000, 17022) + // Standard Error: 102_732 + .saturating_add(Weight::from_parts(11_666_897, 0).saturating_mul(i.into())) + // Standard Error: 102_732 + .saturating_add(Weight::from_parts(5_671_417, 0).saturating_mul(r.into())) .saturating_add(RocksDbWeight::get().reads(2_u64)) - .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(p.into()))) + .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(i.into()))) + .saturating_add(RocksDbWeight::get().writes(2_u64)) + .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(i.into()))) + .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(r.into()))) + .saturating_add(Weight::from_parts(0, 16032).saturating_mul(i.into())) + } + /// Storage: `Multisig::Multisigs` (r:1 w:0) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) + /// Storage: `Multisig::Proposals` (r:1 w:0) + /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) + /// Storage: `System::Account` (r:1 w:0) + /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) + /// Storage: `Multisig::DissolveApprovals` (r:1 w:1) + /// Proof: `Multisig::DissolveApprovals` (`max_values`: None, `max_size`: Some(3250), added: 5725, mode: `MaxEncodedLen`) + fn approve_dissolve() -> Weight { + // Proof Size summary in bytes: + // Measured: `526` + // Estimated: `17022` + // Minimum execution time: 13_000_000 picoseconds. + Weight::from_parts(16_000_000, 17022) + .saturating_add(RocksDbWeight::get().reads(4_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) - .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(p.into()))) - .saturating_add(Weight::from_parts(0, 16032).saturating_mul(p.into())) } /// Storage: `Multisig::Multisigs` (r:1 w:1) - /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6924), added: 9399, mode: `MaxEncodedLen`) + /// Proof: `Multisig::Multisigs` (`max_values`: None, `max_size`: Some(6912), added: 9387, mode: `MaxEncodedLen`) /// Storage: `Multisig::Proposals` (r:1 w:0) /// Proof: `Multisig::Proposals` (`max_values`: None, `max_size`: Some(13557), added: 16032, mode: `MaxEncodedLen`) /// Storage: `System::Account` (r:1 w:0) /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) - fn dissolve_multisig() -> Weight { + /// Storage: `Multisig::DissolveApprovals` (r:1 w:1) + /// Proof: `Multisig::DissolveApprovals` (`max_values`: None, `max_size`: Some(3250), added: 5725, mode: `MaxEncodedLen`) + fn approve_dissolve_threshold_reached() -> Weight { // Proof Size summary in bytes: - // Measured: `538` + // Measured: `703` // Estimated: `17022` - // Minimum execution time: 20_000_000 picoseconds. - Weight::from_parts(30_000_000, 17022) - .saturating_add(RocksDbWeight::get().reads(3_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) + // Minimum execution time: 25_000_000 picoseconds. + Weight::from_parts(28_000_000, 17022) + .saturating_add(RocksDbWeight::get().reads(4_u64)) + .saturating_add(RocksDbWeight::get().writes(2_u64)) } } diff --git a/pallets/reversible-transfers/Cargo.toml b/pallets/reversible-transfers/Cargo.toml index c79f934e..a6685469 100644 --- a/pallets/reversible-transfers/Cargo.toml +++ b/pallets/reversible-transfers/Cargo.toml @@ -21,6 +21,7 @@ pallet-assets.workspace = true pallet-assets-holder.workspace = true pallet-balances.workspace = true pallet-recovery.workspace = true +qp-high-security = { path = "../../primitives/high-security", default-features = false } qp-scheduler.workspace = true scale-info = { features = ["derive"], workspace = true } sp-arithmetic.workspace = true @@ -53,6 +54,7 @@ std = [ "pallet-scheduler/std", "pallet-timestamp/std", "pallet-utility/std", + "qp-high-security/std", "qp-scheduler/std", "scale-info/std", "sp-core/std", @@ -67,6 +69,7 @@ runtime-benchmarks = [ "frame-system/runtime-benchmarks", "pallet-assets/runtime-benchmarks", "pallet-balances/runtime-benchmarks", + "qp-high-security/runtime-benchmarks", "sp-runtime/runtime-benchmarks", ] try-runtime = [ diff --git a/pallets/reversible-transfers/README.md b/pallets/reversible-transfers/README.md index 62ca845a..31d1cc65 100644 --- a/pallets/reversible-transfers/README.md +++ b/pallets/reversible-transfers/README.md @@ -1,6 +1,14 @@ -### Motivation +# Reversible Transfers Pallet -To have accounts for which all outgoing transfer are subject to a variable time during which they may be cancelled. The idea is this could be used to deter theft as well as correct mistakes. +## Motivation + +To have accounts for which all outgoing transfers are subject to a variable time during which they may be cancelled. The idea is this could be used to deter theft as well as correct mistakes. + +**Use Cases:** +- **High-security custody:** Corporate treasury with guardian oversight +- **Mistake recovery:** Cancel accidental transfers during delay period +- **Theft deterrence:** Guardian can cancel suspicious transfers before execution +- **Regulatory compliance:** Time-delayed transfers with oversight capabilities ## Design @@ -32,3 +40,119 @@ Pending/delayed transfers can be tracked at `PendingTransfers` storage and by su ### Notes - Transaction id is `((who, call).hash())` where `who` is the account that called the transaction and `call` is the call itself. This is used to identify the transaction in the scheduler and preimage. For identical transfers, there is a counter in `PendingTransfer` to differentiate between them. + +## High-Security Integration + +This pallet provides the **HighSecurityInspector** trait for integrating high-security features with other pallets (like `pallet-multisig`). + +### HighSecurityInspector Trait + +```rust +pub trait HighSecurityInspector { + /// Check if account is registered as high-security + fn is_high_security(who: &AccountId) -> bool; + + /// Check if call is whitelisted for high-security accounts + fn is_whitelisted(call: &RuntimeCall) -> bool; + + /// Get guardian account for high-security account (if exists) + fn guardian(who: &AccountId) -> Option; +} +``` + +**Purpose:** +- Provides unified interface for high-security checks +- Used by `pallet-multisig` for call whitelisting +- Used by transaction extensions for EOA whitelisting +- Implemented by runtime for call pattern matching + +### Implementation + +**This pallet provides:** +- Trait definition (`pub trait HighSecurityInspector`) +- Helper functions for runtime implementation: + - `is_high_security_account(who)` - checks `HighSecurityAccounts` storage + - `get_guardian(who)` - retrieves guardian from storage +- Default no-op implementation: `impl HighSecurityInspector for ()` + +**Runtime implements:** +- The actual `is_whitelisted(call)` logic (requires `RuntimeCall` access) +- Delegates `is_high_security` and `guardian` to pallet helper functions +- Example: + +```rust +pub struct HighSecurityConfig; + +impl HighSecurityInspector for HighSecurityConfig { + fn is_high_security(who: &AccountId) -> bool { + // Delegate to pallet helper + ReversibleTransfers::is_high_security_account(who) + } + + fn is_whitelisted(call: &RuntimeCall) -> bool { + // Runtime implements pattern matching (has RuntimeCall access) + matches!( + call, + RuntimeCall::ReversibleTransfers(Call::schedule_transfer { .. }) | + RuntimeCall::ReversibleTransfers(Call::schedule_asset_transfer { .. }) | + RuntimeCall::ReversibleTransfers(Call::cancel { .. }) + ) + } + + fn guardian(who: &AccountId) -> Option { + // Delegate to pallet helper + ReversibleTransfers::get_guardian(who) + } +} +``` + +### Usage by Other Pallets + +**pallet-multisig:** +```rust +impl pallet_multisig::Config for Runtime { + type HighSecurity = HighSecurityConfig; + // ... +} + +// In multisig propose(): +if T::HighSecurity::is_high_security(&multisig_address) { + let decoded_call = RuntimeCall::decode(&call)?; + ensure!( + T::HighSecurity::is_whitelisted(&decoded_call), + Error::CallNotAllowedForHighSecurityMultisig + ); +} +``` + +**Transaction Extensions:** +```rust +// In ReversibleTransactionExtension::validate(): +if HighSecurityConfig::is_high_security(&who) { + ensure!( + HighSecurityConfig::is_whitelisted(&call), + TransactionValidityError::Invalid(InvalidTransaction::Call) + ); +} +``` + +### Architecture Benefits + +**Single Source of Truth:** +- Whitelist defined once in runtime +- Used by multisig, transaction extensions, and any future consumers +- Easy to maintain and update + +**Modularity:** +- Trait defined in this pallet (storage owner) +- Implementation in runtime (has `RuntimeCall` access) +- Consumers use trait without coupling to implementation + +**Reusability:** +- Same security model for EOAs and multisigs +- Consistent whitelist enforcement across all account types +- Easy to add new consumers (just use the trait) + +### Documentation + +See `MULTISIG_REQ.md` for complete high-security integration architecture and examples. diff --git a/pallets/reversible-transfers/src/benchmarking.rs b/pallets/reversible-transfers/src/benchmarking.rs index ae69f5b0..2824aaf7 100644 --- a/pallets/reversible-transfers/src/benchmarking.rs +++ b/pallets/reversible-transfers/src/benchmarking.rs @@ -13,6 +13,17 @@ use sp_runtime::{ const SEED: u32 = 0; +/// Helper for external benchmarks (e.g., `pallet-multisig`) to set up HS storage state. +/// Bypasses all validation - direct storage write only for benchmarking. +pub fn insert_hs_account_for_benchmark( + who: T::AccountId, + data: HighSecurityAccountData>, +) where + T: Config, +{ + HighSecurityAccounts::::insert(who, data); +} + // Helper to create a RuntimeCall (e.g., a balance transfer) // Adjust type parameters as needed for your actual Balance type if not u128 fn make_transfer_call( diff --git a/pallets/reversible-transfers/src/lib.rs b/pallets/reversible-transfers/src/lib.rs index de235288..45de0e47 100644 --- a/pallets/reversible-transfers/src/lib.rs +++ b/pallets/reversible-transfers/src/lib.rs @@ -20,7 +20,7 @@ pub use pallet::*; mod tests; #[cfg(feature = "runtime-benchmarks")] -mod benchmarking; +pub mod benchmarking; pub mod weights; pub use weights::WeightInfo; @@ -37,6 +37,21 @@ use qp_scheduler::{BlockNumberOrTimestamp, DispatchTime, ScheduleNamed}; use sp_arithmetic::Permill; use sp_runtime::traits::StaticLookup; +// Partial implementation for Pallet - runtime will complete it +impl Pallet { + /// Check if account is registered as high-security + /// This is used by runtime's HighSecurityInspector implementation + pub fn is_high_security_account(who: &T::AccountId) -> bool { + HighSecurityAccounts::::contains_key(who) + } + + /// Get guardian for high-security account + /// This is used by runtime's HighSecurityInspector implementation + pub fn get_guardian(who: &T::AccountId) -> Option { + HighSecurityAccounts::::get(who).map(|data| data.interceptor) + } +} + /// Type alias for this config's `BlockNumberOrTimestamp`. pub type BlockNumberOrTimestampOf = BlockNumberOrTimestamp, ::Moment>; diff --git a/pallets/reversible-transfers/src/weights.rs b/pallets/reversible-transfers/src/weights.rs index b5c0f5e2..632d3504 100644 --- a/pallets/reversible-transfers/src/weights.rs +++ b/pallets/reversible-transfers/src/weights.rs @@ -19,9 +19,9 @@ //! Autogenerated weights for `pallet_reversible_transfers` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 49.1.0 -//! DATE: 2026-01-26, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2026-01-30, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `arunachala.local`, CPU: `` +//! HOSTNAME: `coldbook.local`, CPU: `` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024` // Executed Command: @@ -33,8 +33,8 @@ // --extrinsic=* // --steps=50 // --repeat=20 -// --output=pallets/reversible-transfers/src/weights.rs -// --template=.maintain/frame-weight-template.hbs +// --template=./.maintain/frame-weight-template.hbs +// --output=./pallets/reversible-transfers/src/weights.rs #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -63,10 +63,10 @@ impl WeightInfo for SubstrateWeight { /// Proof: `ReversibleTransfers::InterceptorIndex` (`max_values`: None, `max_size`: Some(1073), added: 3548, mode: `MaxEncodedLen`) fn set_high_security() -> Weight { // Proof Size summary in bytes: - // Measured: `192` + // Measured: `152` // Estimated: `4538` - // Minimum execution time: 78_000_000 picoseconds. - Weight::from_parts(80_000_000, 4538) + // Minimum execution time: 41_000_000 picoseconds. + Weight::from_parts(43_000_000, 4538) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -92,10 +92,10 @@ impl WeightInfo for SubstrateWeight { /// Proof: `ReversibleTransfers::PendingTransfers` (`max_values`: None, `max_size`: Some(291), added: 2766, mode: `MaxEncodedLen`) fn schedule_transfer() -> Weight { // Proof Size summary in bytes: - // Measured: `637` + // Measured: `597` // Estimated: `14183` - // Minimum execution time: 536_000_000 picoseconds. - Weight::from_parts(550_000_000, 14183) + // Minimum execution time: 285_000_000 picoseconds. + Weight::from_parts(307_000_000, 14183) .saturating_add(T::DbWeight::get().reads(9_u64)) .saturating_add(T::DbWeight::get().writes(8_u64)) } @@ -115,16 +115,16 @@ impl WeightInfo for SubstrateWeight { /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(10718), added: 13193, mode: `MaxEncodedLen`) /// Storage: `Balances::Holds` (r:1 w:1) /// Proof: `Balances::Holds` (`max_values`: None, `max_size`: Some(85), added: 2560, mode: `MaxEncodedLen`) - /// Storage: `System::Account` (r:2 w:2) + /// Storage: `System::Account` (r:1 w:1) /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) fn cancel() -> Weight { // Proof Size summary in bytes: - // Measured: `2224` + // Measured: `1880` // Estimated: `14183` - // Minimum execution time: 342_000_000 picoseconds. - Weight::from_parts(349_000_000, 14183) - .saturating_add(T::DbWeight::get().reads(10_u64)) - .saturating_add(T::DbWeight::get().writes(9_u64)) + // Minimum execution time: 172_000_000 picoseconds. + Weight::from_parts(195_000_000, 14183) + .saturating_add(T::DbWeight::get().reads(9_u64)) + .saturating_add(T::DbWeight::get().writes(8_u64)) } /// Storage: `ReversibleTransfers::PendingTransfers` (r:1 w:1) /// Proof: `ReversibleTransfers::PendingTransfers` (`max_values`: None, `max_size`: Some(291), added: 2766, mode: `MaxEncodedLen`) @@ -146,8 +146,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `1360` // Estimated: `3834` - // Minimum execution time: 276_000_000 picoseconds. - Weight::from_parts(290_000_000, 3834) + // Minimum execution time: 180_000_000 picoseconds. + Weight::from_parts(196_000_000, 3834) .saturating_add(T::DbWeight::get().reads(7_u64)) .saturating_add(T::DbWeight::get().writes(8_u64)) } @@ -163,8 +163,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `477` // Estimated: `3593` - // Minimum execution time: 103_000_000 picoseconds. - Weight::from_parts(106_000_000, 3593) + // Minimum execution time: 87_000_000 picoseconds. + Weight::from_parts(96_000_000, 3593) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(3_u64)) } @@ -178,10 +178,10 @@ impl WeightInfo for () { /// Proof: `ReversibleTransfers::InterceptorIndex` (`max_values`: None, `max_size`: Some(1073), added: 3548, mode: `MaxEncodedLen`) fn set_high_security() -> Weight { // Proof Size summary in bytes: - // Measured: `192` + // Measured: `152` // Estimated: `4538` - // Minimum execution time: 78_000_000 picoseconds. - Weight::from_parts(80_000_000, 4538) + // Minimum execution time: 41_000_000 picoseconds. + Weight::from_parts(43_000_000, 4538) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -207,10 +207,10 @@ impl WeightInfo for () { /// Proof: `ReversibleTransfers::PendingTransfers` (`max_values`: None, `max_size`: Some(291), added: 2766, mode: `MaxEncodedLen`) fn schedule_transfer() -> Weight { // Proof Size summary in bytes: - // Measured: `637` + // Measured: `597` // Estimated: `14183` - // Minimum execution time: 536_000_000 picoseconds. - Weight::from_parts(550_000_000, 14183) + // Minimum execution time: 285_000_000 picoseconds. + Weight::from_parts(307_000_000, 14183) .saturating_add(RocksDbWeight::get().reads(9_u64)) .saturating_add(RocksDbWeight::get().writes(8_u64)) } @@ -230,16 +230,16 @@ impl WeightInfo for () { /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(10718), added: 13193, mode: `MaxEncodedLen`) /// Storage: `Balances::Holds` (r:1 w:1) /// Proof: `Balances::Holds` (`max_values`: None, `max_size`: Some(85), added: 2560, mode: `MaxEncodedLen`) - /// Storage: `System::Account` (r:2 w:2) + /// Storage: `System::Account` (r:1 w:1) /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) fn cancel() -> Weight { // Proof Size summary in bytes: - // Measured: `2224` + // Measured: `1880` // Estimated: `14183` - // Minimum execution time: 342_000_000 picoseconds. - Weight::from_parts(349_000_000, 14183) - .saturating_add(RocksDbWeight::get().reads(10_u64)) - .saturating_add(RocksDbWeight::get().writes(9_u64)) + // Minimum execution time: 172_000_000 picoseconds. + Weight::from_parts(195_000_000, 14183) + .saturating_add(RocksDbWeight::get().reads(9_u64)) + .saturating_add(RocksDbWeight::get().writes(8_u64)) } /// Storage: `ReversibleTransfers::PendingTransfers` (r:1 w:1) /// Proof: `ReversibleTransfers::PendingTransfers` (`max_values`: None, `max_size`: Some(291), added: 2766, mode: `MaxEncodedLen`) @@ -261,8 +261,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `1360` // Estimated: `3834` - // Minimum execution time: 276_000_000 picoseconds. - Weight::from_parts(290_000_000, 3834) + // Minimum execution time: 180_000_000 picoseconds. + Weight::from_parts(196_000_000, 3834) .saturating_add(RocksDbWeight::get().reads(7_u64)) .saturating_add(RocksDbWeight::get().writes(8_u64)) } @@ -278,8 +278,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `477` // Estimated: `3593` - // Minimum execution time: 103_000_000 picoseconds. - Weight::from_parts(106_000_000, 3593) + // Minimum execution time: 87_000_000 picoseconds. + Weight::from_parts(96_000_000, 3593) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(3_u64)) } diff --git a/primitives/high-security/Cargo.toml b/primitives/high-security/Cargo.toml new file mode 100644 index 00000000..d213ef10 --- /dev/null +++ b/primitives/high-security/Cargo.toml @@ -0,0 +1,22 @@ +[package] +authors.workspace = true +description = "High-Security account primitives for Quantus blockchain" +edition.workspace = true +homepage.workspace = true +license = "Apache-2.0" +name = "qp-high-security" +publish = false +repository.workspace = true +version = "0.1.0" + +[dependencies] +codec = { workspace = true } +scale-info = { workspace = true } + +[features] +default = ["std"] +runtime-benchmarks = [] +std = [ + "codec/std", + "scale-info/std", +] diff --git a/primitives/high-security/src/lib.rs b/primitives/high-security/src/lib.rs new file mode 100644 index 00000000..d923ec67 --- /dev/null +++ b/primitives/high-security/src/lib.rs @@ -0,0 +1,156 @@ +//! High-Security Account Primitives +//! +//! This crate provides the core trait for High-Security account inspection and validation +//! in the Quantus blockchain. High-Security accounts are designed for institutional users +//! and high-value accounts that require additional security controls: +//! +//! - **Call whitelisting**: Only approved operations can be executed +//! - **Guardian/interceptor role**: Designated account can cancel malicious transactions +//! - **Delayed execution**: Time window for intervention before irreversible actions +//! +//! ## Architecture +//! +//! The `HighSecurityInspector` trait is implemented at the runtime level and consumed by: +//! - `pallet-multisig`: Validates calls in High-Security multisigs +//! - `pallet-reversible-transfers`: Provides the storage and core HS account management +//! - Transaction extensions: Validates calls for High-Security EOAs +//! +//! This primitives crate breaks the circular dependency between pallets by providing +//! a shared abstraction that all consumers can depend on. + +#![cfg_attr(not(feature = "std"), no_std)] + +/// High-Security account inspector trait +/// +/// Provides methods to check if an account is designated as High-Security, +/// validate whitelisted calls, and retrieve guardian information. +/// +/// # Type Parameters +/// +/// - `AccountId`: The account identifier type +/// - `RuntimeCall`: The runtime-level call enum (required for whitelist validation) +/// +/// # Implementation Notes +/// +/// This trait is typically implemented at the runtime level in a configuration struct +/// that bridges multiple pallets. The runtime implementation delegates to the actual +/// storage pallet (e.g., `pallet-reversible-transfers`) for account status checks +/// and defines the runtime-specific whitelist logic. +/// +/// # Example +/// +/// ```ignore +/// // In runtime/src/configs/mod.rs +/// pub struct HighSecurityConfig; +/// +/// impl qp_high_security::HighSecurityInspector +/// for HighSecurityConfig +/// { +/// fn is_high_security(who: &AccountId) -> bool { +/// pallet_reversible_transfers::Pallet::::is_high_security_account(who) +/// } +/// +/// fn is_whitelisted(call: &RuntimeCall) -> bool { +/// matches!( +/// call, +/// RuntimeCall::ReversibleTransfers( +/// pallet_reversible_transfers::Call::schedule_transfer { .. } +/// ) +/// ) +/// } +/// +/// fn guardian(who: &AccountId) -> Option { +/// pallet_reversible_transfers::Pallet::::get_guardian(who) +/// } +/// } +/// ``` +pub trait HighSecurityInspector { + /// Check if an account is designated as High-Security + /// + /// High-Security accounts are restricted to executing only whitelisted calls + /// and have a guardian that can intercept malicious transactions. + /// + /// # Parameters + /// + /// - `who`: The account to check + /// + /// # Returns + /// + /// `true` if the account is High-Security, `false` otherwise + fn is_high_security(who: &AccountId) -> bool; + + /// Check if a runtime call is whitelisted for High-Security accounts + /// + /// The whitelist is typically defined at the runtime level and includes only + /// operations that are reversible or delayed (e.g., scheduled transfers). + /// + /// # Parameters + /// + /// - `call`: The runtime call to validate + /// + /// # Returns + /// + /// `true` if the call is whitelisted, `false` otherwise + /// + /// # Implementation Notes + /// + /// The runtime-level implementation typically uses pattern matching on `RuntimeCall`: + /// + /// ```ignore + /// matches!( + /// call, + /// RuntimeCall::ReversibleTransfers( + /// pallet_reversible_transfers::Call::schedule_transfer { .. } + /// ) | RuntimeCall::ReversibleTransfers( + /// pallet_reversible_transfers::Call::cancel { .. } + /// ) + /// ) + /// ``` + fn is_whitelisted(call: &RuntimeCall) -> bool; + + /// Get the guardian/interceptor account for a High-Security account + /// + /// The guardian has special privileges to cancel pending transactions + /// initiated by the High-Security account. + /// + /// # Parameters + /// + /// - `who`: The High-Security account + /// + /// # Returns + /// + /// `Some(guardian_account)` if the account has a guardian, `None` otherwise + fn guardian(who: &AccountId) -> Option; + + // NOTE: No benchmarking-specific methods in the trait! + // Production API should not be polluted by test/benchmark requirements. + // Use pallet-specific helpers instead (e.g., + // pallet_reversible_transfers::Pallet::add_high_security_benchmark_account) +} + +/// Default implementation for `HighSecurityInspector` +/// +/// This implementation disables all High-Security checks, allowing any call to execute. +/// It's useful for: +/// - Test environments that don't need HS enforcement +/// - Pallets that want optional HS support via `type HighSecurity = ();` +/// - Gradual feature rollout +/// +/// # Behavior +/// +/// - `is_high_security()`: Always returns `false` +/// - `is_whitelisted()`: Always returns `true` (allow everything) +/// - `guardian()`: Always returns `None` +impl HighSecurityInspector for () { + fn is_high_security(_who: &AccountId) -> bool { + false + } + + fn is_whitelisted(_call: &RuntimeCall) -> bool { + true // Allow everything if no High-Security enforcement + } + + fn guardian(_who: &AccountId) -> Option { + None + } +} diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index fe292af7..cda93027 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -49,6 +49,7 @@ pallet-utility.workspace = true primitive-types.workspace = true qp-dilithium-crypto.workspace = true qp-header = { workspace = true, features = ["serde"] } +qp-high-security = { path = "../primitives/high-security", default-features = false } qp-poseidon = { workspace = true, features = ["serde"] } qp-scheduler.workspace = true scale-info = { features = ["derive", "serde"], workspace = true } @@ -114,6 +115,7 @@ std = [ "qp-dilithium-crypto/full_crypto", "qp-dilithium-crypto/std", "qp-header/std", + "qp-high-security/std", "qp-poseidon/std", "qp-scheduler/std", "scale-info/std", diff --git a/runtime/src/configs/mod.rs b/runtime/src/configs/mod.rs index a439621c..5b7f9776 100644 --- a/runtime/src/configs/mod.rs +++ b/runtime/src/configs/mod.rs @@ -577,7 +577,64 @@ parameter_types! { pub const MaxExpiryDuration: BlockNumber = 100_800; // ~2 weeks at 12s blocks (14 days * 24h * 60m * 60s / 12s) } -/// Whitelist for calls that can be proposed in multisigs +/// High-Security configuration wrapper for Runtime +/// +/// This type alias delegates to `ReversibleTransfers` pallet for high-security checks +/// and adds RuntimeCall-specific whitelist validation. +/// +/// Used by: +/// - Multisig pallet: validates calls in `propose()` extrinsic +/// - Transaction extensions: validates calls for high-security EOAs +/// +/// Whitelist includes only delayed, reversible operations: +/// - `schedule_transfer`: Schedule delayed native token transfer +/// - `schedule_asset_transfer`: Schedule delayed asset transfer +/// - `cancel`: Cancel pending delayed transfer +pub struct HighSecurityConfig; + +impl qp_high_security::HighSecurityInspector for HighSecurityConfig { + fn is_high_security(who: &AccountId) -> bool { + // Delegate to reversible-transfers pallet + pallet_reversible_transfers::Pallet::::is_high_security_account(who) + } + + fn is_whitelisted(call: &RuntimeCall) -> bool { + #[cfg(feature = "runtime-benchmarks")] + { + // Production whitelist + remark for propose_high_security benchmark + matches!( + call, + RuntimeCall::ReversibleTransfers( + pallet_reversible_transfers::Call::schedule_transfer { .. } + ) | RuntimeCall::ReversibleTransfers( + pallet_reversible_transfers::Call::schedule_asset_transfer { .. } + ) | RuntimeCall::ReversibleTransfers( + pallet_reversible_transfers::Call::cancel { .. } + ) | RuntimeCall::System(frame_system::Call::remark { .. }) + ) + } + + #[cfg(not(feature = "runtime-benchmarks"))] + { + matches!( + call, + RuntimeCall::ReversibleTransfers( + pallet_reversible_transfers::Call::schedule_transfer { .. } + ) | RuntimeCall::ReversibleTransfers( + pallet_reversible_transfers::Call::schedule_asset_transfer { .. } + ) | RuntimeCall::ReversibleTransfers( + pallet_reversible_transfers::Call::cancel { .. } + ) + ) + } + } + + fn guardian(who: &AccountId) -> Option { + // Delegate to reversible-transfers pallet + pallet_reversible_transfers::Pallet::::get_guardian(who) + } +} + impl pallet_multisig::Config for Runtime { type RuntimeCall = RuntimeCall; type Currency = Balances; @@ -592,6 +649,7 @@ impl pallet_multisig::Config for Runtime { type MaxExpiryDuration = MaxExpiryDuration; type PalletId = MultisigPalletId; type WeightInfo = pallet_multisig::weights::SubstrateWeight; + type HighSecurity = HighSecurityConfig; } impl TryFrom for pallet_balances::Call { diff --git a/runtime/src/genesis_config_presets.rs b/runtime/src/genesis_config_presets.rs index ff91a2cd..61f71c56 100644 --- a/runtime/src/genesis_config_presets.rs +++ b/runtime/src/genesis_config_presets.rs @@ -83,6 +83,46 @@ pub fn development_config_genesis() -> Value { log::info!("🍆 Endowed account raw: {:?}", account); } + #[cfg(feature = "runtime-benchmarks")] + { + use crate::Runtime; + use frame_benchmarking::v2::{account, whitelisted_caller}; + use pallet_multisig::Pallet as Multisig; + + const SEED: u32 = 0; + let caller = whitelisted_caller::(); + let signer1 = account::("signer1", 0, SEED); + let signer2 = account::("signer2", 1, SEED); + let mut signers = vec![caller, signer1, signer2]; + signers.sort(); + let multisig_address = Multisig::::derive_multisig_address(&signers, 2, 0); + let interceptor = crystal_alice().into_account(); + let delay = 10u32; + + let rt_genesis = pallet_reversible_transfers::GenesisConfig:: { + initial_high_security_accounts: vec![(multisig_address, interceptor, delay)], + }; + + let config = RuntimeGenesisConfig { + balances: BalancesConfig { + balances: endowed_accounts + .iter() + .cloned() + .map(|k| (k, 100_000 * UNIT)) + .chain([( + TreasuryPalletId::get().into_account_truncating(), + 21_000_000 * 30 * UNIT / 100, + )]) + .collect::>(), + }, + sudo: SudoConfig { key: Some(crystal_alice().into_account()) }, + reversible_transfers: rt_genesis, + ..Default::default() + }; + return serde_json::to_value(config).expect("Could not build genesis config."); + } + + #[cfg(not(feature = "runtime-benchmarks"))] genesis_template(endowed_accounts, crystal_alice().into_account()) } diff --git a/runtime/src/transaction_extensions.rs b/runtime/src/transaction_extensions.rs index 5afed1e6..63d27871 100644 --- a/runtime/src/transaction_extensions.rs +++ b/runtime/src/transaction_extensions.rs @@ -5,6 +5,7 @@ use core::marker::PhantomData; use frame_support::pallet_prelude::{InvalidTransaction, ValidTransaction}; use frame_system::ensure_signed; +use qp_high_security::HighSecurityInspector; use scale_info::TypeInfo; use sp_core::Get; use sp_runtime::{traits::TransactionExtension, Weight}; @@ -65,25 +66,15 @@ impl ) })?; - if ReversibleTransfers::is_high_security(&who).is_some() { - // High-security accounts can only call schedule_transfer and cancel - match call { - RuntimeCall::ReversibleTransfers( - pallet_reversible_transfers::Call::schedule_transfer { .. }, - ) | - RuntimeCall::ReversibleTransfers( - pallet_reversible_transfers::Call::schedule_asset_transfer { .. }, - ) | - RuntimeCall::ReversibleTransfers(pallet_reversible_transfers::Call::cancel { - .. - }) => { - return Ok((ValidTransaction::default(), (), origin)); - }, - _ => { - return Err(frame_support::pallet_prelude::TransactionValidityError::Invalid( - InvalidTransaction::Custom(1), - )); - }, + // Check if account is high-security using the same inspector as multisig + if crate::configs::HighSecurityConfig::is_high_security(&who) { + // Use the same whitelist check as multisig + if crate::configs::HighSecurityConfig::is_whitelisted(call) { + return Ok((ValidTransaction::default(), (), origin)); + } else { + return Err(frame_support::pallet_prelude::TransactionValidityError::Invalid( + InvalidTransaction::Custom(1), + )); } } @@ -204,7 +195,11 @@ mod tests { assert_ok!(result); // All other calls are disallowed for high-security accounts - let call = RuntimeCall::System(frame_system::Call::remark { remark: vec![1, 2, 3] }); + // (use transfer_keep_alive - not in whitelist for prod or runtime-benchmarks) + let call = RuntimeCall::Balances(pallet_balances::Call::transfer_keep_alive { + dest: MultiAddress::Id(bob()), + value: 10 * EXISTENTIAL_DEPOSIT, + }); let result = check_call(call); assert_eq!( result.unwrap_err(),