Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ public class BlobTransactionForRpc : EIP1559TransactionForRpc, IFromTransaction<
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public byte[][]? Blobs { get; set; }

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public byte[][]? Commitments { get; set; }
Comment thread
svlachakis marked this conversation as resolved.

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public byte[][]? Proofs { get; set; }

[JsonConstructor]
public BlobTransactionForRpc() { }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public abstract class TransactionForRpc
/// the type to match the target block's fork rules.
/// </summary>
[JsonIgnore]
internal bool IsTypeDefaulted { get; set; }
public bool IsTypeDefaulted { get; internal set; }

[JsonConstructor]
protected TransactionForRpc() { }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only

using System;
using System.Threading.Tasks;
using FluentAssertions;
using Nethermind.Core;
using Nethermind.Crypto;
using Nethermind.Facade.Eth.RpcTransaction;
using Nethermind.Int256;
using Nethermind.JsonRpc.Client;
using Nethermind.JsonRpc.Data;
using Nethermind.Serialization.Rlp;
using NUnit.Framework;

namespace Nethermind.JsonRpc.Test.Modules.Eth;

public partial class EthRpcModuleTests
{
// Address derived from PrivateKey 0x00..01 by WalletExtensions.SetupTestAccounts
private const string UnlockedTestAccount = "0x7e5f4552091a69125d5dfcb7b8c2659029395bdf";
private const string LockedAccount = "0x000000000000000000000000000000000000dead";
private const string FeeFieldsMissingMessage = "missing gasPrice or maxFeePerGas/maxPriorityFeePerGas";

[TestCase(TxType.Legacy, "gas", "gas not specified", TestName = "GasMissing")]
[TestCase(TxType.Legacy, "gasPrice", FeeFieldsMissingMessage, TestName = "LegacyFeesMissing")]
[TestCase(TxType.EIP1559, "maxFeePerGas", FeeFieldsMissingMessage, TestName = "Eip1559MaxFeePerGasMissing")]
[TestCase(TxType.EIP1559, "maxPriorityFeePerGas", FeeFieldsMissingMessage, TestName = "Eip1559MaxPriorityFeePerGasMissing")]
[TestCase(TxType.Legacy, "nonce", "nonce not specified", TestName = "NonceMissing")]
Comment thread
svlachakis marked this conversation as resolved.
Outdated
public async Task SignTransaction_WhenRequiredFieldMissing_ReturnsInvalidInput(TxType type, string omitField, string expectedMessage)
{
TransactionForRpc rpcTx = BuildTx(type, omitField);
string response = await SignTransaction(rpcTx);

response.Should().Contain($"\"code\":{ErrorCodes.InvalidInput}",
"missing required field must surface as InvalidInput so callers can branch on it");
response.Should().Contain(expectedMessage,
"error message must be precise so callers know which field to fix");
}

[TestCase(LockedAccount, null, TestName = "WrongAccount")]
[TestCase(null, "from", TestName = "FromMissing")]
public async Task SignTransaction_WhenSenderNotUnlocked_ReturnsAuthError(string? fromOverride, string? omitField)
{
// Missing-from defaults to Address.Zero; both paths fail the IsUnlocked check with the same response.
TransactionForRpc rpcTx = BuildTx(TxType.Legacy, omitField, fromOverride);
string response = await SignTransaction(rpcTx);

response.Should().Contain($"\"code\":{ErrorCodes.InvalidInput}",
"wallet lookup failure surfaces as -32000 to align with keystore error handling");
response.Should().Contain("authentication needed: password or unlock",
"wording must match keystore error so tools that text-match keep working");
}

[Test]
public async Task SignTransaction_WhenTotalFeeExceedsCap_ReturnsInvalidInput()
{
EIP1559TransactionForRpc rpcTx = (EIP1559TransactionForRpc)BuildTx(TxType.EIP1559);
rpcTx.MaxFeePerGas = (UInt256)50_000_000_000_000UL; // 50000 gwei * 30400 gas = 1.52 ETH > 1 ETH cap
rpcTx.MaxPriorityFeePerGas = (UInt256)1_000_000_000UL;

string response = await SignTransaction(rpcTx);

response.Should().Contain("exceeds the configured cap",
"fees above RpcTxFeeCap must be rejected before signing — DOS / fat-finger guard");
}

[Test]
public async Task SignTransaction_WhenBlobTxMissingCommitments_ReturnsInvalidInput()
{
byte[] versionedHash = new byte[32];
versionedHash[0] = 0x01;
BlobTransactionForRpc rpcTx = new()
{
From = new Address(UnlockedTestAccount),
To = new Address("0x2d44c0e097f6cd0f514edac633d82e01280b4a5c"),
Gas = 0x76c0,
Nonce = (UInt256)0,
MaxFeePerGas = (UInt256)0x9184e72a000,
MaxPriorityFeePerGas = (UInt256)0x3b9aca00,
MaxFeePerBlobGas = (UInt256)1_000_000,
BlobVersionedHashes = [versionedHash],
Blobs = [new byte[131072]],
};

string response = await SignTransaction(rpcTx);

response.Should().Contain("commitments must be provided alongside blobs",
"blob signing without commitments must surface a precise error so callers know what to add");
}

[TestCase(TxType.Legacy, typeof(LegacyTransactionForRpc), TestName = "Legacy")]
[TestCase(TxType.EIP1559, typeof(EIP1559TransactionForRpc), TestName = "Eip1559")]
public async Task SignTransaction_WhenValid_RawRoundTripsAndTxEcho(TxType type, Type expectedEchoType)
{
TransactionForRpc rpcTx = BuildTx(type);
SignTransactionResult result = await SignTransactionForResult(rpcTx);

Transaction decoded = TxDecoder.Instance.DecodeCompleteNotNull(
result.Raw,
RlpBehaviors.AllowUnsigned | RlpBehaviors.SkipTypedWrapping | RlpBehaviors.InMempoolForm);

decoded.Type.Should().Be(type, "type must round-trip through RLP encode/decode");
decoded.GasLimit.Should().Be(0x76c0L, "gas must round-trip exactly — caller provided it explicitly");
decoded.Nonce.Should().Be((UInt256)0, "nonce must round-trip — caller provided it explicitly");

Address recovered = new EthereumEcdsa(decoded.ChainId ?? 1).RecoverAddress(decoded)!;
recovered.Should().Be(new Address(UnlockedTestAccount),
"signature must recover to the from address — raw is the canonical signed artifact");

result.Tx.Should().BeOfType(expectedEchoType,
"tx echo must preserve subclass so JSON shape survives for clients that branch on type");
}

private async Task<string> SignTransaction(TransactionForRpc rpcTx)
{
using Context ctx = await Context.Create();
return await ctx.Test.TestEthRpc("eth_signTransaction", rpcTx);
}

private async Task<SignTransactionResult> SignTransactionForResult(TransactionForRpc rpcTx)
{
using Context ctx = await Context.Create();
string serialized = await ctx.Test.TestEthRpc("eth_signTransaction", rpcTx);
JsonRpcResponse<SignTransactionResult> response = ctx.Test.JsonSerializer.Deserialize<JsonRpcResponse<SignTransactionResult>>(serialized)!;
response.Result.Should().NotBeNull("precondition: signing must succeed for valid input");
return response.Result!;
}

private static TransactionForRpc BuildTx(TxType type, string? omitField = null, string? fromOverride = null)
{
Address? from = fromOverride is not null
? new Address(fromOverride)
: (omitField == "from" ? null : new Address(UnlockedTestAccount));

Address to = new("0x2d44c0e097f6cd0f514edac633d82e01280b4a5c");
UInt256 value = 0x9184e72a;
long gas = 0x76c0;
UInt256 nonce = 0;

return type == TxType.EIP1559
? new EIP1559TransactionForRpc
{
From = from,
To = to,
Value = value,
Gas = omitField == "gas" ? null : gas,
Nonce = omitField == "nonce" ? null : nonce,
MaxFeePerGas = omitField == "maxFeePerGas" ? null : (UInt256?)0x9184e72a000,
MaxPriorityFeePerGas = omitField == "maxPriorityFeePerGas" ? null : (UInt256?)0x3b9aca00,
}
: new LegacyTransactionForRpc
{
From = from,
To = to,
Value = value,
Gas = omitField == "gas" ? null : gas,
Nonce = omitField == "nonce" ? null : nonce,
GasPrice = omitField == "gasPrice" ? null : (UInt256?)0x9184e72a000,
};
}
}
16 changes: 16 additions & 0 deletions src/Nethermind/Nethermind.JsonRpc/Data/SignTransactionResult.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only

