proxy: migrate from Currency to fungible traits #9560
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit migrates the proxy pallet from the deprecated Currency trait to modern fungible traits with holds, addressing the balance overlap issue where proxy reserves couldn't overlap with staking holds. Changes: - Added HoldReason enum with ProxyDeposit and AnnouncementDeposit variants - Updated Config trait to use fungible::hold::Mutate and related traits - Replaced all Currency::reserve calls with hold operations - Replaced all Currency::unreserve calls with release operations - Updated rejig_deposit function to handle hold/release with proper reason - Fixed tests to work with new hold-based balance model This enables proper balance overlap: users now only need max(staking_hold, governance_lock) + proxy_hold total balance instead of staking_hold + governance_lock + proxy_reserve.
Implements multi-block migration with graceful degradation to convert proxy and announcement reserves to holds during runtime upgrade. - Add migration cursor for multi-block execution - Convert reserves to holds with fallback to proxy removal + refund - Add migration events for user communication - Include comprehensive pure proxy recovery documentation
Replace Currency trait usage in benchmarks with fungible equivalents: - Use mint_into() instead of make_free_balance_be() to add balance without destroying existing holds - Replace reserved_balance() with balance_on_hold() for specific hold reasons - Replace reserve() with hold() operations using proper HoldReason variants - Fix add_announcements helper to avoid overwriting caller balance when provided - Add proper real account parameter to announcement benchmarks for correct proxy relationships - Use appropriate balance amounts to prevent overflow in test runtime The key difference is that holds come from total balance (reducing free balance), while the old reserves were separate from free balance.
sigurpol
commented
Aug 29, 2025
Pure proxy accounts can become permanently inaccessible when they lose their proxy configuration but retain funds in a keyless account. This creates an unrecoverable state where funds are locked forever. Add four Root-origin force functions to enable governance recovery: - force_add_proxy: restore proxy relationships for inaccessible accounts - force_remove_proxy: remove specific proxy relationships - force_remove_proxies: clear all proxies and release deposits - force_remove_announcements: clear announcements and release deposits These functions provide emergency intervention capabilities for edge cases where normal proxy management fails, ensuring no funds become permanently locked while maintaining security through Root-only access.
kianenigma
reviewed
Aug 29, 2025
kianenigma
reviewed
Aug 29, 2025
…ransfer and update documentation
Refactors proxy pallet v2 migration from OnRuntimeUpgrade to SteppedMigration to integrate with pallet_migrations framework for proper multi-block execution. - Implement SteppedMigration trait with cursor-based batching - Add migration to Asset Hub Westend runtime configuration
|
/cmd bench --pallet pallet_proxy --runtime dev |
|
Command "bench --pallet pallet_proxy --runtime dev" has started 🚀 See logs here |
…t_proxy --runtime dev'
|
Command "bench --pallet pallet_proxy --runtime dev" has finished ✅ See logs here Subweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
…t_staking_async --runtime asset-hub-westend'
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
|
/cmd bench --pallet pallet_proxy --runtime westend |
|
/cmd bench --pallet pallet_proxy --runtime asset-hub-westend |
|
Command "bench --pallet pallet_proxy --runtime westend" has started 🚀 See logs here |
|
Command "bench --pallet pallet_proxy --runtime asset-hub-westend" has started 🚀 See logs here |
sigurpol
commented
Sep 15, 2025
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_staking_async.rs
Outdated
Show resolved
Hide resolved
…t_proxy --runtime westend'
|
Command "bench --pallet pallet_proxy --runtime westend" has finished ✅ See logs here Subweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
…t_proxy --runtime asset-hub-westend'
|
Command "bench --pallet pallet_proxy --runtime asset-hub-westend" has finished ✅ See logs here Subweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
…t_staking_async --runtime asset-hub-westend'
…et pallet_staking_async --runtime asset-hub-westend'" This reverts commit 485dc05.
rockbmb
added a commit
to open-web3-stack/polkadot-ecosystem-tests
that referenced
this pull request
Sep 27, 2025
* Init balances e2e tests (wip) * Rework initial tests for clarity Most of them were written with recourse to Cursor, and were just not very useful. * Refactor `transfer_allow_death` test It scrutinizes the extrinsic's events and their inner data. * Recreate tests for all Polkadot/Kusama relay/SPs * Rework networks that run `transferAllowDeath` test Now, at the calling site of the test tree creation function, chains can signal whether their ED is too low to run the basic `transferAllowDeath` test. * Scrutinize more events in `transferAllowDeath` test * Test non-destructive `transfer_allow_death` * Test `force_transfer` with account reaping * Test to `force_transfer` reaping * Test `transfer_allow_death` below ED * Test `force_transfer` below ED * Test overtransferring account * Update comments and snapshots * Test `force_transfer` with insufficient funds * Test self `transfer_allow_death` of near-entire balance * Update snapshots * Update accounts tests after merge from `master` PR #384 required some additional fixes that auto-merge could not perform. * Update snapshots * Fund new test account on Bridge Hub * Revert change to Polkadot AH multisig suite name * Create test to liquidity restrictions bug See paritytech/polkadot-sdk#9560 and paritytech/polkadot-sdk#8108. TL;DR proxy and multisig pallets are using the old `Currency` trait and not the new fungible traits, it is possible for an operation that is permissible to an account at a given point in time and state to fail. See #401 * Test liquidity restrictions when creating proxy * Parametrize deposit-requiring action liquidity restriction test The `balances.LiquidityRestrictions` error can occur is different contexts, as multiple pallets still use the old `Currency::reserve` operation. This commit parametrizes the test to allow for different kinds of actions to be tested, not just proxy addition. * Extend liquidity restriction tests to cover more cases The test now combinatorially covers several cases: 1. Reserve action can vary between staking bond and nomination pool creation 2. Lock action is always vested transfer (for now) 3. Deposit action can vary between proxy creation, referendum submission and staking bond * Test manual reserve/lock in liquidity restriction tests This adds the possibility of manually set reserve/locks to the reserve/ lock actions used to combinatorially generate tests to liquidity restrictions. This is needed because some chains have no vesting or staking, but the behavior should still be tested. * Fix manual reserve/lock actions in liq. restriction tests Fees were not being accounted for correctly when performing checks on free/frozen balances. * Update snapshots and accounts E2E test tree call sites * Filter vesting as locking action on Asset Hubs ... while the AHM is pending - vesting operations are filtered, and thus cannot be used. * Add more comments to liquidity restriction test Especially the action interfaces. * Add more comments to liquidity restriction tests * Remove leftover debug code from self-transfer test * Fix Asset Hub test suites' use of relay chain An invalid argument was being passed to the `scheduleInlineCallWithOrigin` function: the base's chains block provider, instead of the relay chain's, which is always `Local`. * Snapshot skipped liquidity restriction tests ... instead of logging a message. Logs add some noise to CI test output. * Apply lint fixes * Update block numbers * Change lock identifier used * Remove unstable `Transfer` event from snapshots * Update snapshots * Simplify liquidity restriction tests The `Balance.locks` storage is no longer modified - Polkadot Relay is failing for an undetermined reason. * Use nonces when setting storage * Test `transfer_allow_death` with reserve * Add test to `transfer_all` with reserve * Add `force_transfer` with reserve test * Add origin check tests for gated extrinsics * Add tests to transfer functions with insufficient funds * Test `transfer_all` with `keepAlive = true` * Correct previous transfer all test snapshot * Test `transfer_all` with `keep_alive = false` * Correct snapshots of accounts tests * Test `transfer_keep_alive` with source left below ED * Split `transfer_keep_alive` tests ... into two separate tests: 1. one for chains with low ED (below a typical transfer fee) 2. one for chains with normal ED * Test `force_adjust_total_issuance` with zero delta * Test total issuance changes * Test `force_unreserve` no-op cases * Update accounts E2E test snapshots * Update liquidity restriction test snapshots for AHs * Test forceful unreservation of funds * Add more self-transfer tests * Add self-transfer test to `force_transfer` * Add tests to `force_set_balance` * Refactor `burn` tests into low/normal ED tests * Update snapshots with new `burn` tests * Test burning from account with consumer reference * Add test to burning of entire balance (or more than it) * Update snapshots to remove obsolete data * Improve documentation and `README`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR migrates the proxy pallet from the Currency trait to the fungible traits with holds, addressing the balance overlap issue where proxy reserves couldn't overlap with staking holds.
Changes
This enables proper balance overlap: users now only need
max(staking_hold, governance_lock,proxy_hold)total balance instead ofstaking_hold + governance_lock + proxy_reserve.Migration: Conversion of proxy and announcement reserves into holds.
This migration uses multi-block execution with proxy preservation:
Testing migration
Beside new unit tests added in the proxy pallet, I have tested the migration via
try-runtime-clion Westend AH.There, we have ~1000 accounts with an incorrect state (proxies configured with deposit and zero reserved funds before migrations), for them - as expected - the migration preserves the proxy relationship and set deposits to 0, as explained above.
Similar results when tested against Westend RC.
The expectation on Kusama and Polkadot is to have a decent clear state.
Further test via chopsticks / PET need to confirm that's the case.
Below some logs for WAH case
Benchmarking
Replace Currency trait usage in benchmarks with fungible equivalents:
The key difference is that holds come from total balance (reducing free balance), while the old reserves were separate from free balance.
Weights
Looking at benchmark results, there is a significant weight increase (~40-100%) when moving from reserves to holds, which is expected and can be explained by the fact that holds are more complex than reserves (e.g. for storage:
Reserves-> singleBalancefield per account inReservedBalancebalance vsHolds-> BTreeMap<HoldReason, Balance> in Holds storage). The most affected operations arecreate_pure,add_proxyandremove_proxybecause they rely on hold creation / destruction patterns now. I think the UX improvement is enough to justify the performance cost, especially because proxy addition / removal happens rarely in comparison with proxy execution.Resources
This is part of the umbrella task #226.
More details here
TODO