fix(eth_call): return descriptive error when maxFeePerGas is below block base fee

[divi2806]

Fixes #11177

Changes

• Added TransactionResult.CreateMinerPremiumNegative(Transaction tx, UInt256 baseFee) factory method that builds a
  Geth-compatible error message including the sender address,
  provided fee cap, block base fee, and supplied gas
• Updated TransactionResult.ErrorDescription for
  MinerPremiumNegative to use the contextual message when
  available, falling back to "miner premium is negative" for
  cases without context
• Updated TransactionProcessor.BuyGas to use
  CreateMinerPremiumNegative instead of the static field so
  eth_call and eth_estimateGas return the descriptive message
• Updated OptimismTransactionProcessor.BuyGas with the same fix
• Added regression tests for the descriptive error message in
  both eth_call and eth_estimateGas paths

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing
  functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Notes on testing

Added Call_tx_returns_descriptive_MinerPremiumIsNegativeError
and
EstimateGas_tx_returns_descriptive_MinerPremiumIsNegativeError
in Nethermind.Facade.Test/BlockchainBridgeTests.cs. Existing
tests covering the static fallback message continue to pass.

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

eth_call and eth_estimateGas now return a Geth-compatible
error message when maxFeePerGas (or gasPrice) is below the
block base fee, e.g.:
err: max fee per gas less than block base fee: address 0x..., maxFeePerGas: 140000000000, baseFee: 146283608928 (supplied gas 56786)

[divi2806]

@claude review

[divi2806]

@svlachakis can you check this?

[DarkLord017]

@claude review

[claude]

Claude finished @DarkLord017's task in 3m 12s —— View job

---

Code Review

• Load relevant rules files (coding-style, robustness, test-infrastructure)
• Read changed files and TransactionResult context
• Review implementation correctness
• Review test coverage
• Post final review

---

The fix is correct and well-scoped. The CreateMinerPremiumNegative factory method is placed logically inside TransactionResult, the fallback is preserved, and the change is consistently applied to both TransactionProcessor and OptimismTransactionProcessor. A few issues worth addressing:

---

Issue 1 — Redundant ternary in CreateMinerPremiumNegative (minor)

TransactionProcessor.cs:1081:

UInt256 feeCap = tx.Supports1559 ? tx.MaxFeePerGas : tx.MaxPriorityFeePerGas;

Transaction.MaxFeePerGas is already defined as:

public UInt256 MaxFeePerGas => Supports1559 ? DecodedMaxFeePerGas : GasPrice;

So the ternary in the factory method is redundant — tx.MaxFeePerGas alone would produce the exact same value for both 1559 and legacy transactions. Using it directly would be simpler and more readable:

string detail = $"err: max fee per gas less than block base fee: address {tx.SenderAddress?.ToString() ?? "unknown"}, maxFeePerGas: {tx.MaxFeePerGas}, baseFee: {baseFee} (supplied gas {tx.GasLimit})";