using System.Text.Json.Serialization;
using Nethermind.Facade.Eth.RpcTransaction;

namespace Nethermind.JsonRpc.Data;

public class SignTransactionResult
{
[JsonPropertyName("raw")]
public byte[] Raw { get; init; } = null!;

[JsonPropertyName("tx")]
public TransactionForRpc Tx { get; init; } = null!;
}
3 changes: 3 additions & 0 deletions src/Nethermind/Nethermind.JsonRpc/IJsonRpcConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@ public interface IJsonRpcConfig : IConfig
[ConfigItem(Description = "The error margin used in the `eth_estimateGas` JSON-RPC method, in basis points.", DefaultValue = "150")]
int EstimateErrorMargin { get; set; }

[ConfigItem(Description = "Maximum total tx fee (gasPrice * gasLimit, in wei) the node will sign in eth_signTransaction. 0 disables the cap. Default 1 ETH.", DefaultValue = "1000000000000000000")]
ulong RpcTxFeeCap { get; set; }
Comment thread
svlachakis marked this conversation as resolved.

[ConfigItem(Description = "The JSON-RPC server CORS origins.", DefaultValue = "*")]
string[] CorsOrigins { get; set; }

Expand Down
1 change: 1 addition & 0 deletions src/Nethermind/Nethermind.JsonRpc/JsonRpcConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public string[] EnabledModules
public long? MaxBatchResponseBodySize { get; set; } = 32.MiB;
public long? MaxSimulateBlocksCap { get; set; } = 256;
public int EstimateErrorMargin { get; set; } = 150;
public ulong RpcTxFeeCap { get; set; } = 1_000_000_000_000_000_000UL;
Comment thread
svlachakis marked this conversation as resolved.
Outdated
public string[] CorsOrigins { get; set; } = ["*"];
public int WebSocketsProcessingConcurrency { get; set; } = 1;
public int IpcProcessingConcurrency { get; set; } = 1;
Expand Down
134 changes: 134 additions & 0 deletions src/Nethermind/Nethermind.JsonRpc/Modules/Eth/EthRpcModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
using System.Text.Json.Nodes;
using System.Threading;
using System.Threading.Tasks;
using Nethermind.Crypto;
using Block = Nethermind.Core.Block;
using BlockHeader = Nethermind.Core.BlockHeader;
using ResultType = Nethermind.Core.ResultType;
Expand Down Expand Up @@ -355,6 +356,139 @@ public virtual async Task<ResultWrapper<Hash256>> eth_sendRawTransaction(byte[]
}
}

