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

feat: multisig ed25519 #110

Merged
merged 10 commits into from
Sep 28, 2023

Conversation

eranrund
Copy link
Contributor

@eranrund eranrund commented Sep 19, 2023

Description

This PR adds Ed25519 key support to the multisig contract.

Todos

  • Unit tests
  • Manual tests
  • Documentation
  • Connect epics/issues

Steps to Test

Run cargo test - happy to do any manual testing if you provide guidelines/instructions on how you approach that.

Expected Behaviour

The multisig contract should now support Ed25519 keys.

Other Notes

The multisig-prover contract is still hardcoded to Ecdsa keys. I am not sure if it is a priority to change that.

Linking to AXE-1900

@eranrund eranrund requested a review from a team as a code owner September 19, 2023 19:14
@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #110 (b457283) into main (9f0a344) will increase coverage by 0.27%.
The diff coverage is 98.60%.

@@            Coverage Diff             @@
##             main     #110      +/-   ##
==========================================
+ Coverage   86.85%   87.12%   +0.27%     
==========================================
  Files         114      115       +1     
  Lines       11099    11372     +273     
==========================================
+ Hits         9640     9908     +268     
- Misses       1459     1464       +5     
Files Coverage Δ
contracts/multisig/src/key.rs 91.03% <100.00%> (+5.55%) ⬆️
contracts/multisig/src/signing.rs 99.56% <100.00%> (+0.05%) ⬆️
contracts/multisig/src/test/common.rs 100.00% <100.00%> (ø)
contracts/multisig/src/types.rs 93.75% <100.00%> (ø)
contracts/multisig/src/contract.rs 98.56% <99.62%> (+0.03%) ⬆️
contracts/multisig/src/ed25519.rs 85.71% <85.71%> (ø)
ampd/src/handlers/multisig.rs 82.77% <0.00%> (-2.15%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@eranrund eranrund changed the title Multisig ed25519 feat: Multisig ed25519 Sep 19, 2023
@eranrund eranrund changed the title feat: Multisig ed25519 feat: multisig ed25519 Sep 19, 2023
Copy link
Contributor

@cgorenflo cgorenflo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest some minor adjustments, otherwise looks good to me!

contracts/multisig/src/key.rs Outdated Show resolved Hide resolved
contracts/multisig/src/key.rs Show resolved Hide resolved
@eranrund eranrund requested a review from cgorenflo September 21, 2023 23:05
contracts/multisig/src/ed25519.rs Outdated Show resolved Hide resolved
contracts/multisig/src/ed25519.rs Outdated Show resolved Hide resolved
@eranrund eranrund requested a review from cjcobb23 September 22, 2023 20:32
@cgorenflo cgorenflo enabled auto-merge (squash) September 28, 2023 22:15
@cgorenflo cgorenflo merged commit 4b3fe87 into axelarnetwork:main Sep 28, 2023
7 checks passed
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 this pull request may close these issues.

3 participants