difftreelog
Merge pull request #831 from UniqueNetwork/fix/rft-collection-admin-permissions
in: master
5 files changed
pallets/common/src/lib.rsdiffbeforeafterboth--- a/pallets/common/src/lib.rs
+++ b/pallets/common/src/lib.rs
@@ -390,8 +390,10 @@
Ok(())
}
- /// Return **true** if `user` was not allowed to have tokens, and he can ignore such restrictions.
- pub fn ignores_allowance(&self, user: &T::CrossAccountId) -> bool {
+ /// Returns **true** if
+ /// * the `user`is a collection owner or admin
+ /// * the collection limits allow the owner/admins to transfer/burn any collection token
+ pub fn ignores_token_restrictions(&self, user: &T::CrossAccountId) -> bool {
self.limits.owner_can_transfer() && self.is_owner_or_admin(user)
}
pallets/fungible/src/lib.rsdiffbeforeafterboth--- a/pallets/fungible/src/lib.rs
+++ b/pallets/fungible/src/lib.rs
@@ -677,8 +677,14 @@
// `from`, `to` checked in [`transfer`]
collection.check_allowlist(spender)?;
}
+
+ if collection.ignores_token_restrictions(spender) {
+ return Ok(Self::compute_allowance_decrease(
+ collection, from, spender, amount,
+ ));
+ }
+
if let Some(source) = T::CrossTokenAddressMapping::address_to_token(from) {
- // TODO: should collection owner be allowed to perform this transfer?
ensure!(
<PalletStructure<T>>::check_indirectly_owned(
spender.clone(),
@@ -690,18 +696,25 @@
<CommonError<T>>::ApprovedValueTooLow,
);
return Ok(None);
- }
- let allowance = <Allowance<T>>::get((collection.id, from, spender)).checked_sub(amount);
- if allowance.is_none() {
- ensure!(
- collection.ignores_allowance(spender),
- <CommonError<T>>::ApprovedValueTooLow
- );
}
+ let allowance = Self::compute_allowance_decrease(collection, from, spender, amount);
+ ensure!(allowance.is_some(), <CommonError<T>>::ApprovedValueTooLow);
+
Ok(allowance)
}
+ /// Returns `Some(amount)` if the `spender` have allowance to spend this amount.
+ /// Otherwise, it returns `None`.
+ fn compute_allowance_decrease(
+ collection: &FungibleHandle<T>,
+ from: &T::CrossAccountId,
+ spender: &T::CrossAccountId,
+ amount: u128,
+ ) -> Option<u128> {
+ <Allowance<T>>::get((collection.id, from, spender)).checked_sub(amount)
+ }
+
/// Transfer fungible tokens from one account to another.
/// Same as the [`transfer`][`Pallet::transfer`] but spender doesn't needs to be an owner of the token pieces.
/// The owner should set allowance for the spender to transfer pieces.
pallets/nonfungible/src/lib.rsdiffbeforeafterboth--- a/pallets/nonfungible/src/lib.rs
+++ b/pallets/nonfungible/src/lib.rs
@@ -1246,7 +1246,7 @@
collection.check_allowlist(spender)?;
}
- if collection.limits.owner_can_transfer() && collection.is_owner_or_admin(spender) {
+ if collection.ignores_token_restrictions(spender) {
return Ok(());
}
@@ -1269,11 +1269,8 @@
if <CollectionAllowance<T>>::get((collection.id, from, spender)) {
return Ok(());
}
- ensure!(
- collection.ignores_allowance(spender),
- <CommonError<T>>::ApprovedValueTooLow
- );
- Ok(())
+
+ Err(<CommonError<T>>::ApprovedValueTooLow.into())
}
/// Transfer NFT token from one account to another.
pallets/refungible/src/lib.rsdiffbeforeafterboth--- a/pallets/refungible/src/lib.rs
+++ b/pallets/refungible/src/lib.rs
@@ -1177,6 +1177,13 @@
// `from`, `to` checked in [`transfer`]
collection.check_allowlist(spender)?;
}
+
+ if collection.ignores_token_restrictions(spender) {
+ return Ok(Self::compute_allowance_decrease(
+ collection, token, from, &spender, amount,
+ ));
+ }
+
if let Some(source) = T::CrossTokenAddressMapping::address_to_token(from) {
// TODO: should collection owner be allowed to perform this transfer?
ensure!(
@@ -1191,21 +1198,30 @@
);
return Ok(None);
}
- let allowance =
- <Allowance<T>>::get((collection.id, token, from, &spender)).checked_sub(amount);
+ let allowance = Self::compute_allowance_decrease(collection, token, from, &spender, amount);
+ if allowance.is_some() {
+ return Ok(allowance);
+ }
+
// Allowance (if any) would be reduced if spender is also wallet operator
if <CollectionAllowance<T>>::get((collection.id, from, spender)) {
return Ok(allowance);
}
- if allowance.is_none() {
- ensure!(
- collection.ignores_allowance(spender),
- <CommonError<T>>::ApprovedValueTooLow
- );
- }
- Ok(allowance)
+ Err(<CommonError<T>>::ApprovedValueTooLow.into())
+ }
+
+ /// Returns `Some(amount)` if the `spender` have allowance to spend this amount.
+ /// Otherwise, it returns `None`.
+ fn compute_allowance_decrease(
+ collection: &RefungibleHandle<T>,
+ token: TokenId,
+ from: &T::CrossAccountId,
+ spender: &T::CrossAccountId,
+ amount: u128,
+ ) -> Option<u128> {
+ <Allowance<T>>::get((collection.id, token, from, spender)).checked_sub(amount)
}
/// Transfer RFT token pieces from one account to another.
tests/src/nesting/unnest.test.tsdiffbeforeafterboth161617import {IKeyringPair} from '@polkadot/types/types';17import {IKeyringPair} from '@polkadot/types/types';18import {expect, itSub, Pallets, usingPlaygrounds} from '../util';18import {expect, itSub, Pallets, usingPlaygrounds} from '../util';19import {UniqueFTCollection, UniqueNFToken, UniqueRFToken} from '../util/playgrounds/unique';192020describe('Integration Test: Unnesting', () => {21describe('Integration Test: Unnesting', () => {21 let alice: IKeyringPair;22 let alice: IKeyringPair;23 let bob: IKeyringPair;24 let charlie: IKeyringPair;222523 before(async () => {26 before(async () => {24 await usingPlaygrounds(async (helper, privateKey) => {27 await usingPlaygrounds(async (helper, privateKey) => {25 const donor = await privateKey({filename: __filename});28 const donor = await privateKey({filename: __filename});26 [alice] = await helper.arrange.createAccounts([50n], donor);29 [alice, bob, charlie] = await helper.arrange.createAccounts([200n, 50n, 50n], donor);27 });30 });28 });31 });293284 expect(await targetToken.getChildren()).to.be.length(0);87 expect(await targetToken.getChildren()).to.be.length(0);85 });88 });8990 async function checkNestedAmountState({91 expectedBalance,92 childrenShouldPresent,93 nested,94 targetNft,95 }: {96 expectedBalance: bigint,97 childrenShouldPresent: boolean,98 nested: UniqueFTCollection | UniqueRFToken,99 targetNft: UniqueNFToken,100 }) {101 const balance = await nested.getBalance(targetNft.nestingAccount());102 expect(balance).to.be.equal(expectedBalance);103104 const children = await targetNft.getChildren();105106 if (childrenShouldPresent) {107 expect(children[0]).to.be.deep.equal({108 collectionId: nested.collectionId,109 tokenId: (nested instanceof UniqueFTCollection) ? 0 : nested.tokenId,110 });111 } else {112 expect(children.length).to.be.equal(0);113 }114 }115116 function ownerOrAdminUnnestCases(modes: ('ft' | 'nft' | 'rft')[]): {117 mode: 'ft' | 'nft' | 'rft',118 sender: string,119 op: 'transfer' | 'burn',120 requiredPallets: Pallets[],121 }[] {122 const senders = ['owner', 'admin'];123 const ops = ['transfer', 'burn'];124125 const cases = [];126 for (const mode of modes) {127 const requiredPallets = (mode === 'rft')128 ? [Pallets.ReFungible]129 : [];130131 for (const sender of senders) {132 for (const op of ops) {133 cases.push({134 mode: mode as 'ft' | 'nft' | 'rft',135 sender,136 op: op as 'transfer' | 'burn',137 requiredPallets,138 });139 }140 }141 }142143 return cases;144 }145146 ownerOrAdminUnnestCases(['ft', 'rft']).map(testCase =>147 itSub.ifWithPallets(`[${testCase.mode}]: allows a collection ${testCase.sender} to ${testCase.op} nested token`, testCase.requiredPallets, async({helper}) => {148 const owner = alice;149 const admin = bob;150151 const unnester = (testCase.sender === 'owner')152 ? owner153 : admin;154155 const collectionNFT = await helper.nft.mintCollection(owner);156 await collectionNFT.setPermissions(owner, {nesting: {tokenOwner: true}});157158 const collectionNested = await helper[testCase.mode as 'ft' | 'rft'].mintCollection(owner, {159 limits: {160 ownerCanTransfer: true,161 },162 });163 await collectionNested.addAdmin(owner, {Substrate: admin.address});164165 const targetNft = await collectionNFT.mintToken(owner, {Substrate: charlie.address});166167 let nested: UniqueFTCollection | UniqueRFToken;168 const totalAmount = 5n;169 const firstUnnestAmount = 2n;170 const restUnnestAmount = totalAmount - firstUnnestAmount;171172 if (collectionNested instanceof UniqueFTCollection) {173 await collectionNested.mint(owner, totalAmount, {Substrate: charlie.address});174 nested = collectionNested;175 } else {176 nested = await collectionNested.mintToken(owner, totalAmount, {Substrate: charlie.address});177 }178179 // transfer/burn `amount` of nested assets by `unnester`.180 const doOperationAndCheck = async ({181 amount,182 shouldBeNestedAfterOp,183 }: {184 amount: bigint,185 shouldBeNestedAfterOp: boolean,186 }) => {187 const nestedBalanceBeforeOp = await nested.getBalance(targetNft.nestingAccount());188189 if (testCase.op === 'transfer') {190 const bobBalanceBeforeOp = await nested.getBalance({Substrate: bob.address});191192 await nested.transferFrom(unnester, targetNft.nestingAccount(), {Substrate: bob.address}, amount);193 expect(await nested.getBalance({Substrate: bob.address})).to.be.equal(bobBalanceBeforeOp + amount);194 } else {195 if (nested instanceof UniqueFTCollection) {196 await nested.burnTokensFrom(unnester, targetNft.nestingAccount(), amount);197 } else {198 await nested.burnFrom(unnester, targetNft.nestingAccount(), amount);199 }200 }201202 await checkNestedAmountState({203 expectedBalance: nestedBalanceBeforeOp - amount,204 childrenShouldPresent: shouldBeNestedAfterOp,205 nested,206 targetNft,207 });208 };209210 // Initial setup: nest (fungibles/rft parts).211 // Check NFT's balance of nested assets and NFT's children.212 await nested.transfer(charlie, targetNft.nestingAccount(), totalAmount);213 await checkNestedAmountState({214 expectedBalance: totalAmount,215 childrenShouldPresent: true,216 nested,217 targetNft,218 });219220 // Transfer/burn only a part of nested assets.221 // Check that NFT's balance of the nested assets correctly decreased and NFT's children are not changed.222 await doOperationAndCheck({223 amount: firstUnnestAmount,224 shouldBeNestedAfterOp: true,225 });226227 // Transfer/burn all remaining nested assets.228 // Check that NFT's balance of the nested assets is 0 and NFT has no more children.229 await doOperationAndCheck({230 amount: restUnnestAmount,231 shouldBeNestedAfterOp: false,232 });233 }));234235 ownerOrAdminUnnestCases(['nft']).map(testCase =>236 itSub(`[nft]: allows a collection ${testCase.sender} to ${testCase.op} nested token`, async ({helper}) => {237 const owner = alice;238 const admin = bob;239240 const unnester = (testCase.sender === 'owner')241 ? owner242 : admin;243244 const collectionNFT = await helper.nft.mintCollection(owner);245 await collectionNFT.setPermissions(owner, {nesting: {tokenOwner: true}});246247 const collectionNested = await helper.nft.mintCollection(owner, {248 limits: {249 ownerCanTransfer: true,250 },251 });252 await collectionNested.addAdmin(owner, {Substrate: admin.address});253254 const targetNft = await collectionNFT.mintToken(owner, {Substrate: charlie.address});255 const nested = await collectionNested.mintToken(owner, {Substrate: charlie.address});256257 await nested.transfer(charlie, targetNft.nestingAccount());258 expect(await targetNft.getChildren()).to.be.deep.equal([{259 collectionId: nested.collectionId,260 tokenId: nested.tokenId,261 }]);262263 if (testCase.op === 'transfer') {264 await nested.transferFrom(unnester, targetNft.nestingAccount(), {Substrate: bob.address});265 } else {266 await nested.burnFrom(unnester, targetNft.nestingAccount());267 }268269 expect((await targetNft.getChildren()).length).to.be.equal(0);270 }));86});271});8727288describe('Negative Test: Unnesting', () => {273describe('Negative Test: Unnesting', () => {