public virtual ResultWrapper<SignTransactionResult> eth_signTransaction(TransactionForRpc rpcTx)
{
if (rpcTx.Gas is null)
return ResultWrapper<SignTransactionResult>.Fail("gas not specified", ErrorCodes.InvalidInput);

if (!HasFeeFields(rpcTx))
return ResultWrapper<SignTransactionResult>.Fail("missing gasPrice or maxFeePerGas/maxPriorityFeePerGas", ErrorCodes.InvalidInput);

LegacyTransactionForRpc? legacy = rpcTx as LegacyTransactionForRpc;
if (legacy?.Nonce is null)
return ResultWrapper<SignTransactionResult>.Fail("nonce not specified", ErrorCodes.InvalidInput);

Address from = legacy.From ?? Address.Zero;
Comment thread
svlachakis marked this conversation as resolved.
if (!_wallet.IsUnlocked(from))
return ResultWrapper<SignTransactionResult>.Fail("authentication needed: password or unlock", ErrorCodes.InvalidInput);

rpcTx = PromoteToEip1559IfDefaultLegacy(rpcTx);

Result<Transaction> txResult = rpcTx.ToTransaction(validateUserInput: true);
if (!txResult.Success(out Transaction tx, out string error))
return ResultWrapper<SignTransactionResult>.Fail(error, ErrorCodes.InvalidInput);

ulong chainId = _blockchainBridge.GetChainId();
tx.ChainId = chainId;

ResultWrapper<SignTransactionResult>? feeCapError = CheckTxFeeCap(tx);
if (feeCapError is not null)
return feeCapError;

// Sidecar must be attached before encode; signing only sets tx.Signature so the wrapper survives.
if (rpcTx is BlobTransactionForRpc blobTx)
{
string? attachError = TryAttachBlobSidecar(tx, blobTx);
if (attachError is not null)
return ResultWrapper<SignTransactionResult>.Fail(attachError, ErrorCodes.InvalidInput);
}

try
{
_wallet.Sign(tx, chainId);
}
catch (SecurityException)
{
return ResultWrapper<SignTransactionResult>.Fail("authentication needed: password or unlock", ErrorCodes.InvalidInput);
Comment thread
svlachakis marked this conversation as resolved.
}
Comment on lines +402 to +413
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we have exceptionless TrySign?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried but it's too much for this PR.

Wallet API has Sign + TrySign duplication and the catch only handles SecurityException (Clef throws InvalidOperationException, NullWallet returns null differently). A proper fix needs an IWallet redesign.

One redesign approach is:

single Sign(Hash256, Address) returning null, TrySignTransaction virtual on the interface for Clef's account_signTransaction override, SignMessage as extension. That's a larger refactor with cascading impact across IWallet impls and other RPC handlers; better as a focused follow-up than bundled here.

Will open a separate issue/PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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


tx.Hash = tx.CalculateHash();

byte[] raw = TxDecoder.Instance.Encode(tx, RlpBehaviors.SkipTypedWrapping | RlpBehaviors.InMempoolForm).Bytes;

return ResultWrapper<SignTransactionResult>.Success(new SignTransactionResult
{
Raw = raw,
Tx = TransactionForRpc.FromTransaction(tx)
});
}

