Add controller_transfer_to - #2001
Open
HenriqueNogara wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
It introduces new extrinsics/ABI but leaves generated artifacts and weights in a likely-stale/placeholder state (and lacks runtime tests for the new controller_transfer_to path).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends Polymesh’s forced/controller transfer and freezing capabilities by adding destination-addressable controller transfers (controller_transfer_to) for both fungible assets and NFTs, and by introducing incremental freeze/unfreeze operations for fungible assets (including EVM precompile surface-area updates).
Changes:
- Added
controller_transfer_toextrinsics (asset + NFT) and updated the fungible-asset precompileforcedTransferflow to target an explicit destination. - Added
freeze_partial_tokens/unfreeze_partial_tokensextrinsics and exposed them via the ERC-3643 precompile (plus integration tests). - Adjusted controller-transfer unfreeze behavior for frozen balances and added runtime tests; added new benchmarks/weights entries (with two weights currently left as placeholders).
File summaries
| File | Description |
|---|---|
| precompiles/src/interfaces/FungibleAssetStub.sol | Updates Solidity stub ABI: forcedTransfer(from,to,amount) and adds partial freeze/unfreeze funcs + events. |
| pallets/weights/src/pallet_nft.rs | Adds weight for controller_transfer_to(n). |
| pallets/weights/src/pallet_asset.rs | Adds weights for new extrinsics; partial freeze/unfreeze weights currently marked as placeholders. |
| pallets/runtime/tests/src/asset_pallet/controller_transfer.rs | Adds tests for controller-transfer freeze/unfreeze shortfall behavior and refactors agent setup helper. |
| pallets/precompiles/src/interface/fungible_asset/mod.rs | Routes new ABI calls to the new precompile handlers. |
| pallets/precompiles/src/interface/fungible_asset/erc7943.rs | Updates forced_transfer to accept to and dispatch controller_transfer_to. |
| pallets/precompiles/src/interface/fungible_asset/erc3643.rs | Adds precompile handlers for partial freeze/unfreeze and emits EVM-side events. |
| pallets/nft/src/lib.rs | Adds controller_transfer_to extrinsic + base implementation and a new error for receiver affirmation requirement. |
| pallets/nft/src/benchmarking.rs | Adds benchmark for controller_transfer_to. |
| pallets/asset/src/lib.rs | Adds controller_transfer_to, partial freeze/unfreeze extrinsics, new errors, and updates controller-transfer unfreeze logic. |
| pallets/asset/src/benchmarking.rs | Adds benchmarks for controller_transfer_to, partial freeze, and partial unfreeze. |
| integration/tests/utility_calls.rs | Formatting-only changes. |
| integration/tests/sto.rs | Formatting-only changes. |
| integration/tests/statistics_enforcement.rs | Formatting-only changes. |
| integration/tests/settlement_venues.rs | Formatting-only changes. |
| integration/tests/settlement_scheduling.rs | Formatting-only changes. |
| integration/tests/settlement_mediators.rs | Formatting-only changes. |
| integration/tests/revive_erc721.rs | Formatting-only changes. |
| integration/tests/revive_erc3643.rs | Adds integration tests for partial freeze/unfreeze via the ERC-3643 precompile. |
| integration/tests/relayer_negative.rs | Formatting-only changes. |
| integration/tests/portfolio_custody.rs | Formatting-only changes. |
| integration/tests/nft.rs | Formatting-only changes. |
| integration/tests/identity_lifecycle.rs | Formatting-only changes. |
| integration/tests/economics.rs | Formatting-only changes. |
| integration/tests/corporate_ballot.rs | Formatting-only changes. |
| integration/tests/compliance_enforcement.rs | Formatting-only changes. |
| integration/tests/checkpoints.rs | Formatting-only changes. |
| integration/tests/capital_distribution.rs | Formatting-only changes. |
| integration/tests/ca_extended.rs | Formatting-only changes. |
| integration/tests/asset_controls.rs | Formatting-only changes. |
| integration/src/lib.rs | Minor pattern-match refactor. |
| integration/src/erc20_helper.rs | Adds ERC-3643 helper methods for partial freeze/unfreeze calls. |
Review details
Suppressed comments (1)
pallets/weights/src/pallet_asset.rs:963
- Same issue for
unfreeze_partial_tokens: the weight is a TODO placeholder despite a benchmark being added inpallets/asset/src/benchmarking.rs. Please regenerate the weights output so this extrinsic’s weight reflects the actual benchmark results.
// TODO: Placeholder weight copied from `set_frozen_tokens`, pending its own benchmark run.
fn unfreeze_partial_tokens() -> Weight {
// Minimum execution time: 62_991 nanoseconds.
Weight::from_parts(64_868_000, 0)
.saturating_add(DbWeight::get().reads(6))
.saturating_add(DbWeight::get().writes(1))
}
- Files reviewed: 32/33 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+2532
to
+2536
| Self::deposit_event(Event::ControllerTransfer( | ||
| caller_data.primary_did, | ||
| asset_id, | ||
| source, | ||
| transfer_value, |
Comment on lines
+127
to
+133
| #[test] | ||
| fn controller_transfer_within_free_balance_does_not_unfreeze() { | ||
| ExtBuilder::default().build().execute_with(|| { | ||
| let bob = User::new(Sr25519Keyring::Bob); | ||
| let alice = User::new(Sr25519Keyring::Alice); | ||
| let alice_default_portfolio = PortfolioId::default_portfolio(alice.did); | ||
| let alice_holder = AssetHolder::from(alice_default_portfolio.clone()); |
Comment on lines
+950
to
+956
| // TODO: Placeholder weight copied from `set_frozen_tokens`, pending its own benchmark run. | ||
| fn freeze_partial_tokens() -> Weight { | ||
| // Minimum execution time: 62_991 nanoseconds. | ||
| Weight::from_parts(64_868_000, 0) | ||
| .saturating_add(DbWeight::get().reads(6)) | ||
| .saturating_add(DbWeight::get().writes(1)) | ||
| } |
Comment on lines
+180
to
+186
| /// @notice Takes tokens from one address and transfers them to another. | ||
| /// @dev Requires specific authorization. Used for regulatory compliance or recovery scenarios. | ||
| /// @param from The address from which `amount` is taken. | ||
| /// @param to The address which receives the seized tokens. | ||
| /// @param amount The amount to force transfer. | ||
| /// @return True if the transfer executed correctly. Reverts on failure. | ||
| function forcedTransfer(address from, uint256 amount) external returns (bool); | ||
| function forcedTransfer(address from, address to, uint256 amount) external returns (bool); |
Comment on lines
+180
to
+186
| /// @notice Takes tokens from one address and transfers them to another. | ||
| /// @dev Requires specific authorization. Used for regulatory compliance or recovery scenarios. | ||
| /// @param from The address from which `amount` is taken. | ||
| /// @param to The address which receives the seized tokens. | ||
| /// @param amount The amount to force transfer. | ||
| /// @return True if the transfer executed correctly. Reverts on failure. | ||
| function forcedTransfer(address from, uint256 amount) external returns (bool); | ||
| function forcedTransfer(address from, address to, uint256 amount) external returns (bool); |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
changelog
new features
controller_transfer_to,freeze_partial_tokensandunfreeze_partial_tokens;