-
Notifications
You must be signed in to change notification settings - Fork 12
feat: add new signature scheme variant to contract #1658
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
base: main
Are you sure you want to change the base?
feat: add new signature scheme variant to contract #1658
Conversation
5aa351a to
71916ff
Compare
| @@ -1,3 +1,4 @@ | |||
| // todo [#1657](https://github.com/near/mpc/issues/1657): split this file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
| // todo [#1657](https://github.com/near/mpc/issues/1657): split this file | |
| // TODO(#1657): split this file |
as we are using it like that in some other places and IMO looks better
| }; | ||
| pub const PARTICIPANT_LEN: usize = 10; | ||
| use utilities::AccountIdExtV1; | ||
|
|
||
| use crate::sandbox::initializing_utils::start_keygen_instance; | ||
|
|
||
| use super::initializing_utils::{vote_add_domains, vote_public_key}; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems we are going back and forth between defining constants and use statements. Could you fix it by putting all constants below?
| @@ -1,5 +1,6 @@ | |||
| --- | |||
| source: crates/contract/tests/abi.rs | |||
| assertion_line: 48 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should not be added. It was also a problem in recent PR's by Simon, not sure why it does not happen to me.
Resolves #1638
This PR ensures the new signature scheme variant is included in all tests, which leads to a slight refactor of the sandbox tests.
The tests are functionally the same, but the main difference is that before, we often only tested with a single domain (usually for Secp256k1), whereas now, we almost always test a contract that holds one domain for each signature scheme ([Secp256k1, Ed25519, Bls12381 V2Secp256k1])`.
Additionally, I took the opportunity to re-organize some helper functions and make them more robust. A natural follow-up is #1657.