Implement spender approvals and transfers by tgntr · Pull Request #1903 · PolymeshAssociation/Polymesh

tgntr · 2026-03-17T13:46:00Z

Implements spender approval primitives for the asset pallet and adds a transfer_funds extrinsic to the settlement pallet for spending approved allowances and same-identity direct transfers.

changelog

new features

Added spender allowance system — owners can approve other accounts to transfer fungible assets on their behalf
Added transfer_funds to the settlement pallet — supports same-identity direct transfers and spender-approved transfers without going through the settlement engine
Same-identity transfers for transfer_asset
Added transfer_nft to the NFT pallet — transfer NFTs between accounts/portfolios with same routing logic as transfer_funds

modified external API

transfer_asset now uses ensure_origin_call_permissions and routes same-DID transfers directly instead of always creating a settlement instruction

new external API

Added approve extrinsic to the asset pallet — set, replace, or revoke a spender allowance for a given asset
Added transfer_funds extrinsic to the settlement pallet — transfer assets with optional spender approval, defaults source to caller's account when from is None
Added allowance(owner, spender, asset_id) Runtime API — query remaining spender allowance without a transaction
Added transfer_nft extrinsic to the NFT pallet — transfer NFTs with same-DID direct / cross-DID settlement routing

new events

Added Approval { owner, spender, asset_id, amount } event — emitted on approve calls, not on spend

other

Added Allowances storage map to the asset pallet — maps (owner, spender, asset_id) to approved balance
Added InsufficientAllowance error to the asset pallet
Added SenderSameAsReceiver and AllowancesNotSupportedForNFTs errors to the settlement pallet
Added spend_allowance public helper to the asset pallet — used by the settlement pallet to check and decrement allowances
Infinite allowance (Balance::MAX) is never decremented on spend; allowance depleted to zero removes the storage entry
Added tests for approve, allowance Runtime API, and transfer_funds covering spender mode, same-identity transfers, cross-identity settlement, portfolio custody, NFT transfers, atomicity, and edge cases
Added PortfolioFnTrait / PortfolioFnConfig
Added transfer_funds / transfer_funds_weight to SettlementFnTrait — exposes routing logic for cross-pallet callers
simplified_nft_transfer now accepts Option<InstructionId> (was required)
Removed dead code: transfer_asset_base_weight benchmark, transfer_asset_and_try_execute, transfer_nft_and_try_execute

pallets/asset/src/lib.rs

Neopallium · 2026-03-18T12:08:32Z

pallets/asset/src/lib.rs

