Skip to content

Adds message signing to wallet #4286

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+84 −18
Open

Adds message signing to wallet #4286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6297300
Adds message signing to wallet
Nov 7, 2025
642cc9e
Add comment about padding for the "sign_message" command
Nov 8, 2025
3bca0e3
Merge branch 'master' into feature/cli-arbitrary-message-sign
ajara87 Nov 9, 2025
599ce81
Fixes sign command signatures and ambiguous calls bugs
Nov 10, 2025
7ccfbbf
Merge remote-tracking branch 'adrian-fjellberg/feature/cli-arbitrary-…
Nov 10, 2025
211b2b1
Merge branch 'master' into feature/cli-arbitrary-message-sign
Wi1l-B0t Nov 10, 2025
bdc9a28
Renamed CLI command "sign transaction" to "sign tx"
Nov 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 72 additions & 2 deletions src/Neo.CLI/CLI/MainService.Wallet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

using Akka.Actor;
using Neo.ConsoleService;
using Neo.Cryptography;
using Neo.Extensions;
using Neo.Json;
using Neo.Network.P2P.Payloads;
Expand All @@ -27,9 +28,11 @@
using System.Linq;
using System.Numerics;
using System.Security.Cryptography;
using System.Text;
using System.Threading.Tasks;
using static Neo.SmartContract.Helper;
using ECPoint = Neo.Cryptography.ECC.ECPoint;
using ECCurve = Neo.Cryptography.ECC.ECCurve;

namespace Neo.CLI
{
Expand Down Expand Up @@ -475,8 +478,8 @@ private void OnListKeyCommand()
/// <summary>
/// Process "sign" command
/// </summary>
/// <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 tx", Category = "Wallet Commands")]
private void OnSignCommand(JObject jsonObjectToSign)
{
if (NoWallet()) return;
Expand Down Expand Up @@ -508,6 +511,73 @@ private void OnSignCommand(JObject jsonObjectToSign)
}
}

/// <summary>
/// Process "sign message" command
/// </summary>
/// <param name="message">Message to sign</param>
[ConsoleCommand("sign message", Category = "Wallet Commands")]
private void OnSignMessageCommand(string message)
{
if (NoWallet()) return;

string password = ConsoleHelper.ReadUserInput("password", true);
if (password.Length == 0)
{
ConsoleHelper.Info("Cancelled");
return;
}
if (!CurrentWallet!.VerifyPassword(password))
{
ConsoleHelper.Error("Incorrect password");
return;
}

var saltBytes = new byte[16];
RandomNumberGenerator.Fill(saltBytes);
var saltHex = saltBytes.ToHexString().ToLowerInvariant();

var paramBytes = Encoding.UTF8.GetBytes(saltHex + message);

byte[] payload;
using (var ms = new MemoryStream())
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);
Copy link
Member

@shargon shargon Nov 11, 2025

Choose a reason for hiding this comment

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

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

Copy link
Member

@erikzhang erikzhang Nov 11, 2025

Choose a reason for hiding this comment

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

neo/src/Neo/Network/P2P/Helper.cs

Lines 71 to 88 in ac7ebab

public static byte[] GetSignData(this IVerifiable verifiable, uint network)
{
/* Same as:
using MemoryStream ms = new();
using BinaryWriter writer = new(ms);
writer.Write(network);
writer.Write(verifiable.Hash);
writer.Flush();
return ms.ToArray();
*/
var buffer = new byte[SignDataLength];
BinaryPrimitives.WriteUInt32LittleEndian(buffer, network);
verifiable.Hash.Serialize(buffer.AsSpan(sizeof(uint)));
return buffer;
}

Copy link
Member

@shargon shargon Nov 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@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

w.Write((byte)0x00);
w.Write((byte)0x01);
w.Write((byte)0xF0);
// Write the actual message to sign
w.WriteVarBytes(paramBytes);
// We add these 2 bytes to prevent the signature from being a valid transaction
w.Write((ushort)0);
w.Flush();
payload = ms.ToArray();
}

ConsoleHelper.Info("Signed Payload: ", $"{Environment.NewLine}{payload.ToHexString()}");
Console.WriteLine();
ConsoleHelper.Info(" Curve: ", "secp256r1");
ConsoleHelper.Info("Algorithm: ", "010001f0 + VarBytes(Salt + Message) + 0000");
ConsoleHelper.Info(" ", "See the online documentation for details on how to verify this signature.");
ConsoleHelper.Info(" ", "https://developers.neo.org/docs/n3/node/cli/cli#sign_message");
Console.WriteLine();
ConsoleHelper.Info("Generated signatures:");
Console.WriteLine();

