Skip to content

Signmessage #8226

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
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Signmessage #8226

wants to merge 3 commits into from

Conversation

Lagrang3
Copy link
Collaborator

@Lagrang3 Lagrang3 commented Apr 10, 2025

Add a new rpc called signmessagewithkey that can be used to sign messages using any key
from our wallet. You cannot directly select the key to use, but instead you provide a bitcoin address
and the wallet will figure out which of our keys corresponds to that bitcoin address.

Solves issue #8199

TODO:

  • add verification command
  • add tests
  • add documentation

@Lagrang3
Copy link
Collaborator Author

Lagrang3 commented Apr 11, 2025

There is a variety of signature schemes in the market.
But it seems that Electrum's is somehow accepted as a standard and that seems to be
what Ocean accepts as well.

Therefore I implemented Electrum's signature scheme:

signature = base64(SigRec(SHA256(SHA256(
    "\x18Bitcoin Signed Message:\n" + var_int(len(message)) + message
))))

I checked against sparrow wallet and the verification succeeds.

$ l1-cli signmessagewithkey "some_string" "bcrt1q30zh8czhw4vzcv78t3ddv7zthtsw7w6wpgqz6z"
{
   "bech32": "bcrt1q30zh8czhw4vzcv78t3ddv7zthtsw7w6wpgqz6z",
   "keyidx": 1,
   "pubkey": "02fa14da5717ee05cd1eaf31ff871b8278f0b5dbe3a77ef3bf607c81b7391febe2",
   "signature": "204b7748da7641b218e0e3369f10c12bfd42fa39ed87aa27006499b8f5238b627b22f1d2c3f11de9b577a313760a9c5009fb91ed3d05480e782662559485a78fd7",
   "base64": "IEt3SNp2QbIY4OM2nxDBK/1C+jnth6onAGSZuPUji2J7IvHSw/Ed6bV3oxN2CpxQCfuR7T0FSA54JmJVlIWnj9c="
}

Screenshot from 2025-04-11 10-28-06

@Lagrang3
Copy link
Collaborator Author

In the future if it becomes necessary one could add a flag to select the signature scheme of choice.

Also for reviewers: I was tempted to make hsmd sign any message fromwire, adding more flexibility in
the signing format. But I didn't because I am not sure how much of a security vulnerability that could represent.
As a matter of fact in the comments of handle_sign_message in hsmd/libhsmd.c one can read
that prefixing the message save us from signing unwanted data.

@Lagrang3 Lagrang3 marked this pull request as ready for review April 11, 2025 09:46
@Lagrang3 Lagrang3 requested a review from cdecker as a code owner April 11, 2025 09:46
@Lagrang3 Lagrang3 force-pushed the signmessage branch 3 times, most recently from d291224 to 0a82edd Compare April 11, 2025 09:59
@vincenzopalazzo vincenzopalazzo self-requested a review April 11, 2025 10:33
@vincenzopalazzo vincenzopalazzo self-assigned this Apr 11, 2025
@luke-jr
Copy link

luke-jr commented Apr 11, 2025

Electrum-style is probably the worst of the options accepted by OCEAN fwiw

BIP 322 will be required if you need to support Taproot in the future.

BIP 137 would be probably trivial to add here (just a version byte change)

@Lagrang3 Lagrang3 force-pushed the signmessage branch 2 times, most recently from ec989c7 to 0024b60 Compare April 14, 2025 10:49
@ShahanaFarooqui ShahanaFarooqui added this to the v25.05 milestone Apr 15, 2025
with custom keys.

Changelog-Added: HSMD: add new wire api to sign messages with bitcoin wallet keys.

Signed-off-by: Lagrang3 <[email protected]>
To validate BIP137 signatures produced by core-lightning in tests.

Changelog-None.

Signed-off-by: Lagrang3 <[email protected]>
signmessagewithkey: allows to sign a message with a key associated with
one bitcoin address in our wallet.

Changelog-Added: add a new rpc command signmessagewithkey to sign input messages with keys from our wallet.

Signed-off-by: Lagrang3 <[email protected]>
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.

5 participants