--- a/pallets/common/src/lib.rs +++ b/pallets/common/src/lib.rs @@ -1730,6 +1730,15 @@ ); Ok(new_permission) } + + /// Repair possibly broken properties of a collection. + pub fn repair_collection(collection_id: CollectionId) -> DispatchResult { + CollectionProperties::::mutate(collection_id, |properties| { + properties.recompute_consumed_space(); + }); + + Ok(()) + } } /// Indicates unsupported methods by returning [Error::UnsupportedOperation]. @@ -1819,7 +1828,7 @@ fn set_allowance_for_all() -> Weight; /// The price of repairing an item. - fn repair_item() -> Weight; + fn force_repair_item() -> Weight; } /// Weight info extension trait for refungible pallet. --- a/pallets/fungible/src/common.rs +++ b/pallets/fungible/src/common.rs @@ -113,7 +113,7 @@ Weight::zero() } - fn repair_item() -> Weight { + fn force_repair_item() -> Weight { Weight::zero() } } --- a/pallets/nonfungible/src/common.rs +++ b/pallets/nonfungible/src/common.rs @@ -127,7 +127,7 @@ >::set_allowance_for_all() } - fn repair_item() -> Weight { + fn force_repair_item() -> Weight { >::repair_item() } } @@ -540,7 +540,7 @@ fn repair_item(&self, token: TokenId) -> DispatchResultWithPostInfo { with_weight( >::repair_item(self, token), - >::repair_item(), + >::force_repair_item(), ) } } --- a/pallets/refungible/src/common.rs +++ b/pallets/refungible/src/common.rs @@ -157,7 +157,7 @@ >::set_allowance_for_all() } - fn repair_item() -> Weight { + fn force_repair_item() -> Weight { >::repair_item() } } @@ -544,7 +544,7 @@ fn repair_item(&self, token: TokenId) -> DispatchResultWithPostInfo { with_weight( >::repair_item(self, token), - >::repair_item(), + >::force_repair_item(), ) } } --- a/pallets/unique/src/lib.rs +++ b/pallets/unique/src/lib.rs @@ -82,7 +82,7 @@ BoundedVec, }; use scale_info::TypeInfo; -use frame_system::{self as system, ensure_signed}; +use frame_system::{self as system, ensure_signed, ensure_root}; use sp_std::{vec, vec::Vec}; use up_data_structs::{ MAX_COLLECTION_NAME_LENGTH, MAX_COLLECTION_DESCRIPTION_LENGTH, MAX_TOKEN_PREFIX_LENGTH, @@ -983,18 +983,33 @@ }) } - /// Repairs a broken item + /// Repairs a collection's properties if the data was somehow corrupted. /// /// # Arguments /// + /// * `collection_id`: ID of the collection to repair. + #[weight = >::force_repair_collection()] + pub fn force_repair_collection( + origin, + collection_id: CollectionId, + ) -> DispatchResult { + ensure_root(origin)?; + >::repair_collection(collection_id) + } + + /// Repairs a token's properties if the data was somehow corrupted. + /// + /// # Arguments + /// /// * `collection_id`: ID of the collection the item belongs to. /// * `item_id`: ID of the item. - #[weight = T::CommonWeightInfo::repair_item()] - pub fn repair_item( - _origin, + #[weight = T::CommonWeightInfo::force_repair_item()] + pub fn force_repair_item( + origin, collection_id: CollectionId, item_id: TokenId, ) -> DispatchResultWithPostInfo { + ensure_root(origin)?; dispatch_tx::(collection_id, |d| { d.repair_item(item_id) }) --- a/pallets/unique/src/weights.rs +++ b/pallets/unique/src/weights.rs @@ -45,6 +45,7 @@ fn remove_collection_sponsor() -> Weight; fn set_transfers_enabled_flag() -> Weight; fn set_collection_limits() -> Weight; + fn force_repair_collection() -> Weight; } /// Weights for pallet_unique using the Substrate node and recommended hardware. @@ -139,6 +140,12 @@ .saturating_add(T::DbWeight::get().reads(1 as u64)) .saturating_add(T::DbWeight::get().writes(1 as u64)) } + // Storage: Common CollectionProperties (r:1 w:1) + fn force_repair_collection() -> Weight { + Weight::from_ref_time(5_701_000 as u64) + .saturating_add(T::DbWeight::get().reads(1 as u64)) + .saturating_add(T::DbWeight::get().writes(1 as u64)) + } } // For backwards compatibility and tests @@ -232,4 +239,10 @@ .saturating_add(RocksDbWeight::get().reads(1 as u64)) .saturating_add(RocksDbWeight::get().writes(1 as u64)) } + // Storage: Common CollectionProperties (r:1 w:1) + fn force_repair_collection() -> Weight { + Weight::from_ref_time(5_701_000 as u64) + .saturating_add(RocksDbWeight::get().reads(1 as u64)) + .saturating_add(RocksDbWeight::get().writes(1 as u64)) + } } --- a/runtime/common/weights.rs +++ b/runtime/common/weights.rs @@ -125,8 +125,8 @@ max_weight_of!(set_allowance_for_all()) } - fn repair_item() -> Weight { - max_weight_of!(repair_item()) + fn force_repair_item() -> Weight { + max_weight_of!(force_repair_item()) } } --- a/tests/package.json +++ b/tests/package.json @@ -46,6 +46,7 @@ "testUnnesting": "mocha --timeout 9999999 -r ts-node/register ./**/unnest.test.ts", "testProperties": "mocha --timeout 9999999 -r ts-node/register ./**/collectionProperties.test.ts ./**/tokenProperties.test.ts ./**/getPropertiesRpc.test.ts", "testCollectionProperties": "mocha --timeout 9999999 -r ts-node/register ./**/collectionProperties.test.ts", + "testTokenProperties": "mocha --timeout 9999999 -r ts-node/register ./**/tokenProperties.test.ts", "testMigration": "mocha --timeout 9999999 -r ts-node/register ./**/nesting/migration-check.test.ts", "testAddCollectionAdmin": "mocha --timeout 9999999 -r ts-node/register ./**/addCollectionAdmin.test.ts", "testSetCollectionLimits": "mocha --timeout 9999999 -r ts-node/register ./**/setCollectionLimits.test.ts", --- a/tests/src/nesting/collectionProperties.test.ts +++ b/tests/src/nesting/collectionProperties.test.ts @@ -18,11 +18,13 @@ import {itSub, Pallets, usingPlaygrounds, expect, requirePalletsOrSkip} from '../util'; describe('Integration Test: Collection Properties', () => { + let superuser: IKeyringPair; let alice: IKeyringPair; let bob: IKeyringPair; before(async () => { await usingPlaygrounds(async (helper, privateKey) => { + superuser = await privateKey('//Alice'); const donor = await privateKey({filename: __filename}); [alice, bob] = await helper.arrange.createAccounts([200n, 10n], donor); }); @@ -199,6 +201,23 @@ expectedConsumedSpaceDiff = biggerPropDataSize - smallerPropDataSize; expect(consumedSpace).to.be.equal(biggerPropDataSize - expectedConsumedSpaceDiff); }); + + itSub('Modifying a collection property with different sizes correctly changes the consumed space', async({helper}) => { + const properties = [ + {key: 'sea-creatures', value: 'mermaids'}, + {key: 'goldenratio', value: '1.6180339887498948482045868343656381177203091798057628621354486227052604628189'}, + ]; + const collection = await helper[testSuite.mode].mintCollection(alice, {properties}); + + const newProperty = ' '.repeat(4096); + await collection.setProperties(alice, [{key: 'space', value: newProperty}]); + const originalSpace = await collection.getPropertiesConsumedSpace(); + expect(originalSpace).to.be.equal(properties[0].value.length + properties[1].value.length + newProperty.length); + + await helper.getSudo().executeExtrinsic(superuser, 'api.tx.unique.forceRepairCollection', [collection.collectionId], true); + const recomputedSpace = await collection.getPropertiesConsumedSpace(); + expect(recomputedSpace).to.be.equal(originalSpace); + }); })); }); @@ -314,6 +333,16 @@ ).to.be.rejectedWith(/common\.InvalidCharacterInPropertyKey/); } }); + + itSub('Modifying a collection property with different sizes correctly changes the consumed space', async({helper}) => { + const collection = await helper[testSuite.mode].mintCollection(alice, {properties: [ + {key: 'sea-creatures', value: 'mermaids'}, + {key: 'goldenratio', value: '1.6180339887498948482045868343656381177203091798057628621354486227052604628189'}, + ]}); + + await expect(helper.executeExtrinsic(alice, 'api.tx.unique.forceRepairCollection', [collection.collectionId], true)) + .to.be.rejectedWith(/BadOrigin/); + }); })); }); \ No newline at end of file --- a/tests/src/nesting/tokenProperties.test.ts +++ b/tests/src/nesting/tokenProperties.test.ts @@ -19,6 +19,7 @@ import {UniqueHelper, UniqueNFToken, UniqueRFToken} from '../util/playgrounds/unique'; describe('Integration Test: Token Properties', () => { + let superuser: IKeyringPair; let alice: IKeyringPair; // collection owner let bob: IKeyringPair; // collection admin let charlie: IKeyringPair; // token owner @@ -27,6 +28,7 @@ before(async () => { await usingPlaygrounds(async (helper, privateKey) => { + superuser = await privateKey('//Alice'); const donor = await privateKey({filename: __filename}); [alice, bob, charlie] = await helper.arrange.createAccounts([200n, 100n, 100n], donor); }); @@ -406,7 +408,7 @@ {mode: 'nft' as const, pieces: undefined, requiredPallets: []}, {mode: 'rft' as const, pieces: 100n, requiredPallets: [Pallets.ReFungible]}, ].map(testCase => - itSub.ifWithPallets(`repair_item preserves valid consumed space (${testCase.mode})`, testCase.requiredPallets, async({helper}) => { + itSub.ifWithPallets(`force_repair_item preserves valid consumed space (${testCase.mode})`, testCase.requiredPallets, async({helper}) => { const propKey = 'tok-prop'; const collection = await helper[testCase.mode].mintCollection(alice, { @@ -430,7 +432,7 @@ const originalSpace = await token.getTokenPropertiesConsumedSpace(); expect(originalSpace).to.be.equal(propDataSize); - await helper.executeExtrinsic(alice, 'api.tx.unique.repairItem', [token.collectionId, token.tokenId], true); + await helper.getSudo().executeExtrinsic(superuser, 'api.tx.unique.forceRepairItem', [token.collectionId, token.tokenId], true); const recomputedSpace = await token.getTokenPropertiesConsumedSpace(); expect(recomputedSpace).to.be.equal(originalSpace); })); @@ -697,6 +699,35 @@ permission: {mutable: true, tokenOwner: true, collectionAdmin: true}, }])).to.be.rejectedWith(/common\.PropertyLimitReached/); })); + + [ + {mode: 'nft' as const, pieces: undefined, requiredPallets: []}, + {mode: 'rft' as const, pieces: 100n, requiredPallets: [Pallets.ReFungible]}, + ].map(testCase => + itSub.ifWithPallets(`Forbids force_repair_item from non-sudo (${testCase.mode})`, testCase.requiredPallets, async({helper}) => { + const propKey = 'tok-prop'; + + const collection = await helper[testCase.mode].mintCollection(alice, { + tokenPropertyPermissions: [ + { + key: propKey, + permission: {mutable: true, tokenOwner: true}, + }, + ], + }); + const token = await ( + testCase.pieces + ? collection.mintToken(alice, testCase.pieces) + : collection.mintToken(alice) + ); + + const propDataSize = 4096; + const propData = 'a'.repeat(propDataSize); + await token.setProperties(alice, [{key: propKey, value: propData}]); + + await expect(helper.executeExtrinsic(alice, 'api.tx.unique.forceRepairItem', [token.collectionId, token.tokenId], true)) + .to.be.rejectedWith(/BadOrigin/); + })); }); describe('ReFungible token properties permissions tests', () => {