MET-129: Restrict token standard on bubblegum #87
Conversation
|
You should be able to run |
kstepanovdev
force-pushed
the
restrict-token-standard-on-bubblegum
branch
3 times, most recently
from
dca5db8 to
613102a
Compare
| use crate::{error::BubblegumError, state::metaplex_adapter::MetadataArgs, utils::cmp_pubkeys}; | ||
| use crate::{ | ||
| error::BubblegumError, | ||
| state::metaplex_adapter::{MetadataArgs, TokenStandard as MetadataTokenStandard}, |
There was a problem hiding this comment.
Nit: I think its clearer to just call it TokenStandard since that is the type and that's how its been used here and in other places.
There was a problem hiding this comment.
I had to rename it because assert.rs already contains TokenStandard from mpl_token_metadata::types. These types are different.
| @@ -84,6 +84,8 @@ pub enum BubblegumError { | |||
| PrimarySaleCanOnlyBeFlippedToTrue, | |||
| #[msg("Creator did not unverify the metadata")] | |||
| CreatorDidNotUnverify, | |||
| #[msg("Minting Bubblegum is allowed only for NonFungible Standard")] | |||
There was a problem hiding this comment.
wdyt about "Only NonFungible standard is supported" ?
clients/rust/tests/mint.rs
Outdated
| @@ -117,4 +117,94 @@ mod mint { | |||
|
|
|||
| tree_manager.assert_root(&mut context).await; | |||
| } | |||
|
|
|||
| #[tokio::test] | |||
| async fn mint_empty_token_standard() { | |||
There was a problem hiding this comment.
wdyt about "cannot_mint_empty_token_standard"?
clients/rust/tests/mint.rs
Outdated
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn mint_wrong_token_standard() { |
There was a problem hiding this comment.
wdyt about "cannot_mint_wrong_token_standard"?
clients/rust/tests/mint.rs
Outdated
| // Minting must fail because the token standard hasn't been provided. | ||
| assert!(minting_result.is_err()); |
There was a problem hiding this comment.
Could you also do assert_eq!(minting_result.unwrap_err(), BubblegumError::InvalidTokenStandard) to make sure it fails for the exact expected reason?
although you already covered this in the JS test so its ok without it, but might as well.
There was a problem hiding this comment.
Yeah, I agree.
However, TreeManager is a wrapper and has Result<_, BanksClientError> signature. I suppose it's better to remove these tests then.
clients/rust/tests/mint.rs
Outdated
| // Minting must fail because the provided token standard is Fungible. | ||
| assert!(minting_result.is_err()); |
There was a problem hiding this comment.
Could you also do assert_eq!(minting_result.unwrap_err(), BubblegumError::InvalidTokenStandard) to make sure it fails for the exact expected reason?
although you already covered this in the JS test so its ok without it, but might as well.
There was a problem hiding this comment.
I doubt they could've been compared in a such way because they're not implementing PartialEq, could have they? asset_matches should've been used instead.
kstepanovdev
force-pushed
the
restrict-token-standard-on-bubblegum
branch
from
bb61573 to
45b426d
Compare
kstepanovdev
force-pushed
the
restrict-token-standard-on-bubblegum
branch
from
45b426d to
eba7643
Compare
* Prevent non NonFungible from minting * update clients * updated tests * update program's test accordingly to the latest changes regarding TokenStandard
No description provided.