near / near-sdk-rs

Rust library for writing NEAR smart contracts
https://near-sdk.io
Apache License 2.0
452 stars 242 forks source link

Provide default implementations for token standards #1050

Open agostbiro opened 1 year ago

agostbiro commented 1 year ago

We're deprecating declarative macros for token standards (see #1042 and #1049) to favor explicit implementations.

As suggested in https://github.com/near/near-sdk-rs/issues/422#issuecomment-1016886719, we could provide default implementations for token standard traits for contracts that implement traits to expose their token struct members.

For example, in near-contract-standards:

pub trait NonFungibleTokenContract {
    fn non_fungible_token(&self) -> &NonFungibleToken;
    fn non_fungible_token_mut(&mut self) -> &mut NonFungibleToken;
}

impl<T: NonFungibleTokenContract> NonFungibleTokenCore for T {
    fn nft_transfer(
        &mut self,
        receiver_id: AccountId,
        token_id: TokenId,
        approval_id: Option<u64>,
        memo: Option<String>,
    ) {
        self.non_fungible_token_mut().nft_transfer(receiver_id, token_id, approval_id, memo)
    }

    fn nft_transfer_call(
        &mut self,
        receiver_id: AccountId,
        token_id: TokenId,
        approval_id: Option<u64>,
        memo: Option<String>,
        msg: String,
    ) -> PromiseOrValue<bool> {
        self.non_fungible_token_mut().nft_transfer_call(
            receiver_id,
            token_id,
            approval_id,
            memo,
            msg,
        )
    }

    fn nft_token(&self, token_id: TokenId) -> Option<Token> {
        self.non_fungible_token().nft_token(token_id)
    }
}

impl<T: NonFungibleTokenContract> NonFungibleTokenResolver for T {
    fn nft_resolve_transfer(
        &mut self,
        previous_owner_id: AccountId,
        receiver_id: AccountId,
        token_id: TokenId,
        approved_account_ids: Option<HashMap<AccountId, u64>>,
    ) -> bool {
        self.non_fungible_token_mut().nft_resolve_transfer(
            previous_owner_id,
            receiver_id,
            token_id,
            approved_account_ids,
        )
    }
}

// etc

And then users can just implement NonFungibleTokenContract to get the NFT trait methods:

struct MyContract {
    non_fungible_token: NonFungibleToken,
}

impl NonFungibleTokenContract for MyContract {
    fn non_fungible_token(&self) -> &NonFungibleToken {
        &self.non_fungible_token
    }

    fn non_fungible_token_mut(&mut self) -> &mut NonFungibleToken {
        &mut self.non_fungible_token
    }
}

The examples should be updated as well with this pattern where it makes sense.

frol commented 1 year ago

Great suggestion to implement NonFungibleTokenContract trait, so then we can have default impls for NonFungibleTokenCore! Though, I would avoid blanket implementation. Instead, I propose the following:

Assuming we have the following setup:

struct NonFungibleToken {}
impl NonFungibleToken {
    fn inner_nft_transfer(&self, id: String, receiver: String) {
        println!("INNER NFT TRANSFER {id} to {receiver}");
    }
}

trait NonFungibleTokenContract {
    fn get_non_fungible_token(&self) -> &NonFungibleToken;
}

// This is just the default implementation, but it is not a blanket implementation:
trait NonFungibleTokenCore: NonFungibleTokenContract {
    fn nft_transfer(&self, id: String, receiver: String) {
        self.get_non_fungible_token().inner_nft_transfer(id, receiver);
    }
}

Here is how I would implement my smart contract that needs the default NFT logic:

struct MyContract {
    non_fungible_token: NonFungibleToken,
}

impl NonFungibleTokenContract for MyContract {
    fn get_non_fungible_token(&self) -> &NonFungibleToken {
        &self.non_fungible_token
    }
}

impl NonFungibleTokenCore for MyContract {}

Here is how I would customize one of the methods (just implement it in the trait implementation):

impl NonFungibleTokenCore for MyContract {
    fn nft_transfer(&self, id: String, receiver: String) {
        println!("CUSTOM LOGIC");
    }
}

Here is the playground.

The problem, however, is #[near_bindgen] has no clue about the default methods and thus it cannot generate extern "C" functions. That is:

#[near_bindgen]
impl NonFungibleTokenCore for MyContract {}

, will not generate the exported functions. There is no clear solution in my head at this point. Maybe we can implement "trait registration"?

frol commented 1 year ago

Another consideration point is the function attributes like #[payable]: https://github.com/near/near-sdk-rs/blob/587a47be1df5351faa28e8a8f25de3a7ff713315/near-contract-standards/src/non_fungible_token/approval/mod.rs#L35-L56. Those also need to be aligned. I feel that at this stage it might only make things harder to reason about in contrast to just offering developers nice copy-pastable snippets in the documentation.

agostbiro commented 1 year ago

Thanks @frol for raising great points! I jumped the gun on the example.

I agree with avoiding the blanket implementations in favor of the solution that you proposed.

I don't have a good solution either for generating extern "C" functions, but I feel that if we find a nice solution there, we could just add the payable attributes in the default implementation. But I also understand your concern that we might end up adding too much complexity with this feature.

Should we leave this open for a weeks to see if we come up with something simple for the extern generation problem and if not just close it?

frol commented 1 year ago

Should we leave this open for a weeks to see if we come up with something simple for the extern generation problem and if not just close it?

I think we can keep it open similarly to #1056. Probably we need to introduce a new label for such dead-end issues, so we can close them and easily discover them in the future.