From 55bb4eb6e5afb980445ada2ea53d640446959512 Mon Sep 17 00:00:00 2001 From: str-mv <51784859+str-mv@users.noreply.github.com> Date: Tue, 23 Nov 2021 11:19:27 +0000 Subject: [PATCH] Merge pull request #238 from UniqueNetwork/fix/sponsoring-checks Fix sponsoring token ownership check --- --- a/pallets/fungible/src/lib.rs +++ b/pallets/fungible/src/lib.rs @@ -52,7 +52,7 @@ StorageMap; #[pallet::storage] - pub(super) type Balance = StorageNMap< + pub type Balance = StorageNMap< Key = ( Key, Key, --- a/pallets/nft/src/eth/sponsoring.rs +++ b/pallets/nft/src/eth/sponsoring.rs @@ -10,7 +10,7 @@ use core::convert::TryInto; use nft_data_structs::TokenId; use up_evm_mapping::EvmBackwardsAddressMapping; -use pallet_evm::AddressMapping; +use pallet_common::account::CrossAccountId; use pallet_nonfungible::erc::{UniqueNFTCall, ERC721UniqueExtensionsCall, ERC721Call}; use pallet_fungible::erc::{UniqueFungibleCall, ERC20Call}; @@ -23,7 +23,7 @@ let sponsor = collection.sponsorship.sponsor()?.clone(); let sponsor = ::EvmBackwardsAddressMapping::from_account_id(sponsor); - let who = ::EvmAddressMapping::into_account_id(*who); + let who = T::CrossAccountId::from_eth(*who); let (method_id, mut reader) = AbiReader::new_call(&call.1).ok()?; match &collection.mode { crate::CollectionMode::NFT => { @@ -31,14 +31,19 @@ match call { UniqueNFTCall::ERC721UniqueExtensions( ERC721UniqueExtensionsCall::Transfer { token_id, .. }, - ) - | UniqueNFTCall::ERC721(ERC721Call::TransferFrom { token_id, .. }) => { + ) => { let token_id: TokenId = token_id.try_into().ok()?; withdraw_transfer::(&collection, &who, &token_id).map(|()| sponsor) } + UniqueNFTCall::ERC721(ERC721Call::TransferFrom { token_id, from, .. }) => { + let token_id: TokenId = token_id.try_into().ok()?; + let from = T::CrossAccountId::from_eth(from); + withdraw_transfer::(&collection, &from, &token_id).map(|()| sponsor) + } UniqueNFTCall::ERC721(ERC721Call::Approve { token_id, .. }) => { let token_id: TokenId = token_id.try_into().ok()?; - withdraw_approve::(&collection, &who, &token_id).map(|()| sponsor) + withdraw_approve::(&collection, who.as_sub(), &token_id) + .map(|()| sponsor) } _ => None, } @@ -47,12 +52,17 @@ let call = UniqueFungibleCall::parse(method_id, &mut reader).ok()??; #[allow(clippy::single_match)] match call { - UniqueFungibleCall::ERC20( - ERC20Call::Transfer { .. } | ERC20Call::TransferFrom { .. }, - ) => withdraw_transfer::(&collection, &who, &TokenId::default()) - .map(|()| sponsor), + UniqueFungibleCall::ERC20(ERC20Call::Transfer { .. }) => { + withdraw_transfer::(&collection, &who, &TokenId::default()) + .map(|()| sponsor) + } + UniqueFungibleCall::ERC20(ERC20Call::TransferFrom { from, .. }) => { + let from = T::CrossAccountId::from_eth(from); + withdraw_transfer::(&collection, &from, &TokenId::default()) + .map(|()| sponsor) + } UniqueFungibleCall::ERC20(ERC20Call::Approve { .. }) => { - withdraw_approve::(&collection, &who, &TokenId::default()) + withdraw_approve::(&collection, who.as_sub(), &TokenId::default()) .map(|()| sponsor) } _ => None, --- a/pallets/nft/src/sponsorship.rs +++ b/pallets/nft/src/sponsorship.rs @@ -10,25 +10,38 @@ storage::{StorageMap, StorageDoubleMap, StorageNMap}, }; use nft_data_structs::{ - CollectionId, FUNGIBLE_SPONSOR_TRANSFER_TIMEOUT, NFT_SPONSOR_TRANSFER_TIMEOUT, - REFUNGIBLE_SPONSOR_TRANSFER_TIMEOUT, TokenId, + CollectionId, FUNGIBLE_SPONSOR_TRANSFER_TIMEOUT, MetaUpdatePermission, + NFT_SPONSOR_TRANSFER_TIMEOUT, REFUNGIBLE_SPONSOR_TRANSFER_TIMEOUT, TokenId, }; use pallet_common::{CollectionHandle}; use pallet_common::account::CrossAccountId; pub fn withdraw_transfer( collection: &CollectionHandle, - who: &T::AccountId, + who: &T::CrossAccountId, item_id: &TokenId, ) -> Option<()> { // preliminary sponsoring correctness check - if !((pallet_nonfungible::TokenData::::get((collection.id, item_id))?.owner).as_sub() == who) - || (pallet_refungible::Owned::::get(( - collection.id, - T::CrossAccountId::from_sub(who.clone()), - item_id, - ))) { - return None; + match collection.mode { + CollectionMode::NFT => { + let owner = pallet_nonfungible::TokenData::::get((collection.id, item_id))?.owner; + if !owner.conv_eq(who) { + return None; + } + } + CollectionMode::Fungible(_) => { + if item_id != &TokenId::default() { + return None; + } + if >::get((collection.id, who)) == 0 { + return None; + } + } + CollectionMode::ReFungible => { + if !>::get((collection.id, who, item_id)) { + return None; + } + } } // sponsor timeout @@ -43,9 +56,11 @@ let last_tx_block = match collection.mode { CollectionMode::NFT => >::get(collection.id, item_id), - CollectionMode::Fungible(_) => >::get(collection.id, who), + CollectionMode::Fungible(_) => { + >::get(collection.id, who.as_sub()) + } CollectionMode::ReFungible => { - >::get((collection.id, item_id, who)) + >::get((collection.id, item_id, who.as_sub())) } }; @@ -59,11 +74,12 @@ match collection.mode { CollectionMode::NFT => >::insert(collection.id, item_id, block_number), CollectionMode::Fungible(_) => { - >::insert(collection.id, who, block_number) - } - CollectionMode::ReFungible => { - >::insert((collection.id, item_id, who), block_number) + >::insert(collection.id, who.as_sub(), block_number) } + CollectionMode::ReFungible => >::insert( + (collection.id, item_id, who.as_sub()), + block_number, + ), }; Some(()) @@ -101,20 +117,37 @@ } pub fn withdraw_set_variable_meta_data( - who: &T::AccountId, + who: &T::CrossAccountId, collection: &CollectionHandle, item_id: &TokenId, data: &[u8], ) -> Option<()> { - // preliminary sponsoring correctness check - if !((pallet_nonfungible::TokenData::::get((collection.id, item_id))?.owner).as_sub() == who) - || (pallet_refungible::Owned::::get(( - collection.id, - T::CrossAccountId::from_sub(who.clone()), - item_id, - ))) { + // TODO: make it work for admins + if collection.meta_update_permission != MetaUpdatePermission::ItemOwner { return None; } + // preliminary sponsoring correctness check + match collection.mode { + CollectionMode::NFT => { + let owner = pallet_nonfungible::TokenData::::get((collection.id, item_id))?.owner; + if !owner.conv_eq(who) { + return None; + } + } + CollectionMode::Fungible(_) => { + if item_id != &TokenId::default() { + return None; + } + if >::get((collection.id, who)) == 0 { + return None; + } + } + CollectionMode::ReFungible => { + if !>::get((collection.id, who, item_id)) { + return None; + } + } + } // Can't sponsor fungible collection, this tx will be rejected // as invalid @@ -203,14 +236,23 @@ collection_id, item_id, .. + } => { + let (sponsor, collection) = load(*collection_id)?; + withdraw_transfer::( + &collection, + &T::CrossAccountId::from_sub(who.clone()), + item_id, + ) + .map(|()| sponsor) } - | Call::transfer_from { + Call::transfer_from { collection_id, item_id, + from, .. } => { let (sponsor, collection) = load(*collection_id)?; - withdraw_transfer::(&collection, who, item_id).map(|()| sponsor) + withdraw_transfer::(&collection, from, item_id).map(|()| sponsor) } Call::approve { collection_id, @@ -226,8 +268,13 @@ data, } => { let (sponsor, collection) = load(*collection_id)?; - withdraw_set_variable_meta_data::(&who, &collection, item_id, data) - .map(|()| sponsor) + withdraw_set_variable_meta_data::( + &T::CrossAccountId::from_sub(who.clone()), + &collection, + item_id, + data, + ) + .map(|()| sponsor) } _ => None, } -- gitstuff