-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add methods to NodeAPI for signmessage and checkmessage [issue #109] #319
Conversation
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.
Thank you @lnd3v looks good, dropped some comments.
2031f76
to
caea3a9
Compare
859de80
to
a05f9c1
Compare
For the record, i had to ask about signing on the greenlight discord. The response I got: cdecker — Today at 15:50
We are currently using the signmessage method to create attestations and authorizations for keyless clients. If we were to allow clients to call signmessage they could escalate privileges trivially.
We are going to change that soon and signmessage will be enabled. In the meantime you may find the vls code helpful in deriving the secret and signing the message. After that it was easily to find the signing code. It works when doing a copy and paste, however when trying to use the vls code directly it fails as trait fields are private. If they were public it would be as simple as: # cargo.toml
vls-core = "0.9.1-rc.2"
# node_api.rs
use lightning_signer::signer::derive::{KeyDerive, NativeKeyDerive};
async fn sign_message(&self, message: &str) -> Result<String> {
let native_key_derive = NativeKeyDerive { network: bitcoin::network::constants::Network::Bitcoin };
let (_pubkey_from_node_id, seckey) = KeyDerive::node_keys(&native_key_derive, &self.secret, &Secp256k1::new());
println!("_pubkey_from_node_id: {} ", hex::encode(_pubkey_from_node_id.serialize()));
Ok(sign(message.as_bytes(), &seckey).unwrap())
} However since the NativeKeyDerive trait doesn't have a public constructor i'll go ahead and do the copy & paste approach for now, which will just have to get cleaned out once the signer's sign_message works as expected for signing lightning messages. |
164e447
to
c1038c8
Compare
bf0fe2c
to
5440aca
Compare
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.
Some small comments about cleaning up.
libs/sdk-react-native/android/src/main/java/com/breezsdk/BreezSDKMapper.kt
Outdated
Show resolved
Hide resolved
libs/sdk-react-native/android/src/main/java/com/breezsdk/BreezSDKMapper.kt
Outdated
Show resolved
Hide resolved
libs/sdk-react-native/android/src/main/java/com/breezsdk/BreezSDKModule.kt
Outdated
Show resolved
Hide resolved
libs/sdk-react-native/android/src/main/java/com/breezsdk/BreezSDKModule.kt
Outdated
Show resolved
Hide resolved
libs/sdk-react-native/android/src/main/java/com/breezsdk/BreezSDKModule.kt
Outdated
Show resolved
Hide resolved
libs/sdk-react-native/android/src/main/java/com/breezsdk/BreezSDKModule.kt
Outdated
Show resolved
Hide resolved
9770d78
to
bab3bd3
Compare
|
Renamed |
Added sign/check message to the flutter bridge |
ba64975
to
f9cced6
Compare
|
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.
Looks good! Left 2 nitty comments
@JssDWt looks good! |
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.
Generally LGTM, left a few comments.
I don't think it's a problem that the GL dependencies were updated, especially since the new GL version is needed for this feature.
Fixes #109 |
18c4606
to
bb1b29d
Compare
|
] This commit augments the Breez SDK with message signing and verification facilities. Changes span across the Node API interface, its implementation (Greenlight), BreezServices and the UniFFI binding, with amendments in the FFI definition. - Added sign_message() and check_message() methods to the NodeAPI trait and Greenlight. - Added async calls for these methods in BreezServices. - Exposed said calls via the UniFFI binding for FFI. - Updated the FFI definition to mirror these additions.
Early (partial) commit.
This commit augments the Breez SDK with message signing and verification facilities. Changes span across the Node API interface, its implementation (Greenlight), BreezServices and the UniFFI binding, with amendments in the FFI definition.
Todo:
[ x] formatting
[ x] tests
[ ] etc.