foreach (WalletAccount account in CurrentWallet.GetAccounts().Where(p => p.HasKey))
{
var key = account.GetKey();
var signature = Crypto.Sign(payload, key.PrivateKey, ECCurve.Secp256r1);

ConsoleHelper.Info(" Address: ", account.Address);
ConsoleHelper.Info(" PublicKey: ", key.PublicKey.EncodePoint(true).ToHexString());
Copy link
Contributor

@Jim8y Jim8y Nov 8, 2025

Choose a reason for hiding this comment

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

we dont really support secp256k1, the key you get is still p-256 pubkey.

Copy link
Author

@adrian-fjellberg adrian-fjellberg Nov 8, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Member

@erikzhang erikzhang Nov 9, 2025

Choose a reason for hiding this comment

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

We need a structured format that specifies the address, public key, and algorithm for signing. This also facilitates verification.

Copy link
Author

@adrian-fjellberg adrian-fjellberg Nov 9, 2025

Choose a reason for hiding this comment

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

Some questions:

  1. Should we require the user specifiy either the public key or address for signing?

  2. 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.

  3. For specifing the algorithim, do you mean the curve or padding, salt, etc.? Or both?

  4. Can we provide defaults for each option so that the command become user friendly?

Copy link
Member

@erikzhang erikzhang Nov 9, 2025

Choose a reason for hiding this comment

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

I mean we need to define an output format including address, public key, and algorithm of the signature.

Copy link
Author

@adrian-fjellberg adrian-fjellberg Nov 10, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Member

@erikzhang erikzhang Nov 10, 2025

Choose a reason for hiding this comment

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

I want a standard with json format please.

Copy link
Author

@adrian-fjellberg adrian-fjellberg Nov 10, 2025

Choose a reason for hiding this comment

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

@erikzhang I made a draft: neo-project/proposals#210

ConsoleHelper.Info(" Signature: ", signature.ToHexString());
ConsoleHelper.Info(" Salt: ", saltHex);
Console.WriteLine();
}
}

/// <summary>
/// Process "send" command
/// </summary>
Expand Down
28 changes: 12 additions & 16 deletions src/Neo.ConsoleService/ConsoleServiceBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ internal bool OnCommand(string commandLine)

var possibleHelp = "";
var tokens = commandLine.Tokenize();
var availableCommands = new List<(ConsoleCommandMethod Command, object?[] Arguments)>();
var availableCommands = new List<(ConsoleCommandMethod Command, Func<object?[]> Arguments)>();

foreach (var entries in _verbs.Values)
{
foreach (var command in entries)
Expand All @@ -139,19 +140,11 @@ internal bool OnCommand(string commandLine)
if (consumed <= 0) continue;

var args = tokens.Skip(consumed).ToList().Trim();
try
{
if (args.Any(u => u.IsIndicator))
availableCommands.Add((command, ParseIndicatorArguments(command.Method, args)));
else
availableCommands.Add((command, ParseSequentialArguments(command.Method, args)));
}
catch (Exception ex)
{
// Skip parse errors
possibleHelp = command.Key;
ConsoleHelper.Error($"{ex.InnerException?.Message ?? ex.Message}");
}

if (args.Any(u => u.IsIndicator))
availableCommands.Add((command, () => ParseIndicatorArguments(command.Method, args)));
else
availableCommands.Add((command, () => ParseSequentialArguments(command.Method, args)));
}
}

Expand All @@ -167,7 +160,9 @@ internal bool OnCommand(string commandLine)

if (availableCommands.Count == 1)
{
var (command, arguments) = availableCommands[0];
// var (command, arguments) = availableCommands[0];
var (command, getArguments) = availableCommands[0];
var arguments = getArguments();
object? result = command.Method.Invoke(command.Instance, arguments);

if (result is Task task) task.Wait();
Expand All @@ -176,7 +171,8 @@ internal bool OnCommand(string commandLine)

// Show Ambiguous call
var ambiguousCommands = availableCommands.Select(u => u.Command.Key).Distinct().ToList();
throw new ArgumentException($"Ambiguous calls for: {string.Join(',', ambiguousCommands)}");
var ambiguousCommandsQuoted = ambiguousCommands.Select(u => $"'{u}'").ToList();
throw new ArgumentException($"Ambiguous calls for: {string.Join(',', ambiguousCommandsQuoted)}");
}

private bool TryProcessValue(Type parameterType, IList<CommandToken> args, bool consumeAll, out object? value)
Expand Down
Loading