-
Notifications
You must be signed in to change notification settings - Fork 52
Implement spender approvals and transfers #1903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 10 commits
ebd64a5
e827244
041ec21
aa978d4
60b2e6f
1cd0f5d
9a70642
370426e
8b61b77
70d2799
0a2a614
6fa1409
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,6 +117,7 @@ use polymesh_primitives::asset_metadata::{ | |
| AssetMetadataSpec, AssetMetadataValue, AssetMetadataValueDetail, | ||
| }; | ||
| use polymesh_primitives::constants::*; | ||
| use polymesh_primitives::portfolio::{Fund, FundDescription}; | ||
| use polymesh_primitives::protocol_fee::{ChargeProtocolFee, ProtocolOp}; | ||
| use polymesh_primitives::settlement::InstructionId; | ||
| use polymesh_primitives::traits::{ | ||
|
|
@@ -344,6 +345,13 @@ pub mod pallet { | |
| memo: Option<Memo>, | ||
| pending_transfer_id: Option<InstructionId>, | ||
| }, | ||
| /// A spender allowance was set for an asset. | ||
| Approval { | ||
| owner: T::AccountId, | ||
| spender: T::AccountId, | ||
| asset_id: AssetId, | ||
| amount: Balance, | ||
| }, | ||
| } | ||
|
|
||
| /// Map each [`Ticker`] to its registration details ([`TickerRegistration`]). | ||
|
|
@@ -593,6 +601,14 @@ pub mod pallet { | |
| ValueQuery, | ||
| >; | ||
|
|
||
| /// 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>; | ||
|
|
||
| /// Storage version. | ||
| #[pallet::storage] | ||
| pub type StorageVersion<T: Config> = StorageValue<_, Version, ValueQuery>; | ||
|
|
@@ -1638,7 +1654,7 @@ pub mod pallet { | |
| /// * `UnexpectedOFFChainAsset` - If the asset could not be found on-chain. | ||
| /// * `MissingIdentity` - The caller doesn't have an identity. | ||
| #[pallet::call_index(34)] | ||
| #[pallet::weight(<T as Config>::SettlementFn::transfer_and_try_execute_weight_meter(<T as Config>::WeightInfo::transfer_asset_base_weight(), true).limit())] | ||
| #[pallet::weight(<T as Config>::SettlementFn::transfer_funds_weight())] | ||
| pub fn transfer_asset( | ||
| origin: OriginFor<T>, | ||
| asset_id: AssetId, | ||
|
|
@@ -1718,6 +1734,51 @@ pub mod pallet { | |
| ) -> DispatchResultWithPostInfo { | ||
| Self::base_reject_asset_transfer(origin, transfer_id) | ||
| } | ||
|
|
||
| /// Set the allowance for `spender` to transfer up to `amount` of `asset_id` from | ||
| /// the caller's account. | ||
| /// | ||
| /// Replaces any existing allowance for this (owner, spender, asset_id) combination. | ||
| /// Setting `amount` to 0 revokes the allowance (removes the storage entry). | ||
| /// Setting `amount` to `Balance::MAX` grants an unlimited allowance that is never | ||
| /// decremented on spend. | ||
Neopallium marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /// | ||
| /// # Arguments | ||
| /// * `origin` — Signed origin. Caller must have a registered DID. | ||
| /// * `asset_id` — The asset for which the allowance is set. | ||
| /// * `spender` — The AccountId authorized to spend. | ||
| /// * `amount` — Maximum amount the spender may transfer. 0 = revoke. Balance::MAX = unlimited. | ||
| /// | ||
| /// # Errors | ||
| /// * `BadOrigin` — Unsigned origin. | ||
| /// * `MissingIdentity` — Caller's key is not linked to a DID. | ||
| #[pallet::call_index(37)] | ||
| #[pallet::weight(<T as Config>::WeightInfo::approve())] | ||
| pub fn approve( | ||
| origin: OriginFor<T>, | ||
| asset_id: AssetId, | ||
| spender: T::AccountId, | ||
| amount: Balance, | ||
| ) -> DispatchResult { | ||
| let caller_data = IdentityPallet::<T>::ensure_origin_call_permissions(origin)?; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on our discussion we will keep using call permissions here. Which means we should change |
||
| let owner = caller_data.sender; | ||
|
|
||
| let key = (owner.clone(), spender.clone(), asset_id); | ||
| if amount == 0 { | ||
| Allowances::<T>::remove(&key); | ||
| } else { | ||
| Allowances::<T>::insert(&key, amount); | ||
| } | ||
|
|
||
| Self::deposit_event(Event::Approval { | ||
| owner, | ||
| spender, | ||
| asset_id, | ||
| amount, | ||
| }); | ||
|
|
||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| #[pallet::error] | ||
|
|
@@ -1826,6 +1887,8 @@ pub mod pallet { | |
| KeyNotFoundForDid, | ||
| /// Insufficient tokens are locked. | ||
| InsufficientTokensLocked, | ||
| /// The spender's allowance for this asset is insufficient for the requested transfer amount. | ||
| InsufficientAllowance, | ||
| } | ||
|
|
||
| pub trait WeightInfo { | ||
|
|
@@ -1863,8 +1926,8 @@ pub mod pallet { | |
| fn link_ticker_to_asset_id() -> Weight; | ||
| fn unlink_ticker_from_asset_id() -> Weight; | ||
| fn update_global_metadata_spec() -> Weight; | ||
| fn transfer_asset_base_weight() -> Weight; | ||
| fn receiver_affirm_asset_transfer_base_weight() -> Weight; | ||
| fn approve() -> Weight; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -2599,6 +2662,35 @@ impl<T: AssetConfig> Pallet<T> { | |
| Ok(()) | ||
| } | ||
|
|
||
| /// Check and decrement spender allowance for a fungible transfer. | ||
| /// | ||
| /// - `Balance::MAX` (infinite allowance): no storage write. | ||
| /// - Depletes to zero: removes the storage entry. | ||
| /// - No `Approval` event is emitted on spend. | ||
| pub fn spend_allowance( | ||
| owner: &T::AccountId, | ||
| spender: &T::AccountId, | ||
| asset_id: AssetId, | ||
| amount: Balance, | ||
| ) -> DispatchResult { | ||
| let key = (owner.clone(), spender.clone(), asset_id); | ||
| let current = Allowances::<T>::get(&key); | ||
| ensure!(current >= amount, Error::<T>::InsufficientAllowance); | ||
|
|
||
| // Infinite allowance — no deduction. | ||
| if current == Balance::MAX { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let new_allowance = current.saturating_sub(amount); | ||
| if new_allowance == 0 { | ||
| Allowances::<T>::remove(&key); | ||
| } else { | ||
| Allowances::<T>::insert(&key, new_allowance); | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn base_link_ticker_to_asset_id( | ||
| origin: T::RuntimeOrigin, | ||
| ticker: Ticker, | ||
|
|
@@ -2713,24 +2805,23 @@ impl<T: AssetConfig> Pallet<T> { | |
| #[cfg(feature = "runtime-benchmarks")] bench_base_weight: bool, | ||
| ) -> DispatchResultWithPostInfo { | ||
| let from = ensure_signed(origin.clone())?; | ||
| 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( | ||
|
Comment on lines
-2716
to
-2722
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| let to_holder = AssetHolder::try_from(to.encode()) | ||
| .map_err(|_| DispatchError::Other("InvalidAccountId"))?; | ||
| let fund = Fund { | ||
| description: FundDescription::Fungible { asset_id, amount }, | ||
| memo: memo.clone(), | ||
| }; | ||
|
|
||
| let (instruction_id, consumed) = T::SettlementFn::transfer_funds( | ||
| origin, | ||
| to.clone(), | ||
| asset_id, | ||
| amount, | ||
| memo.clone(), | ||
| &mut weight_meter, | ||
| None, | ||
| to_holder, | ||
| fund, | ||
| #[cfg(feature = "runtime-benchmarks")] | ||
| bench_base_weight, | ||
| )?; | ||
|
|
||
| // Emit a transfer event. | ||
| Self::deposit_event(Event::CreatedAssetTransfer { | ||
| asset_id, | ||
| from, | ||
|
|
@@ -2740,7 +2831,7 @@ impl<T: AssetConfig> Pallet<T> { | |
| pending_transfer_id: instruction_id, | ||
| }); | ||
|
|
||
| Ok(PostDispatchInfo::from(Some(weight_meter.consumed()))) | ||
| Ok(PostDispatchInfo::from(Some(consumed))) | ||
| } | ||
|
|
||
| pub fn base_receiver_affirm_asset_transfer( | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,8 +19,9 @@ use polymesh_primitives::nft::{ | |||||||||||||||||||||||||||||||||||||
| NFTCollection, NFTCollectionId, NFTCollectionKeys, NFTCount, NFTId, NFTMetadataAttribute, | ||||||||||||||||||||||||||||||||||||||
| NFTOwnerStatus, NFTs, | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
| use polymesh_primitives::portfolio::{Fund, FundDescription}; | ||||||||||||||||||||||||||||||||||||||
| use polymesh_primitives::settlement::InstructionId; | ||||||||||||||||||||||||||||||||||||||
| use polymesh_primitives::traits::{ComplianceFnConfig, NFTTrait}; | ||||||||||||||||||||||||||||||||||||||
| use polymesh_primitives::traits::{ComplianceFnConfig, NFTTrait, SettlementFnTrait}; | ||||||||||||||||||||||||||||||||||||||
| use polymesh_primitives::{ | ||||||||||||||||||||||||||||||||||||||
| AccountId as AccountId32, HoldingsUpdateReason, IdentityId, Memo, PortfolioId, WeightMeter, | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -297,6 +298,29 @@ pub mod pallet { | |||||||||||||||||||||||||||||||||||||
| ) -> DispatchResult { | ||||||||||||||||||||||||||||||||||||||
| Self::base_controller_transfer(origin, nfts, source, callers_holdings_kind) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /// Transfer NFTs between accounts or portfolios. | ||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||
| /// Same-identity transfers move NFTs directly. Cross-identity transfers | ||||||||||||||||||||||||||||||||||||||
| /// route through the settlement engine. | ||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||
| /// # Arguments | ||||||||||||||||||||||||||||||||||||||
| /// * `origin` — Signed origin. Caller must have a registered DID. | ||||||||||||||||||||||||||||||||||||||
| /// * `nfts` — The NFTs to transfer. | ||||||||||||||||||||||||||||||||||||||
| /// * `from` — Source. `None` defaults to caller's account. | ||||||||||||||||||||||||||||||||||||||
| /// * `to` — Destination account or portfolio. | ||||||||||||||||||||||||||||||||||||||
| /// * `memo` — Optional memo attached to the transfer. | ||||||||||||||||||||||||||||||||||||||
| #[pallet::call_index(4)] | ||||||||||||||||||||||||||||||||||||||
| #[pallet::weight(<T as pallet_asset::Config>::SettlementFn::transfer_funds_weight())] | ||||||||||||||||||||||||||||||||||||||
| pub fn transfer_nft( | ||||||||||||||||||||||||||||||||||||||
| origin: OriginFor<T>, | ||||||||||||||||||||||||||||||||||||||
| nfts: NFTs, | ||||||||||||||||||||||||||||||||||||||
| from: Option<AssetHolder>, | ||||||||||||||||||||||||||||||||||||||
| to: AssetHolder, | ||||||||||||||||||||||||||||||||||||||
| memo: Option<Memo>, | ||||||||||||||||||||||||||||||||||||||
| ) -> DispatchResultWithPostInfo { | ||||||||||||||||||||||||||||||||||||||
| Self::base_transfer_nft(origin, nfts, from, to, memo) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| #[pallet::error] | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -740,6 +764,33 @@ impl<T: Config> Pallet<T> { | |||||||||||||||||||||||||||||||||||||
| Ok(()) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| pub fn base_transfer_nft( | ||||||||||||||||||||||||||||||||||||||
| origin: T::RuntimeOrigin, | ||||||||||||||||||||||||||||||||||||||
| nfts: NFTs, | ||||||||||||||||||||||||||||||||||||||
| from: Option<AssetHolder>, | ||||||||||||||||||||||||||||||||||||||
| to: AssetHolder, | ||||||||||||||||||||||||||||||||||||||
| memo: Option<Memo>, | ||||||||||||||||||||||||||||||||||||||
| ) -> DispatchResultWithPostInfo { | ||||||||||||||||||||||||||||||||||||||
| 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 _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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a double map, so that the owner can easily query all current allowances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could even use a NMap.