Initial RMRK Market pallet skeleton #65
Conversation
…s for new functions and offer trait
…c tests to ensure functionality.
… function for making offers on NFTs
…lude UNITS and update market pallet with MinimumAmountOffer type
…ed and throws errors for expired offers or no longer valid offers
|
Added a test scenario that needs to be addressed in code:
To prevent this, we need to have a type name Or would this be better handled by UI dapp? |
… to ensure NFTs cannot be bought after transferred or if a listing has expired
| #[pallet::event] | ||
| #[pallet::generate_deposit(pub(super) fn deposit_event)] | ||
| pub enum Event<T: Config> { | ||
| /// The price for a token was updated \[owner, collection_id, nft_id, price\] |
There was a problem hiding this comment.
It's not necessary to leave the param names in the docstring. The field names in the structure enum variant are already self-explained.
| } | ||
|
|
||
| parameter_types! { | ||
| pub ClassBondAmount: Balance = 100; |
| collection_id: CollectionId, | ||
| nft_id: NftId, | ||
| ) -> DispatchResult { | ||
| let sender = ensure_signed(origin.clone())?; |
There was a problem hiding this comment.
Not necessary to call .clone() here?
| // Ensure that the NFT is not owned by an NFT | ||
| ensure!(!Self::is_nft_owned_by_nft(collection_id, nft_id), | ||
| Error::<T>::CannotListNftOwnedByNft); | ||
| // Ensure sender is not the owner |
There was a problem hiding this comment.
Ensure sender is the owner
| } | ||
|
|
||
| // Add new ListInfo with listed_by, amount, Option<BlockNumber> | ||
| ListedNfts::<T>::insert( |
There was a problem hiding this comment.
Should we lock the NFT after listing?
There was a problem hiding this comment.
Good question. Wouldn't this prevent the NFT from being bought? Or do you think it would be best to unlock the NFT when a buy is initiated.
| }, | ||
| ); | ||
| // Reserve currency from offerer account | ||
| <T as pallet::Config>::Currency::reserve(&sender, amount)?; |
There was a problem hiding this comment.
It's a good habit to do all the checks before making any change to the state. So I'd suggest to move the reserve action before making the change to Offers.
In Substrate, if we don't have transactional modifier, all the changes to the state will take place immediately. If if the function fails later, the changes are still applied. Although we do have transactional here, I think it's still good to always follow the best practice whenever possible.
| ensure!(sender == owner || sender == offer.maker, Error::<T>::CannotWithdrawOffer); | ||
|
|
||
| // Unreserve currency from offerer account | ||
| <T as pallet::Config>::Currency::unreserve(&sender, offer.amount); |
There was a problem hiding this comment.
It looks like if the NFT owner withdrew the offer, he can rob the reserved money
There was a problem hiding this comment.
Yes, need to change this to offer.maker
Targets
rmrk-marketlistandbuywith testsunlistwith testsoffer,accept_offerandwithdraw_offerwith testsProcedure
README.mdsimilar to the description in issue Market pallet design #59Cargo.toml,src/lib.rs,src/mock.rs,src/tests.rslistandbuywith unit tests that omit the royalties logic firstRMRK Spec