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.

Sign up for GitHub

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

Jump to bottom

[r2r] Sign and verify messages #1256

Merged
merged 24 commits into from
May 6, 2022
Merged

[r2r] Sign and verify messages #1256

merged 24 commits into from
May 6, 2022

Conversation

caglaryucekaya
Copy link

@caglaryucekaya caglaryucekaya commented Apr 18, 2022 •
edited
Loading

Sign&verify message feature for all supported coins, resolves #1220.

  • Add sign_message_hash methods to hash messages for signature
  • Add recover_compact method to recover public keys from messages and signatures
  • Implement sign&verify message methods
  • Add RPC functions and unit tests

@caglaryucekaya caglaryucekaya changed the title Issues/message sign verify [wip] Issues/message sign verify Apr 19, 2022
@caglaryucekaya caglaryucekaya changed the title [wip] Issues/message sign verify [r2r] Sign and verify messages Apr 19, 2022
artemii235
artemii235 requested changes Apr 20, 2022
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Great work! I have a few minor notes.

mm2src/coins/lp_coins.rs Outdated Show resolved Hide resolved
mm2src/coins/lp_coins.rs Outdated Show resolved Hide resolved
mm2src/mm2_bitcoin/keys/src/public.rs Show resolved Hide resolved
artemii235
artemii235 requested changes Apr 20, 2022
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Two more notes 🙂

mm2src/mm2_bitcoin/keys/src/public.rs Outdated Show resolved Hide resolved
mm2src/mm2_bitcoin/keys/src/public.rs Outdated Show resolved Hide resolved
@artemii235 artemii235 changed the title [r2r] Sign and verify messages [wip] Sign and verify messages Apr 22, 2022
@caglaryucekaya caglaryucekaya changed the title [wip] Sign and verify messages [r2r] Sign and verify messages Apr 28, 2022
@artemii235
Copy link
Member

@caglaryucekaya Clippy checks failed on your PR. Please ensure that CI checks pass before you put [r2r] mark. Please also sync your branch with dev.

@caglaryucekaya caglaryucekaya changed the title [r2r] Sign and verify messages [wip] Sign and verify messages Apr 29, 2022
@caglaryucekaya
Add sign&verify message feature
6c00f13
@caglaryucekaya
Add tests and small fixes
1ca582f
@caglaryucekaya
move MarketCoinOps trait unimplemented bodies into coin implementations
2d36e90
@caglaryucekaya
fix sign&verify message requests and responses
ae4a6be
@caglaryucekaya
improve error handling for sign&verify message methods
19772a6
@caglaryucekaya
remove bitcoin_hashes dependency
01befb1
@caglaryucekaya
remove bitcoin dependency for variable length integer serialization
608d8a0
@caglaryucekaya
improve error handling for sign&verify message functions
3fb1a59
@caglaryucekaya
add sign_message_prefix to coin config & move message_hash function i…
ddad264 
…nto lp_coin implementations
@caglaryucekaya
add errors if message prefix is not included in coin configuration
14aaad7
@caglaryucekaya
Fix H256 conflict in eth.rs
05b2abb
@caglaryucekaya
make sign_message_hash method return bytes
9042d3f
@caglaryucekaya
Add sign&verify message feature for ethereum
872381b
@caglaryucekaya
Add minor fixes
078d5ad
@caglaryucekaya
Add sign_message_prefix for ethereum wasm tests
d90680f
@caglaryucekaya
Move UTXO message signature-related implementations into utxo_common
c053b44
@caglaryucekaya
add sign/verify message feature for all supported coin types
a7a85c3
@caglaryucekaya
resolve conflicts after rebase
451f8a5
@caglaryucekaya caglaryucekaya changed the title [wip] Sign and verify messages [r2r] Sign and verify messages May 2, 2022
artemii235
artemii235 requested changes May 2, 2022
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

One minor change and one possible bug 🙂

mm2src/coins/lightning.rs Outdated Show resolved Hide resolved
mm2src/coins/utxo/utxo_common.rs Outdated Show resolved Hide resolved
@caglaryucekaya caglaryucekaya changed the title [r2r] Sign and verify messages [wip] Sign and verify messages May 2, 2022
@caglaryucekaya caglaryucekaya changed the title [wip] Sign and verify messages [r2r] Sign and verify messages May 2, 2022
artemii235
artemii235 requested changes May 3, 2022
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Few more changes requests.

mm2src/mm2_tests.rs Outdated Show resolved Hide resolved
mm2src/mm2_tests.rs Outdated Show resolved Hide resolved
mm2src/mm2_tests/bch_and_slp_tests.rs Outdated Show resolved Hide resolved
mm2src/mm2_tests/bch_and_slp_tests.rs Outdated Show resolved Hide resolved
@caglaryucekaya caglaryucekaya changed the title [r2r] Sign and verify messages [wip] Sign and verify messages May 3, 2022
@caglaryucekaya caglaryucekaya changed the title [wip] Sign and verify messages [r2r] Sign and verify messages May 3, 2022
artemii235
artemii235 requested changes May 4, 2022
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Few more minor changes. Please also solve conflicts.

mm2src/coins/utxo/utxo_common.rs Outdated Show resolved Hide resolved
mm2src/coins/eth.rs Outdated Show resolved Hide resolved
mm2src/mm2_tests.rs Outdated Show resolved Hide resolved
artemii235
artemii235 previously approved these changes May 4, 2022
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Great work 👍

@artemii235 artemii235 requested a review from shamardy May 4, 2022 11:43
@artemii235
Copy link
Member

@shamardy Could you please check the Lightning specific part?

shamardy
shamardy previously approved these changes May 4, 2022
View reviewed changes
Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Great Work 🔥 !

@caglaryucekaya caglaryucekaya dismissed stale reviews from shamardy and artemii235 via ba2aa48 May 5, 2022 11:58
artemii235
artemii235 approved these changes May 6, 2022
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Reapproving.

@artemii235 artemii235 merged commit 128d096 into dev May 6, 2022
@artemii235 artemii235 deleted the issues/message-sign-verify branch May 6, 2022 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artemii235 artemii235 artemii235 approved these changes

@shamardy shamardy shamardy left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@caglaryucekaya @artemii235 @shamardy