private static bool HasFeeFields(TransactionForRpc rpcTx)
{
if (rpcTx is EIP1559TransactionForRpc eip1559
&& eip1559.MaxFeePerGas is not null
&& eip1559.MaxPriorityFeePerGas is not null)
{
return true;
}

return rpcTx is LegacyTransactionForRpc legacy && legacy.GasPrice is not null;
}
Comment thread
svlachakis marked this conversation as resolved.
Outdated

private static TransactionForRpc PromoteToEip1559IfDefaultLegacy(TransactionForRpc rpcTx)
{
if (!rpcTx.IsTypeDefaulted) return rpcTx;
if (rpcTx is AccessListTransactionForRpc or EIP1559TransactionForRpc) return rpcTx;
if (rpcTx is not LegacyTransactionForRpc legacy) return rpcTx;

return new EIP1559TransactionForRpc
{
From = legacy.From,
To = legacy.To,
Value = legacy.Value,
Gas = legacy.Gas,
Nonce = legacy.Nonce,
Input = legacy.Input,
ChainId = legacy.ChainId,
MaxFeePerGas = legacy.GasPrice,
MaxPriorityFeePerGas = legacy.GasPrice,
};
}
Comment thread
svlachakis marked this conversation as resolved.
Outdated

private ResultWrapper<SignTransactionResult>? CheckTxFeeCap(Transaction tx)
{
ulong cap = _rpcConfig.RpcTxFeeCap;
if (cap == 0) return null;

UInt256 perGas = tx.Type >= TxType.EIP1559 ? tx.MaxFeePerGas : tx.GasPrice;
UInt256 totalFee = perGas * (UInt256)tx.GasLimit;
Comment thread
svlachakis marked this conversation as resolved.
Outdated
UInt256 capWei = cap;

if (totalFee <= capWei) return null;

return ResultWrapper<SignTransactionResult>.Fail(
$"tx fee ({FormatWeiAsEther(totalFee)} ether) exceeds the configured cap ({FormatWeiAsEther(capWei)} ether)",
ErrorCodes.InvalidInput);
}