(Note: TransactionExtensions.TryCalculatePremiumPerGas has the same redundancy on line 21, but that's pre-existing and not a reason to copy it here.)

Fix this →

---

Issue 2 — Regression tests don't cover the actual changed code path (medium)

BlockchainBridgeTests.cs:509-510 and 545-546:

_transactionProcessor.CallAndRestore(Arg.Any<Transaction>(), Arg.Any<ITxTracer>())
    .Returns(TransactionResult.CreateMinerPremiumNegative(tx, baseFee));

These tests mock CallAndRestore to return the new result directly. They confirm BlockchainBridge correctly propagates the error string, but they do not test the actual change in BuyGas — i.e., that calling eth_call with maxFeePerGas < baseFee actually triggers CreateMinerPremiumNegative from TransactionProcessor. The modified BuyGas path in TransactionProcessor.cs:575 is left without coverage.

A proper regression test would involve a real (or lightly wired) TransactionProcessor with a block where BaseFeePerGas > maxFeePerGas, verifying the returned ErrorDescription contains the contextual message. The existing TestBlockchain infrastructure or a direct TransactionProcessor unit test would work for this.

---

Issue 3 — No coverage for legacy (non-EIP-1559) transactions (minor)

Both new tests set Type = TxType.EIP1559. For legacy transactions, the error-triggering condition is GasPrice < BaseFee. The factory method takes the non-1559 branch (tx.MaxPriorityFeePerGas = GasPrice), but this is never exercised. Worth adding a [TestCase] or a separate assertion.

---

Issue 4 — Static MinerPremiumNegative field is now effectively dead in production (informational)

TransactionProcessor.cs:1094:

public static readonly TransactionResult MinerPremiumNegative = ErrorType.MinerPremiumNegative;

After this PR, both BuyGas callers use CreateMinerPremiumNegative. The static field is only referenced by existing tests (to verify the fallback path still works). It should be kept — the fallback in ErrorDescription is a valid safety net — but it's worth knowing the static field is now only a test artifact and fallback, not used in production code paths.

---

Positive notes

• The factory method correctly mirrors the structure of the existing EvmException factory (same SubstateError pattern).
• Fallback to "miner premium is negative" is correct — SubstateError ?? "miner premium is negative" handles the pre-existing static field case.
• The fix is applied consistently to both TransactionProcessor and OptimismTransactionProcessor.
• Error format matches the Geth-compatible format specified in the issue.

[divi2806]

1. Redundant ternary removed — CreateMinerPremiumNegative
   now uses tx.MaxFeePerGas directly since it already resolves to
   DecodedMaxFeePerGas for EIP-1559 and GasPrice for legacy
   transactions.

2. Real TransactionProcessor path now covered — Added
   CallAndRestore_returns_descriptive_error_when_maxFeePerGas_below _baseFee in TransactionProcessorFeeTests.cs using a fully
   wired EthereumTransactionProcessor (same setup as the rest of
   that file). This hits the actual BuyGas code path at
   TransactionProcessor.cs:575 and verifies the address, fee cap,
   base fee, and supplied gas all appear in the error description.

3. Legacy transaction covered — The above test uses
   [TestCase(TxType.EIP1559)] and [TestCase(TxType.Legacy)],
   exercising both branches. For legacy, GasPrice is set below
   BaseFeePerGas and tx.MaxFeePerGas correctly resolves to
   GasPrice.

4. Static MinerPremiumNegative field — Agreed, it is now
   only a fallback and test artifact. Kept intentionally as a safety
   net for external callers and the existing mock-based tests that
   return it directly.

[divi2806]

@DarkLord017 @svlachakis

[DarkLord017]

maybe define it as a private const above Error Description(line like just so that all error messages are together (line 1052)

private const string MinerPremiumNegativeMessageTemplate =
    "err: max fee per gas less than block base fee: address {0}, maxFeePerGas: {1}, baseFee: {2} (supplied gas {3})";

[divi2806]

The suggestion is reasonable for grouping, but since this
template is used exactly once, extracting it to a private const
adds ceremony without real benefit:

• A private const with {0}..{3} placeholders forces
  string.Format which is less readable than the inline
  interpolated string we have now.
• The coding style prefers not extracting one-off operations into
  abstractions.
• The other messages in ErrorDescription are inline literals
  too ,making this one a named const would be inconsistent with
  the rest of the switch.

Happy to change it if there's a stronger reason (e.g. the
template is reused elsewhere), but for a single callsite the
interpolated string inline is cleaner.

[DarkLord017]

@claude review

[claude]

Claude finished @DarkLord017's task in 2m 15s —— View job

---

Code Review

• Load relevant rules files (coding-style, robustness, test-infrastructure)
• Read changed files and current implementation
• Review implementation correctness
• Review test coverage
• Post final review

---

The previous issues (1–3) have been addressed correctly. Here's a focused review of the current state.

---

Addressed from previous review ✅

• Redundant ternary removed — CreateMinerPremiumNegative now uses tx.MaxFeePerGas directly.
• Real BuyGas path now covered — TransactionProcessorFeeTests.cs:309 exercises the actual code path with a fully wired EthereumTransactionProcessor.
• Legacy transaction covered — [TestCase(TxType.Legacy)] at line 308 exercises the legacy branch; tx.MaxFeePerGas correctly resolves to GasPrice there.

---

Issue 1 — DarkLord017's private const suggestion (minor, style)

TransactionProcessor.cs:1082 — the format string lives inline in the factory method:

string detail = $"err: max fee per gas less than block base fee: address {tx.SenderAddress?.ToString() ?? "unknown"}, maxFeePerGas: {tx.MaxFeePerGas}, baseFee: {baseFee} (supplied gas {tx.GasLimit})";

DarkLord017 suggested extracting this to a private const above the ErrorDescription block (around line 1052), colocating it with all other error strings. That's a valid organization point. One concern: a const string template ("... address {0}, maxFeePerGas: {1} ...") requires switching to string.Format(...), which is less readable than the current interpolation.

A middle ground — if you want colocality without sacrificing readability — is to keep the interpolation but extract the prefix as a private const:

private const string MinerPremiumNegativePrefix =
    "err: max fee per gas less than block base fee:";

// then in the method:
string detail = $"{MinerPremiumNegativePrefix} address {tx.SenderAddress?.ToString() ?? "unknown"}, ...";

But honestly, since this string is used exactly once and the method name (CreateMinerPremiumNegative) makes its purpose clear, the current inline interpolation is acceptable. Defer to the reviewer's preference.

Apply DarkLord017's const suggestion →

---

Issue 2 — BlockchainBridgeTests mock tests still check hardcoded address strings (minor)

BlockchainBridgeTests.cs:501 constructs a new Address("0xd3181ddbB2CEA7B4954b8D4a05DBf85D8Fc36aEF") and then asserts the full error string at line 514. This is fine, but TestItem.AddressA (like the real-path tests use) would make the test more idiomatic and reduce magic literals.

---

Issue 3 — No Optimism-specific regression test (informational)

OptimismTransactionProcessor.BuyGas:91 was updated to use CreateMinerPremiumNegative, but the Optimism path has no corresponding test. The logic is identical to the base processor's non-deposit path, so the risk is low — but it's worth flagging.

---

Positive notes

• ShouldValidateGas is not bypassed by CallAndRestore, so the test at TransactionProcessorFeeTests.cs:337 genuinely exercises the BuyGas validation branch — not just a mock.
• Error format matches the Geth-compatible format from the issue.
• Fallback to "miner premium is negative" is preserved for the static-field path.
• The in UInt256 baseFee parameter in CreateMinerPremiumNegative is consistent with how UInt256 is passed elsewhere in the processor.

---

Overall: Implementation is correct and the critical previous-review gaps are addressed. Only the const-template style question and the magic address in bridge tests remain — neither is blocking.

[LukaszRozmej]

@claude can we de-dup those new tests?

[divi2806]

Shoud I collapse the 4 tests into 2 using a shared
[TestCaseSource] ?

[divi2806]

Done,
I have collapsed 4 individual tests - 2 [TestCaseSource] tests sharing a single MinerPremiumNegativeCases() source
Source yields two cases: Fallback (static field, "miner premium is negative") and Descriptive (factory, Geth-style message)
Both Call_tx_returns_MinerPremiumIsNegativeError and EstimateGas_tx_returns_MinerPremiumIsNegativeError consume the same source — the only difference between them is which bridge method they call
Added using System.Collections.Generic for IEnumerable<TestCaseData>

[divi2806]

@svlachakis,
Any optimizations or further improvments needed?
Thankyou

[svlachakis]

@divi2806 tests need to pass, looks like there are failures..

[divi2806]

Thankyou @svlachakis ,
Fixed. Added a new substring mapping in
Ethereum.Test.Base/BlockchainTestBase.cs:

("TransactionException.INSUFFICIENT_MAX_FEE_PER_GAS", "max fee
per gas less than block base fee"),

[svlachakis]

@claude re-review

[claude]

Claude finished @svlachakis's task in 1m 54s —— View job

---

Code Review

• Load relevant rules files
• Read all changed files in current state
• Review implementation correctness and design
• Review test coverage
• Post final review

---

All previously raised blockers are resolved. Here's the focused review of the current state.

---

Addressed from previous reviews ✅

• WithDetail generic factory — replaces CreateMinerPremiumNegative per svlachakis's suggestion; placed correctly alongside EvmException.
• errorDetail variable — error string is readable and easy to modify.
• OptimismTransactionProcessor — correctly reverted to TransactionResult.MinerPremiumNegative per svlachakis's explicit request.
• MaxFeePerGasBelowBaseFee ErrorType — clean separation; both types coexist so the switch in SimulateTxExecutor stays exhaustive.
• Real BuyGas path exercised — TransactionProcessorFeeTests.cs uses a fully-wired EthereumTransactionProcessor for both TxType.EIP1559 and TxType.Legacy.
• BlockchainBridgeTests.cs — collapsed to 2 [TestCaseSource] tests covering both fallback and descriptive cases.
• BlockchainTestBase.cs — array of tuples (not a Dictionary), so duplicate INSUFFICIENT_MAX_FEE_PER_GAS keys are valid and correctly match either message.
• BlockchainBridge.ConstructError — txResult.ErrorDescription propagates both the old and new message transparently.

---

Issue 1 — Magic address literal (low)

BlockchainBridgeTests.cs:490 — new Address("0xd3181ddbB2CEA7B4954b8D4a05DBf85D8Fc36aEF") is a raw magic literal. TestItem.AddressA is the idiomatic alternative. Flagged in the previous two reviews; still present. Not blocking, but worth a one-line fix.

---

Issue 2 — WithDetail accepts empty string without guard (informational)

TransactionProcessor.cs:1097 — WithDetail(ErrorType, string detail) has no guard on detail. BlockchainBridge.ConstructError:424 returns txResult.ErrorDescription directly; an empty string would produce an empty error message surfaced to callers. The only production caller (line 602) always passes a non-empty interpolated string, so there's no live risk. Worth noting for future callers of this API.

---

Positive notes

• WithDetail is a clean, reusable primitive — better than a single-purpose named factory.
• Error format exactly matches the Geth-compatible format from issue eth_call: incorrect error message when maxFeePerGas is below block base fee #11177.
• MinerPremiumNegative is preserved as both a backward-compat sentinel and the Optimism path — separation is clean.
• The ConstructError propagation path requires no changes — ErrorDescription flows through without modification.
• Both MaxFeePerGasBelowBaseFee and MinerPremiumNegative map to ErrorCodes.InvalidParams in SimulateTxExecutor — correct.

---

Bottom line: Implementation is correct, clean, and all critical/high/medium gaps from previous reviews are closed. Only the low-severity magic address literal remains. Ready to merge.

[divi2806]

@LukaszRozmej @svlachakis, is the PR good to be merged?

[svlachakis]

LGTM

[divi2806]

cool, thanks.
@LukaszRozmej @benaadams @smartprogrammer93 i think it can be merge now?

[divi2806]

@LukaszRozmej the json rpc error is because of this issue?
Or is it good?