NEP-364

[blasrodri]

Implementing the following host functions according to the design proposed in NEP-364

    pub fn ed25519_verify(
        &mut self,
        sig_len: u64,
        sig_ptr: u64,
        msg_len: u64,
        msg_ptr: u64,
        pub_key_len: u64,
        pub_key_ptr: u64,
        register_id: u64,
    ) -> Result<()>;

• [x] added unit tests
• [x] added costs (this needs to be reviewed, since I'm not sure how to infer the right numbers)

[seunlanlege]

Absolutely epic work 🙌🏿

For tracking we're missing:

• [ ] in-memory trie construction: https://github.com/paritytech/substrate/blob/6a946fc36d68b89599d7ca1ab03803d10c78468c/primitives/io/src/lib.rs#L515-L520

• [ ] membership and non-membership trie verification: https://github.com/ComposableFi/composable/blob/be2aebf23aa38888a7ceba1c4a2b698500497e83/frame/ibc/src/host_functions.rs#L36-L61

[akhi3030]

As someone with the most context here, assigning self as reviewer here. Once the PR is ready for proper review, will do a detailed review and / or delegate as needed.

[akhi3030]

@blasrodri: PTAL: https://near.zulipchat.com/#narrow/stream/295306-pagoda.2Fcontract-runtime/topic/NEP-364. There are some interesting discussions about this PR.

[blasrodri]

@akhi3030 I've removed all non signature verification related code. So that hashing related functions are moved onto another NEP (will add it shortly).

I'm not sure about https://github.com/near/nearcore/pull/7165#issuecomment-1181830146 What's needed from my side?

[akhi3030]

@akhi3030 I've removed all non signature verification related code. So that hashing related functions are moved onto another NEP (will add it shortly).

I'm not sure about #7165 (comment) What's needed from my side?

Thanks @blasrodri! This comment in the zulip discussion was particularly interesting: https://near.zulipchat.com/#narrow/stream/295306-pagoda.2Fcontract-runtime/topic/NEP-364/near/289315918. We have to figure out how to make sure that all the validations are deterministic. I recommend having a chat with @evgenykuzyakov to further understand the concerns.

[bowenwang1996]

@akhi3030 could you review this PR again?

[blasrodri]

finally, got green :)

[blasrodri]

@jakmeier I just ran it on another machine (M1) using

export CARGO_PROFILE_RELEASE_CODEGEN_UNITS=1;
export CARGO_PROFILE_RELEASE_LTO=fat;

     Running `target/release/runtime-params-estimator --accounts-num 20000 --additional-accounts-num 20000 --iters 5 --warmup-iters 1 --metric time --costs Ed25519VerifyBase,Ed25519VerifyByte`
[elapsed 00:00:03 remaining 00:00:00] Writing into storage ████████████████████   20000/20000  
Ed25519VerifyBase                                   73_232_640_700 gas [                 73.233µs]           (computed in 17.18s) 
Ed25519VerifyByte                                      754_106_187 gas [                    754ns]           (computed in 9.14s) 

[jakmeier]

@jakmeier I just ran it on another machine (M1) using

export CARGO_PROFILE_RELEASE_CODEGEN_UNITS=1;
export CARGO_PROFILE_RELEASE_LTO=fat;

     Running `target/release/runtime-params-estimator --accounts-num 20000 --additional-accounts-num 20000 --iters 5 --warmup-iters 1 --metric time --costs Ed25519VerifyBase,Ed25519VerifyByte`
[elapsed 00:00:03 remaining 00:00:00] Writing into storage ████████████████████   20000/20000  
Ed25519VerifyBase                                   73_232_640_700 gas [                 73.233µs]           (computed in 17.18s) 
Ed25519VerifyByte                                      754_106_187 gas [                    754ns]           (computed in 9.14s) 

Yeah that looks about right. Good to see the uncertainty warning is gone now!

But let's keep the numbers that you already had. The M1 platform is not suited well for our estimations for a number of reasons. And we will need to do fine-tuning anyway before stabilising this.

I think the estimation part is looking good now for this PR.

@matklad do you want to take another more holistic look at the code before we merge it?

[bowenwang1996]

@matklad do you mind reviewing this PR again?

[matklad]

@blasrodri sorry, I forgot auto-merge label last time, could you update this, so that we get this in?

[matklad]

Created a tracking issue for a bunch of unresolved questions we have here: https://github.com/near/nearcore/issues/7567