Skip to content

Commit

Permalink
chore: resolve review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
chungquantin committed Jan 6, 2025
1 parent 1159074 commit c5fd9e3
Show file tree
Hide file tree
Showing 10 changed files with 29 additions and 26 deletions.
10 changes: 5 additions & 5 deletions pallets/nfts/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ benchmarks_instance_pallet! {
mint {
let (collection, caller, caller_lookup) = create_collection::<T, I>();
let item = T::Helper::item(0);
T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance() + T::CollectionDeposit::get() + T::BalanceDeposit::get());
T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance() + T::CollectionDeposit::get() + T::CollectionBalanceDeposit::get());
}: _(SystemOrigin::Signed(caller.clone()), collection, item, caller_lookup, None)
verify {
assert_last_event::<T, I>(Event::Issued { collection, item, owner: caller }.into());
Expand All @@ -283,7 +283,7 @@ benchmarks_instance_pallet! {
force_mint {
let (collection, caller, caller_lookup) = create_collection::<T, I>();
let item = T::Helper::item(0);
T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance() + T::CollectionDeposit::get() + T::BalanceDeposit::get());
T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance() + T::CollectionDeposit::get() + T::CollectionBalanceDeposit::get());
}: _(SystemOrigin::Signed(caller.clone()), collection, item, caller_lookup, default_item_config())
verify {
assert_last_event::<T, I>(Event::Issued { collection, item, owner: caller }.into());
Expand All @@ -303,7 +303,7 @@ benchmarks_instance_pallet! {

let target: T::AccountId = account("target", 0, SEED);
let target_lookup = T::Lookup::unlookup(target.clone());
T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance() + T::BalanceDeposit::get());
T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance() + T::CollectionBalanceDeposit::get());
}: _(SystemOrigin::Signed(caller.clone()), collection, item, target_lookup)
verify {
assert_last_event::<T, I>(Event::Transferred { collection, item, from: caller, to: target }.into());
Expand Down Expand Up @@ -669,7 +669,7 @@ benchmarks_instance_pallet! {
let price = ItemPrice::<T, I>::from(0u32);
let origin = SystemOrigin::Signed(seller.clone()).into();
Nfts::<T, I>::set_price(origin, collection, item, Some(price), Some(buyer_lookup))?;
T::Currency::make_free_balance_be(&buyer, T::Currency::minimum_balance() + price + T::BalanceDeposit::get());
T::Currency::make_free_balance_be(&buyer, T::Currency::minimum_balance() + price + T::CollectionBalanceDeposit::get());
}: _(SystemOrigin::Signed(buyer.clone()), collection, item, price)
verify {
assert_last_event::<T, I>(Event::ItemBought {
Expand Down Expand Up @@ -759,7 +759,7 @@ benchmarks_instance_pallet! {
let duration = T::MaxDeadlineDuration::get();
let target: T::AccountId = account("target", 0, SEED);
let target_lookup = T::Lookup::unlookup(target.clone());
T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance() + T::BalanceDeposit::get());
T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance() + T::CollectionBalanceDeposit::get());
let origin = SystemOrigin::Signed(caller.clone());
frame_system::Pallet::<T>::set_block_number(One::one());
Nfts::<T, I>::transfer(origin.clone().into(), collection, item2, target_lookup)?;
Expand Down
2 changes: 1 addition & 1 deletion pallets/nfts/src/common_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
maybe_balance.as_mut().ok_or(Error::<T, I>::NoItemOwned)?;

*balance = balance.checked_sub(1).ok_or(ArithmeticError::Underflow)?;
if *balance == 0 {
if balance.is_zero() {
T::Currency::unreserve(deposit_account, *deposit_amount);
*maybe_balance = None;
}
Expand Down
2 changes: 1 addition & 1 deletion pallets/nfts/src/features/approvals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
);
ensure!(
AccountBalance::<T, I>::get(collection, &owner)
.filter(|(balance, _)| *balance > 0)
.filter(|(balance, _)| !balance.is_zero())
.is_some(),
Error::<T, I>::NoItemOwned
);
Expand Down
5 changes: 3 additions & 2 deletions pallets/nfts/src/features/create_delete_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
let deposit_account =
maybe_depositor.unwrap_or_else(|| collection_details.owner.clone());

let balance_deposit =
deposit_required.then_some(T::BalanceDeposit::get()).unwrap_or_default();
let balance_deposit = deposit_required
.then_some(T::CollectionBalanceDeposit::get())
.unwrap_or_default();
Self::increment_account_balance(
collection,
&mint_to,
Expand Down
15 changes: 8 additions & 7 deletions pallets/nfts/src/features/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,16 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
// Update account balance of the destination account.
let deposit_amount =
match collection_config.is_setting_enabled(CollectionSetting::DepositRequired) {
true => T::BalanceDeposit::get(),
true => T::CollectionBalanceDeposit::get(),
false => Zero::zero(),
};
// The destination account covers the `BalanceDeposit` if it has sufficient balance.
// Otherwise, the caller is accountable for it.
let deposit_account = match T::Currency::can_reserve(&dest, T::BalanceDeposit::get()) {
true => &dest,
false => caller,
};
// The destination account covers the `CollectionBalanceDeposit` if it has sufficient
// balance. Otherwise, the caller is accountable for it.
let deposit_account =
match T::Currency::can_reserve(&dest, T::CollectionBalanceDeposit::get()) {
true => &dest,
false => caller,
};

Self::increment_account_balance(collection, &dest, (deposit_account, deposit_amount))?;

Expand Down
5 changes: 3 additions & 2 deletions pallets/nfts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,11 @@ pub mod pallet {
#[pallet::constant]
type CollectionApprovalDeposit: Get<DepositBalanceOf<Self, I>>;

/// The basic amount of funds that must be reversed for an account balance.
/// The basic amount of funds that must be reversed for an account's collection balance.
// Key: `sizeof((CollectionId, AccountId))` bytes.
// Value: `sizeof((u32, Some(AccountId, Balance)))` bytes.
#[pallet::constant]
type BalanceDeposit: Get<DepositBalanceOf<Self, I>>;
type CollectionBalanceDeposit: Get<DepositBalanceOf<Self, I>>;
}

/// Details of a collection.
Expand Down Expand Up @@ -441,6 +441,7 @@ pub mod pallet {
T::CollectionId,
Blake2_128Concat,
T::AccountId,
// (Account's collection items, Deposit details).
(u32, AccountDepositOf<T, I>),
>;

Expand Down
2 changes: 1 addition & 1 deletion pallets/nfts/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ parameter_types! {
impl Config for Test {
type ApprovalsLimit = ConstU32<10>;
type AttributeDepositBase = ConstU64<1>;
type BalanceDeposit = ConstU64<1>;
type CollectionApprovalDeposit = ConstU64<1>;
type CollectionBalanceDeposit = ConstU64<1>;
type CollectionDeposit = ConstU64<2>;
type CollectionId = u32;
type CreateOrigin = AsEnsureOriginWithArg<frame_system::EnsureSigned<Self::AccountId>>;
Expand Down
2 changes: 1 addition & 1 deletion pallets/nfts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ fn cancel_collection_approval(
}

fn balance_deposit() -> DepositBalanceOf<Test> {
<<Test as Config>::BalanceDeposit>::get()
<<Test as Config>::CollectionBalanceDeposit>::get()
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions runtime/devnet/src/config/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ parameter_types! {
parameter_types! {
pub NftsPalletFeatures: PalletFeatures = PalletFeatures::all_enabled();
// Key = 68 bytes (4+16+32+16), Value = 52 bytes (4+32+16)
pub const NftsBalanceDeposit: Balance = deposit(1, 120);
pub const NftsCollectionBalanceDeposit: Balance = deposit(1, 120);
pub const NftsCollectionDeposit: Balance = 10 * UNIT;
// Key = 116 bytes (4+16+32+16+32+16), Value = 21 bytes (1+4+16)
pub const NftsCollectionApprovalDeposit: Balance = deposit(1, 137);
Expand All @@ -45,8 +45,8 @@ impl pallet_nfts::Config for Runtime {
// TODO: source from primitives
type ApprovalsLimit = ConstU32<20>;
type AttributeDepositBase = NftsAttributeDepositBase;
type BalanceDeposit = NftsBalanceDeposit;
type CollectionApprovalDeposit = NftsCollectionApprovalDeposit;
type CollectionBalanceDeposit = NftsCollectionBalanceDeposit;
type CollectionDeposit = NftsCollectionDeposit;
// TODO: source from primitives
type CollectionId = CollectionId;
Expand Down Expand Up @@ -139,7 +139,7 @@ mod tests {
.first()
.and_then(|info| info.max_size)
.unwrap_or_default();
assert_eq!(deposit(1, max_size), NftsBalanceDeposit::get());
assert_eq!(deposit(1, max_size), NftsCollectionBalanceDeposit::get());
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions runtime/testnet/src/config/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ parameter_types! {
parameter_types! {
pub NftsPalletFeatures: PalletFeatures = PalletFeatures::all_enabled();
// Key = 68 bytes (4+16+32+16), Value = 52 bytes (4+32+16)
pub const NftsBalanceDeposit: Balance = deposit(1, 120);
pub const NftsCollectionBalanceDeposit: Balance = deposit(1, 120);
pub const NftsCollectionDeposit: Balance = 10 * UNIT;
// Key = 116 bytes (4+16+32+16+32+16), Value = 21 bytes (1+4+16)
pub const NftsCollectionApprovalDeposit: Balance = deposit(1, 137);
Expand All @@ -45,8 +45,8 @@ impl pallet_nfts::Config for Runtime {
// TODO: source from primitives
type ApprovalsLimit = ConstU32<20>;
type AttributeDepositBase = NftsAttributeDepositBase;
type BalanceDeposit = NftsBalanceDeposit;
type CollectionApprovalDeposit = NftsCollectionApprovalDeposit;
type CollectionBalanceDeposit = NftsCollectionBalanceDeposit;
type CollectionDeposit = NftsCollectionDeposit;
// TODO: source from primitives
type CollectionId = CollectionId;
Expand Down Expand Up @@ -139,7 +139,7 @@ mod tests {
.first()
.and_then(|info| info.max_size)
.unwrap_or_default();
assert_eq!(deposit(1, max_size), NftsBalanceDeposit::get());
assert_eq!(deposit(1, max_size), NftsCollectionBalanceDeposit::get());
}

#[test]
Expand Down

0 comments on commit c5fd9e3

Please sign in to comment.