+            spender: T::AccountId,
+            amount: Balance,
+        ) -> DispatchResult {
+            let caller_data = IdentityPallet::<T>::ensure_origin_call_permissions(origin)?;


I don't think we should be doing call permissions here. When account keys hold the assets only that key has the authority over those assets.

Based on our discussion we will keep using call permissions here. Which means we should change Asset.transfer_asset to also use call permissions.

pallets/settlement/src/lib.rs

Neopallium · 2026-03-27T12:23:57Z

pallets/asset/src/lib.rs

+    /// Maps (owner, spender, asset_id) to the approved allowance amount.
+    ///
+    /// A non-existent entry returns 0 (via `ValueQuery`), matching ERC-20 behavior.
+    /// When an allowance is revoked (set to 0), the entry is removed to bound storage growth.
+    #[pallet::storage]
+    pub type Allowances<T: Config> =
+        StorageMap<_, Blake2_128Concat, (T::AccountId, T::AccountId, AssetId), Balance, ValueQuery>;


This should be a double map, so that the owner can easily query all current allowances.

Could even use a NMap.

pallets/settlement/src/benchmarking.rs

Neopallium · 2026-03-27T13:46:59Z

pallets/settlement/src/benchmarking.rs

+        let from = Some(AssetHolder::try_from(alice.account().encode()).unwrap());
+        let to = AssetHolder::try_from(bob.account().encode()).unwrap();


Using accounts instead of portfolios will be cheaper (no custodianship checks).

Neopallium · 2026-03-27T13:51:46Z

pallets/nft/src/lib.rs

+        let _caller_data = IdentityPallet::<T>::ensure_origin_call_permissions(origin.clone())?;
+
+        let fund = Fund {
+            description: FundDescription::NonFungible(nfts),
+            memo,
+        };
+
+        let (_instruction_id, consumed) =
+            <T as pallet_asset::Config>::SettlementFn::transfer_funds(
+                origin,


The call permission check here is a duplicate, since it is checked in transfer_funds we don't need it here.
The pallet/extrinsic names are set before the extrinsic is called, the permissions checks are not based on the fn name.

Suggested change

let _caller_data = IdentityPallet::<T>::ensure_origin_call_permissions(origin.clone())?;

let fund = Fund {

description: FundDescription::NonFungible(nfts),

memo,

};

let (_instruction_id, consumed) =

<T as pallet_asset::Config>::SettlementFn::transfer_funds(

origin,

let fund = Fund {

description: FundDescription::NonFungible(nfts),

memo,

};

let (_instruction_id, consumed) =

<T as pallet_asset::Config>::SettlementFn::transfer_funds(

origin,

Neopallium · 2026-03-27T13:55:17Z

pallets/asset/src/lib.rs

-        let mut weight_meter = <T as Config>::SettlementFn::transfer_and_try_execute_weight_meter(
-            <T as Config>::WeightInfo::transfer_asset_base_weight(),
-            true,
-        );

-        // Create the transfer instruction via the settlement engine and affirm it as the sender.
-        let instruction_id = T::SettlementFn::transfer_asset_and_try_execute(


Since this method emits an event, we should still create the weight meter here. Also since this code path only uses account holder (no portfolios), it's worse-case weight should be lower.

Neopallium · 2026-03-27T14:38:22Z

pallets/settlement/src/lib.rs

+        // In spender mode (sender != caller), pass None to skip the caller's secondary key
+        // check and instead verify the sender's account via their DID's primary key.
+        let sender_sk = if from_did == origin_did {
+            origin_data.secondary_key.as_ref()
+        } else {
+            None
+        };
+
        // Affirm the instruction on behalf of the sender.
-        Self::unsafe_affirm_instruction(
-            origin_data.primary_did,
-            instruction_id,
-            [from].into(),
-            origin_data.secondary_key.as_ref(),
-            None,
-        )?;
+        Self::unsafe_affirm_instruction(from_did, instruction_id, [from].into(), sender_sk, None)?;


This is very confusing. The spend_allowance check is outside of this function and it is not clear how that check matches up with the logic here.

Please move the spender/portfolio check into a function that can be called here for the "different DIDs, use settlement" case.

Neopallium · 2026-03-27T14:48:31Z

pallets/settlement/src/lib.rs

@@ -3888,9 +4069,17 @@ impl<T: Config> SettlementFnTrait<T> for Pallet<T> {
        weight_meter: &mut WeightMeter,
        #[cfg(feature = "runtime-benchmarks")] bench_base_weight: bool,
    ) -> Result<Option<InstructionId>, DispatchErrorWithPostInfo> {


I think this is dead code now.

Neopallium · 2026-03-27T15:49:50Z

pallets/settlement/src/lib.rs

+            Self::base_transfer_and_try_execute(
+                origin,
+                Some(resolved_from),
+                Some(from_did),
+                to,
+                Some(to_did),


I think this is the only call to that function, so don't need optional parameters.

Neopallium · 2026-03-27T15:51:30Z

pallets/settlement/src/lib.rs

+            AssetHolder::Portfolio(ref portfolio_id) => {
+                T::PortfolioFn::ensure_portfolio_custody(portfolio_id, caller_did)?;
+            }
+        }


Please move this spender/custody check into a function, so it can be called in base_transfer_and_try_execute or inside the "Same-identity: transfer directly" block below.

Also I think we only need to do the spender check if from.is_some().

Neopallium · 2026-03-31T10:16:38Z

pallets/weights/src/pallet_settlement.rs

+    // TODO: Replace with generated weights after benchmark run.
+    // Based on base_transfer (21r/5w) + origin check (1r) + allowance (1r/1w) + DID resolution (2r).
+    fn transfer_funds() -> Weight {


Please run new benchmarks locally (use a placeholder weight function during the first build/run), since the benchmarks will record storage read/writes.

Neopallium · 2026-03-31T10:17:55Z

pallets/settlement/src/benchmarking.rs

        let alice = UserBuilder::<T>::default().generate_did().build("Alice");
    }: _(alice.origin, AffirmationRequirement::Required)
+
+    transfer_funds {


I think we will need to split this benchmark/weight function into multiple functions.

Benchmarks for the main code paths (with receiver affirmation, no execution):

sender or receiver is a portfolio, same DID

sender or receiver is a portfolio, different DID

sender and receiver accounts, same DID

sender and receiver accounts, different DID

Also a benchmark to cover (

My last comment got cut off.

Also a benchmark to cover the try execution logic.

tgntr added 4 commits March 17, 2026 10:03

implement spender approvals

ebd64a5

implement transfers

e827244

update ts types

041ec21

improve readability

aa978d4

tgntr requested review from HenriqueNogara and Neopallium March 17, 2026 13:46

tgntr added 2 commits March 17, 2026 16:13

refactor

60b2e6f

Merge branch 'develop' into MESH-2299-erc20-spender-approvals

1cd0f5d

Neopallium requested changes Mar 18, 2026

View reviewed changes

address comments + cross-identity transfers

9a70642

tgntr requested a review from Neopallium March 19, 2026 11:28

tgntr added 3 commits March 20, 2026 14:52

add portfolio check, refactor transfer_asset

370426e

add nft transfers, remove dead code, add tests

8b61b77

remove redundant ensure_origin_call_permissions

70d2799

Neopallium requested changes Mar 27, 2026

View reviewed changes

Neopallium reviewed Mar 31, 2026

View reviewed changes

address PR comments

0a2a614

tgntr requested a review from Neopallium April 1, 2026 10:21

fix lint

6fa1409

		let from = Some(AssetHolder::try_from(alice.account().encode()).unwrap());
		let to = AssetHolder::try_from(bob.account().encode()).unwrap();

Conversation

tgntr commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

changelog

new features

modified external API

new external API

new events

other

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tgntr commented Mar 17, 2026 •

edited

Loading