private static string FormatWeiAsEther(UInt256 wei)
{
const ulong WeiPerEther = 1_000_000_000_000_000_000UL;
const ulong WeiPerCenti = 10_000_000_000_000_000UL;
UInt256 ether = wei / (UInt256)WeiPerEther;
UInt256 centiTotal = wei / (UInt256)WeiPerCenti;
UInt256.Mod(centiTotal, (UInt256)100, out UInt256 centi);
return $"{ether}.{(ulong)centi:D2}";
}

private string? TryAttachBlobSidecar(Transaction tx, BlobTransactionForRpc blobTx)
{
if (blobTx.Blobs is null || blobTx.Blobs.Length == 0)
return "blob transaction requires non-empty blobs";
if (blobTx.Commitments is null || blobTx.Commitments.Length != blobTx.Blobs.Length)
return "commitments must be provided alongside blobs (one per blob)";
if (blobTx.Proofs is null)
return "proofs must be provided alongside blobs";
Comment thread
svlachakis marked this conversation as resolved.
Outdated

BlockHeader? head = _blockFinder.Head?.Header;
ProofVersion version = head is null
? ProofVersion.V0
: _specProvider.GetSpec(head).BlobProofVersion;

tx.NetworkWrapper = new ShardBlobNetworkWrapper(blobTx.Blobs, blobTx.Commitments, blobTx.Proofs, version);
return null;
}
Comment thread
svlachakis marked this conversation as resolved.

private async Task<ResultWrapper<Hash256>> SendTx(Transaction tx,
TxHandlingOptions txHandlingOptions = TxHandlingOptions.None)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,13 @@ public interface IEthRpcModule : IRpcModule
)]
Task<ResultWrapper<Hash256>> eth_sendRawTransaction([JsonRpcParameter(ExampleValue = "[\"0xf86380843b9aca0082520894b943b13292086848d8180d75c73361107920bb1a80802ea0385656b91b8f1f5139e9ba3449b946a446c9cfe7adb91b180ddc22c33b17ac4da01fe821879d386b140fd8080dcaaa98b8c709c5025c8c4dea1334609ebac41b6c\"]")] byte[] transaction);

[JsonRpcMethod(IsImplemented = true,
Description = "Signs the transaction using the unlocked sender account and returns the RLP-encoded signed transaction together with the parsed object.",
IsSharable = true,
ExampleResponse = "{\"raw\":\"0x02f86c0182520894b943b13292086848d8180d75c73361107920bb1a80...\",\"tx\":{\"type\":\"0x2\",\"nonce\":\"0x0\",\"gas\":\"0x5208\",\"to\":\"0x...\"}}")]
ResultWrapper<SignTransactionResult> eth_signTransaction(
[JsonRpcParameter(ExampleValue = "[{\"from\":\"0xc2208fe87805279b03c1a8a78d7ee4bfdb0e48ee\",\"to\":\"0x2d44c0e097f6cd0f514edac633d82e01280b4a5c\",\"value\":\"0x9184e72a\",\"gas\":\"0x76c0\",\"gasPrice\":\"0x9184e72a000\",\"nonce\":\"0x0\"}]")] TransactionForRpc rpcTx);

[JsonRpcMethod(IsImplemented = true,
Description = "Executes a tx call (does not create a transaction)",
IsSharable = false,
Expand Down
Loading