Skip to content

Commit

Permalink
feat: not require deposit for force_approve_collection_transfer
Browse files Browse the repository at this point in the history
  • Loading branch information
chungquantin committed Jan 10, 2025
1 parent 3e2620a commit 4ca88c9
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 18 deletions.
7 changes: 4 additions & 3 deletions pallets/nfts/src/features/approvals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// - `owner`: The owner of the collection items.
/// - `collection`: The identifier of the collection.
/// - `delegate`: The account that will be approved to take control of the collection items.
/// - `deposit`: The reserved amount for granting a collection approval.
/// - `maybe_deadline`: The optional deadline (in block numbers) specifying the time limit for
/// the approval.
pub(crate) fn do_approve_collection_transfer(
owner: T::AccountId,
collection: T::CollectionId,
delegate: T::AccountId,
deposit: DepositBalanceOf<T, I>,
maybe_deadline: Option<BlockNumberFor<T>>,
) -> DispatchResult {
ensure!(
Expand All @@ -234,11 +236,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
CollectionApprovals::<T, I>::try_mutate_exists(
(&collection, &owner, &delegate),
|maybe_approval| -> DispatchResult {
let deposit_required = T::CollectionApprovalDeposit::get();
let current_deposit =
maybe_approval.map(|(_, deposit)| deposit).unwrap_or_default();
T::Currency::reserve(&owner, deposit_required.saturating_sub(current_deposit))?;
*maybe_approval = Some((deadline, deposit_required));
T::Currency::reserve(&owner, deposit.saturating_sub(current_deposit))?;
*maybe_approval = Some((deadline, deposit));
Ok(())
},
)?;
Expand Down
18 changes: 16 additions & 2 deletions pallets/nfts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2033,7 +2033,13 @@ pub mod pallet {
) -> DispatchResult {
let origin = ensure_signed(origin)?;
let delegate = T::Lookup::lookup(delegate)?;
Self::do_approve_collection_transfer(origin, collection, delegate, maybe_deadline)
Self::do_approve_collection_transfer(
origin,
collection,
delegate,
T::CollectionApprovalDeposit::get(),
maybe_deadline,
)
}

/// Force-approve collection items owned by the `owner` to be transferred by a delegated
Expand All @@ -2042,6 +2048,8 @@ pub mod pallet {
///
/// Origin must be the `ForceOrigin`.
///
/// Any deposit is left alone.
///
/// - `owner`: The owner of the collection items to be force-approved by the `origin`.
/// - `collection`: The collection of the items to be approved for delegated transfer.
/// - `delegate`: The account to delegate permission to transfer collection items owned by
Expand All @@ -2064,7 +2072,13 @@ pub mod pallet {
T::ForceOrigin::ensure_origin(origin)?;
let delegate = T::Lookup::lookup(delegate)?;
let owner = T::Lookup::lookup(owner)?;
Self::do_approve_collection_transfer(owner, collection, delegate, maybe_deadline)
Self::do_approve_collection_transfer(
owner,
collection,
delegate,
Zero::zero(),
maybe_deadline,
)
}

/// Cancel a collection approval.
Expand Down
22 changes: 9 additions & 13 deletions pallets/nfts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4815,7 +4815,6 @@ fn force_approve_collection_transfer_works() {
let collection_owner = account(1);
let deadline: BlockNumberFor<Test> = 20;
let delegate = account(3);
let deposit = CollectionApprovalDeposit::get();
let item_id = 42;
let item_owner = account(2);

Expand Down Expand Up @@ -4866,16 +4865,13 @@ fn force_approve_collection_transfer_works() {
default_item_config()
));
// Approve collection without balance.
assert_noop!(
Nfts::force_approve_collection_transfer(
RuntimeOrigin::root(),
item_owner.clone(),
collection_id,
delegate.clone(),
None
),
BalancesError::<Test, _>::InsufficientBalance,
);
assert_ok!(Nfts::force_approve_collection_transfer(
RuntimeOrigin::root(),
item_owner.clone(),
collection_id,
delegate.clone(),
None
));

Balances::make_free_balance_be(&item_owner, 100);
// Approving a collection to a delegate with:
Expand Down Expand Up @@ -4909,10 +4905,10 @@ fn force_approve_collection_transfer_works() {
}
.into(),
);
assert_eq!(Balances::reserved_balance(&item_owner), deposit);
assert_eq!(Balances::reserved_balance(&item_owner), 0);
assert_eq!(
CollectionApprovals::get((collection_id, &item_owner, &delegate)),
Some((deadline, deposit))
Some((deadline, 0))
);
}

Expand Down

0 comments on commit 4ca88c9

Please sign in to comment.