Skip to content

Commit

Permalink
🐛 Depositing PalletAttributeSet on incorrect nft (paritytech#2740)
Browse files Browse the repository at this point in the history
## Context

Implementing `HolderOf(collection_id)` we have observed a fancy glitch
where pallet deposits event with incorrect values

### Test case 

[Observe following
extrinsic](https://assethub-polkadot.subscan.io/extrinsic/0xdc72321b7674aa209c2f194ed49bd6bd12708af103f98b5b9196e0132dcba777)

To mint in collection `51` user needs to be `HolderOf(50)`.
Therefore current user is owner of item `394` `witness_data {
owned_item: 394 }`

All checking is done correctly, storage is updated correctly

 
![photo_2023-12-18 16 07
11](https://github.com/paritytech/polkadot-sdk/assets/22471030/ca991272-156d-4db1-97b2-1a2873fc5d3f)

However the event which is emitted does not make semantic sense as we
updated storage for `50-394` not for `51-114`

![photo_2023-12-18 16 07
17](https://github.com/paritytech/polkadot-sdk/assets/22471030/c998a92c-e306-4433-aad8-103078140e23)

## The fix 

This PR fixes that depositing `PalletAttributeSet` emits correct values.

---------

Co-authored-by: Jegor Sidorenko <[email protected]>
Co-authored-by: Jegor Sidorenko <[email protected]>
  • Loading branch information
3 people authored Mar 9, 2024
1 parent 9f5d9fa commit 1c435e9
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
4 changes: 2 additions & 2 deletions substrate/frame/nfts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -874,8 +874,8 @@ pub mod pallet {
),
);
Self::deposit_event(Event::PalletAttributeSet {
collection,
item: Some(item),
collection: collection_id,
item: Some(owned_item),
attribute: pallet_attribute,
value: attribute_value,
});
Expand Down
6 changes: 6 additions & 0 deletions substrate/frame/nfts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,12 @@ fn mint_should_work() {
account(2),
Some(MintWitness { owned_item: Some(43), ..Default::default() })
));
assert!(events().contains(&Event::<Test>::PalletAttributeSet {
collection: 0,
item: Some(43),
attribute: PalletAttributes::<<Test as Config>::CollectionId>::UsedToClaim(1),
value: Nfts::construct_attribute_value(vec![]).unwrap(),
}));

// can't mint twice
assert_noop!(
Expand Down

0 comments on commit 1c435e9

Please sign in to comment.