Adds message signing to wallet#4286
Adds message signing to wallet#4286adrian-fjellberg wants to merge 7 commits intoneo-project:masterfrom
Conversation
Adds the ability to sign arbitrary messages using wallet accounts. The user can specify the elliptic curve to use for signing (secp256r1 or secp256k1). It generates a payload using salt and the provided message, then signs the payload with each available wallet account, displaying the address, public key, salt and signature.
| var signature = Crypto.Sign(payload, key.PrivateKey, selectedCurve); | ||
|
|
||
| ConsoleHelper.Info("Address: ", account.Address); | ||
| ConsoleHelper.Info(" PublicKey: ", key.PublicKey.EncodePoint(true).ToHexString()); |
There was a problem hiding this comment.
we dont really support secp256k1, the key you get is still p-256 pubkey.
There was a problem hiding this comment.
Ok. I think I saw in the code for Crypto.Sign that both were supported so that is why I added it.
Do you want me tor remove the option to select curve and only use secp256r1?
There was a problem hiding this comment.
We need a structured format that specifies the address, public key, and algorithm for signing. This also facilitates verification.
There was a problem hiding this comment.
Some questions:
-
Should we require the user specifiy either the public key or address for signing?
-
If yes to question 1: If there is only one address in the open wallet, could we default to using this one? I think this will make signing more user friendly.
-
For specifing the algorithim, do you mean the curve or padding, salt, etc.? Or both?
-
Can we provide defaults for each option so that the command become user friendly?
There was a problem hiding this comment.
I mean we need to define an output format including address, public key, and algorithm of the signature.
There was a problem hiding this comment.
Latest commit includes the algorithm and curve used + some helpful text and link to documentation on how to verify the generated signatures.
@erikzhang, is this what you wanted?
Or did you want a stuctured format in terms of JSON or YAML?
Or did you want us to define a standard for the padding and salting?
There was a problem hiding this comment.
I want a standard with json format please.
There was a problem hiding this comment.
@erikzhang I made a draft: neo-project/proposals#210
| w.Write((byte)0x01); | ||
| w.Write((byte)0x00); | ||
| w.Write((byte)0x01); | ||
| w.Write((byte)0xF0); | ||
| w.WriteVarBytes(paramBytes); | ||
| w.Write((ushort)0); | ||
| w.Flush(); |
There was a problem hiding this comment.
what is the meaning of this script, some standard?
There was a problem hiding this comment.
It is to ensure that the message is not also a valid NEO transaction. I do not think it is a defined standard but it is the way both NEON wallet and NeoLine signs messages.
Do you want me to add comments to the code to clearify?
There was a problem hiding this comment.
I can add comments to the code.
There was a problem hiding this comment.
Comments added in the latest commit.
|
I also started on documenting the command line feature: |
Refactors the `sign message` command to: - allow only signing only using the secp256r1 curve. - output the algorithm used to create the payload - output guide on how to verify the signed message Renames the `sign` command to `sign transaction` to avoid ambiguous calls with the new `sign message` command. Refactors the `OnCommand` method on the ConsoleServiceBase class so that parsing of arguments are differed to later in the execution. This is to solve an issue that occurs when ambiguous calls occur, one method might output errors in when parsing of arguments.
…message-sign' into feature/cli-arbitrary-message-sign
| /// <param name="jsonObjectToSign">Json object to sign</param> | ||
| [ConsoleCommand("sign", Category = "Wallet Commands")] | ||
| /// <param name="jsonObjectToSign">The json string that records the transaction information</param> | ||
| [ConsoleCommand("sign transaction", Category = "Wallet Commands")] |
|
Is this armed for Neo4 or next release? @adrian-fjellberg |
|
|
|
|
As fast as possilbe beacuse it will be important for nodes to be able to do offline signing in a secure way to claim candidate roles in the new Goevernance platform set to be released soon. So I should change to |
| using (var w = new BinaryWriter(ms, Encoding.UTF8, true)) | ||
| { | ||
| // We add these 4 bytes to prevent the signature from being a valid transaction | ||
| w.Write((byte)0x01); |
There was a problem hiding this comment.
I would like to follow something like the https://www.cyfrin.io/blog/understanding-ethereum-signature-standards-eip-191-eip-712, domain separator it's important to avoid signature replays between different chains, like testnet and mainnet
There was a problem hiding this comment.
neo/src/Neo/Network/P2P/Helper.cs
Lines 71 to 88 in ac7ebab
There was a problem hiding this comment.
@adrian-fjellberg we should call this method, let me add some changes. Also this PR should be moved now to https://github.com/neo-project/neo-node
|
I only need the posibilty for nodes to sign an arbitrary message... We get 100% value from the current implementation. It satisfies all the needs of the reason behind the proposed change. The value is:
|
|
The |
|
@adrian-fjellberg No CLI in Neo |
|
Porting to neo-node on master-n3 on PR: neo-project/neo-node#924 |
|
Moved to neo-project/neo-node#924 |
Adds the ability to sign arbitrary messages using wallet accounts. The user can specify the elliptic curve to use for signing (secp256r1 or secp256k1). It generates a payload using salt and the provided message, then signs the payload with each available wallet account, displaying the address, public key, salt and signature.
Description
This PR introduces a new
sign_messagecommand to the Neo CLI, enabling message signing outside of transactions.Previously, the CLI only supported signing serialized transaction data, which made it impossible to produce verifiable message signatures for use in external applications, authentication workflows, or dApp integrations.
With this change, users can now easily sign and verify arbitrary messages directly from their local wallet.
Change Log
Fixes #4285
Type of change
How Has This Been Tested?
sign_messageon local CLI with both curvesChecklist: