-
Notifications
You must be signed in to change notification settings - Fork 192
feat(wallet)!: apply FRC-0102 envelope to sign/verify by default #6967
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
Open
0xDevNinja
wants to merge
1
commit into
ChainSafe:main
Choose a base branch
from
0xDevNinja:0xdevninja/issue-6442-frc0102-wallet
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+114
−8
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,6 @@ use crate::{ | |
| rpc::{self, prelude::*}, | ||
| }; | ||
| use anyhow::{Context as _, bail}; | ||
| use base64::{Engine, prelude::BASE64_STANDARD}; | ||
| use clap::Subcommand; | ||
| use dialoguer::{Password, console::Term, theme::ColorfulTheme}; | ||
| use directories::ProjectDirs; | ||
|
|
@@ -169,17 +168,17 @@ impl WalletBackend { | |
| } | ||
| } | ||
|
|
||
| async fn wallet_sign(&self, address: Address, message: String) -> anyhow::Result<Signature> { | ||
| async fn wallet_sign(&self, address: Address, message: Vec<u8>) -> anyhow::Result<Signature> { | ||
| if let Some(keystore) = &self.local { | ||
| let key = crate::key_management::try_find_key(&address, keystore)?; | ||
|
|
||
| Ok(crate::key_management::sign( | ||
| *key.key_info.key_type(), | ||
| key.key_info.private_key(), | ||
| &BASE64_STANDARD.decode(message)?, | ||
| &message, | ||
| )?) | ||
| } else { | ||
| Ok(WalletSign::call(&self.remote, (address, message.into_bytes())).await?) | ||
| Ok(WalletSign::call(&self.remote, (address, message)).await?) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -262,6 +261,11 @@ pub enum WalletCommands { | |
| /// The address to be used to sign the message | ||
| #[arg(short)] | ||
| address: StrictAddress, | ||
| /// Sign the raw message bytes without the FRC-0102 envelope. Use this | ||
| /// for interoperating with pre-FRC-0102 tooling, or when the bytes are | ||
| /// already an on-chain Filecoin message (which must not be wrapped). | ||
| #[arg(long)] | ||
| raw: bool, | ||
| }, | ||
| /// Validates whether a given string can be decoded as a well-formed address | ||
| ValidateAddress { | ||
|
|
@@ -280,6 +284,12 @@ pub enum WalletCommands { | |
| /// The signature of the message to verify | ||
| #[arg(short)] | ||
| signature: String, | ||
| /// Verify against the raw message bytes without applying the | ||
| /// FRC-0102 envelope. Use this for signatures produced by | ||
| /// pre-FRC-0102 tooling or for on-chain Filecoin messages (which are | ||
| /// signed raw, without the envelope). | ||
| #[arg(long)] | ||
| raw: bool, | ||
| }, | ||
| /// Deletes the wallet associated with the given address. | ||
| Delete { | ||
|
|
@@ -446,9 +456,13 @@ impl WalletCommands { | |
| Ok(()) | ||
| } | ||
| Self::SetDefault { key } => backend.wallet_set_default(key.into()).await, | ||
| Self::Sign { address, message } => { | ||
| Self::Sign { | ||
| address, | ||
| message, | ||
| raw, | ||
| } => { | ||
| let message = hex::decode(message).context("Message has to be a hex string")?; | ||
| let message = BASE64_STANDARD.encode(message); | ||
| let message = if raw { message } else { wrap_frc0102(&message) }; | ||
|
|
||
| let signature = backend.wallet_sign(address.into(), message).await?; | ||
| println!("{}", hex::encode(signature.to_bytes())); | ||
|
|
@@ -463,10 +477,12 @@ impl WalletCommands { | |
| message, | ||
| address, | ||
| signature, | ||
| raw, | ||
| } => { | ||
| let sig_bytes = | ||
| hex::decode(signature).context("Signature has to be a hex string")?; | ||
| let msg = hex::decode(message).context("Message has to be a hex string")?; | ||
| let msg = if raw { msg } else { wrap_frc0102(&msg) }; | ||
|
|
||
| let signature = Signature::from_bytes(sig_bytes)?; | ||
| let is_valid = backend | ||
|
|
@@ -615,6 +631,15 @@ fn resolve_target_address(target_address: &str) -> anyhow::Result<(Address, bool | |
| } | ||
| } | ||
|
|
||
| const FRC_0102_FILECOIN_PREFIX: &[u8] = b"\x19Filecoin Signed Message:\n"; | ||
|
|
||
| /// Wraps `msg` with the FRC-0102 envelope: `0x19 || "Filecoin Signed Message:\n" || ascii(len(msg)) || msg` | ||
| // See <https://github.com/filecoin-project/FIPs/blob/bdd5283279fd115c87c9bbf71d2e40c9d075f5aa/FRCs/frc-0102.md>. | ||
| fn wrap_frc0102(msg: &[u8]) -> Vec<u8> { | ||
| let len = msg.len().to_string(); | ||
| [FRC_0102_FILECOIN_PREFIX, len.as_bytes(), msg].concat() | ||
| } | ||
|
|
||
| fn resolve_method_num(from: &Address, to: &Address, is_0x_recipient: bool) -> u64 { | ||
| if !is_eth_address(from) && !is_0x_recipient { | ||
| return METHOD_SEND; | ||
|
|
@@ -635,7 +660,7 @@ mod tests { | |
| use crate::shim::address::{Address, CurrentNetwork, Network}; | ||
| use crate::shim::message::METHOD_SEND; | ||
|
|
||
| use super::{resolve_method_num, resolve_target_address}; | ||
| use super::{SignatureType, resolve_method_num, resolve_target_address, wrap_frc0102}; | ||
|
|
||
| #[test] | ||
| fn test_resolve_target_address_id() { | ||
|
|
@@ -753,4 +778,82 @@ mod tests { | |
| let method = resolve_method_num(&from, &to, true); | ||
| assert_eq!(method, EVMMethod::InvokeContract as u64); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_wrap_frc0102_empty() { | ||
| let wrapped = wrap_frc0102(&[]); | ||
| assert_eq!(wrapped, b"\x19Filecoin Signed Message:\n0"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_wrap_frc0102_short() { | ||
| let wrapped = wrap_frc0102(b"hello"); | ||
| assert_eq!(wrapped, b"\x19Filecoin Signed Message:\n5hello"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_wrap_frc0102_longer() { | ||
| let msg = b"this is a longer test message"; | ||
| let wrapped = wrap_frc0102(msg); | ||
| let mut expected = Vec::new(); | ||
| expected.extend_from_slice(b"\x19Filecoin Signed Message:\n"); | ||
| expected.extend_from_slice(msg.len().to_string().as_bytes()); | ||
| expected.extend_from_slice(msg); | ||
| assert_eq!(wrapped, expected); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_wrap_frc0102_binary_msg() { | ||
| // Non-UTF-8 bytes must pass through unchanged. | ||
| let msg: &[u8] = &[0x00, 0xFF, 0x10, 0x80]; | ||
| let wrapped = wrap_frc0102(msg); | ||
| assert_eq!(&wrapped[..26], b"\x19Filecoin Signed Message:\n"); | ||
| assert_eq!(&wrapped[26..27], b"4"); | ||
| assert_eq!(&wrapped[27..], msg); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_wrap_frc0102_length_boundaries() { | ||
| // Length is encoded as decimal ASCII; check 1-, 2-, 3- and 4-digit boundaries. | ||
| for len in [0, 9, 10, 99, 100, 999, 1000] { | ||
| let msg = vec![0xABu8; len]; | ||
| let wrapped = wrap_frc0102(&msg); | ||
| let digits = len.to_string(); | ||
| assert_eq!(&wrapped[..26], b"\x19Filecoin Signed Message:\n"); | ||
| assert_eq!(&wrapped[26..26 + digits.len()], digits.as_bytes()); | ||
| assert_eq!(&wrapped[26 + digits.len()..], msg.as_slice()); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_wrap_frc0102_contains_newline() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can combine above tests into single test using the |
||
| // The spec does not require any escaping; embedded newlines pass through. | ||
| let msg = b"line1\nline2"; | ||
| let wrapped = wrap_frc0102(msg); | ||
| assert_eq!(wrapped, b"\x19Filecoin Signed Message:\n11line1\nline2"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_frc0102_roundtrip_sign_verify_secp256k1() { | ||
| use crate::key_management::generate_key; | ||
| let key = generate_key(SignatureType::Secp256k1).unwrap(); | ||
| let raw_msg = b"hello world"; | ||
| let wrapped = wrap_frc0102(raw_msg); | ||
|
|
||
| // Sign the wrapped bytes (what `forest-wallet sign` does by default). | ||
| let signature = crate::key_management::sign( | ||
| *key.key_info.key_type(), | ||
| key.key_info.private_key(), | ||
| &wrapped, | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| // `verify` on the wrapped bytes must succeed. | ||
| signature.verify(&wrapped, &key.address).unwrap(); | ||
|
|
||
| assert!( | ||
| signature.verify(raw_msg, &key.address).is_err(), | ||
| "raw-bytes verify should fail when the signature was produced over the FRC-0102 envelope" | ||
| ); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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's is not being used anywhere, you can remove this.