Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate how to reduce boilerplate that needs to be added almost to every struct #1142

Closed
frol opened this issue Feb 10, 2024 · 4 comments · Fixed by #1147
Closed

Investigate how to reduce boilerplate that needs to be added almost to every struct #1142

frol opened this issue Feb 10, 2024 · 4 comments · Fixed by #1147

Comments

@frol
Copy link
Collaborator

frol commented Feb 10, 2024

Moving the discussion from near/borsh-rs#277 since it is bigger than borsh. serde and schemars also contribute a lot to this ugly boilerplate:

use near_sdk::serde::{Deserialize, Serialize}
use near_sdk::borsh::{BorshDeserialize, BorshSerialize};
use near_sdk::schemars::JsonSchema;

#[derive(Deserialize, Serialize, JsonSchema, BorshDeserlialize, BorshSerialize)]
#[serde(crate = "near_sdk::serde")]
#[schemars(crate = "near_sdk::schemars")]
#[borsh(crate = "near_sdk::borsh")]
struct ...
@frol
Copy link
Collaborator Author

frol commented Feb 15, 2024

I have chatted with @PolyProgrammist and we arrived to the following design:

#[near]
struct Contract {
    ...
}

#[near(serializers = [borsh, json])]
struct Post {
    author: AccountId,
    text: String,
}

#[near]
impl Contract {
    fn get_posts(&self) -> Vec<Post> { ... }
}
  • #[near] for struct/enums would imply Borsh serialization by default.
  • #[near(serializers = [borsh, json])] allows to control if borsh and/or json needs to be implemented for the struct/enum - some needs one of them, others may need both at once (if struct is persistent to the state and used in the function arguments / return value)
  • #[near] for impl and trait blocks should behave almost like #[near_bindgen] (it seems that #[near_bindgen] would try to create the root struct if there is no struct marked with #[near_bindgen], I would remove this behavior)
  • #[near] should also enable JsonSchema and/or BorshSchema when abi feature is at play. Otherwise, schemars must be feature-gated

@frol
Copy link
Collaborator Author

frol commented Feb 24, 2024

Just for the context, here is how I see status-message contract to be updated after these improvements land:

Before:

use near_sdk::borsh::{BorshDeserialize, BorshSerialize};
use near_sdk::store::LookupMap;
use near_sdk::{env, near_bindgen, AccountId, BorshStorageKey};

#[derive(BorshSerialize, BorshStorageKey)]
#[borsh(crate = "near_sdk::borsh")]
enum StorageKey {
    StatusMessageRecords,
}

#[near_bindgen]
#[derive(BorshDeserialize, BorshSerialize)]
#[borsh(crate = "near_sdk::borsh")]
pub struct StatusMessage {
    records: LookupMap<AccountId, String>,
}

impl Default for StatusMessage {
    fn default() -> Self {
        Self {
            records: LookupMap::new(StorageKey::StatusMessageRecords),
        }
    }
}

#[near_bindgen]
impl StatusMessage {
    pub fn set_status(&mut self, message: String) {
        let account_id = env::predecessor_account_id();
        self.records.insert(account_id, message);
    }

    pub fn get_status(&self, account_id: AccountId) -> Option<&String> {
        self.records.get(&account_id)
    }
}

After:

use near_sdk::{near, AccountId, NearStorageKey};
use near_sdk::store::LookupMap;

#[derive(NearStorageKey)]
enum StorageKey {
    StatusMessageRecords,
}

#[near]
pub struct StatusMessage {
    records: LookupMap<AccountId, String>,
}

#[near]
impl StatusMessage {
    #[init]
    pub fn new() -> Self {
        Self {
            records: LookupMap::new(StorageKey::StatusMessageRecords),
        }
    }

    pub fn set_status(&mut self, message: String) {
        let account_id = near_sdk::env::predecessor_account_id();
        self.records.insert(account_id, message);
    }

    pub fn get_status(&self, account_id: AccountId) -> Option<&String> {
        self.records.get(&account_id)
    }
}

In addition to the already discussed #[near] attribute macro:

  1. NearStorageKey could automatically implement (BorshStorageKey + BorshSerialize), so developers don't need to write that boilterplate.

  2. Implement Default trait for the struct as part of #[near] for impl blocks:
    a. if new method marked with #[init] has no parameters, call new()
    b. otherwise, panic

    A bit of context here: Default trait is always required for the contract struct; if there is no meaningful default, you have to implement it anyway and panic, so there is PanicOnDefault derive helper - lack of initialization is a common footgun in computer science, so this safety guard is important to keep. Yet, we can make it more ergonomic. In case new needs some arguments to initialize, there is no meaningful Default impl, so we can automatically implement PanicOnDefault, and if there is no new method there, Rust will complain about missing : Default trait bound.

@PolyProgrammist
Copy link
Collaborator

Discussed with Vlad that we don't add NearStorageKey and Default trait for now

@PolyProgrammist
Copy link
Collaborator

PolyProgrammist commented Mar 14, 2024

The reasons are below.

Default: if we do that for impl, there may be multiple definition, because there may be several impl sections. However it can't be done for structure as well, because we don't know information about the #[init] method and others

As for NearStorageKey, for the following structure:

pub enum StorageKey {
    TokensPerOwner { account_hash: Vec<u8> },
}

We would generate the following code:

const _: () = {
    #[allow(non_camel_case_types)]
    type StorageKey_KEKEKKE = StorageKey;
    {
        #[derive(::near_sdk::borsh::BorshSerialize, ::near_sdk::BorshStorageKey)]
        #[borsh(crate = "near_sdk::borsh")]
        pub enum StorageKeyJ {
            TokensPerOwner { account_hash: Vec<u8> },
        }        
        #[automatically_derived]
        impl ::near_sdk::__private::BorshIntoStorageKey for StorageKey_KEKEKKE {
        }

        impl From<StorageKey_KEKEKKE> for StorageKeyJ {
                fn from(value: StorageKey_KEKEKKE) -> Self {
                    // value
                    match value {
                        StorageKey::TokensPerOwner { account_hash } => StorageKeyJ::TokensPerOwner { account_hash: account_hash.clone() }
                    }
                }
        }

        impl BorshSerialize for StorageKey_KEKEKKE {
            fn serialize<W: ::near_sdk::borsh::io::Write>(&self, writer: &mut W) -> Result<(), std::io::Error> {
                let x: StorageKeyJ = StorageKeyJ::from(self);
                let _ = BorshSerialize::serialize(&x, writer);
                Ok(())
            }
        }
    };
};

But there is an error:

error[E0277]: the trait bound `StorageKeyJ: From<&StorageKey>` is not satisfied
  --> near-contract-standards/src/non_fungible_token/core/core_impl.rs:83:38
   |
83 |                 let x: StorageKeyJ = StorageKeyJ::from(self);
   |                                      ^^^^^^^^^^^ the trait `From<&StorageKey>` is not implemented for `StorageKeyJ`
   |
   = help: the trait `From<StorageKey>` is implemented for `StorageKeyJ`
   = help: for that trait implementation, expected `StorageKey`, found `&StorageKey`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants