Skip to content

Estimate Gas fix #10559

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

Merged
smartprogrammer93 merged 8 commits into from
Feb 17, 2026
Merged

Estimate Gas fix #10559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4b55a86
Estimate Gas fix
svlachakis Feb 17, 2026
3b443c4
format
svlachakis Feb 17, 2026
0d723b1
format
svlachakis Feb 17, 2026
ae86a8f
test fix
svlachakis Feb 17, 2026
82cd069
Merge branch 'master' into 10552-estimategas
svlachakis Feb 17, 2026
d8720cc
Update src/Nethermind/Nethermind.Blockchain.Test/TransactionProcessor…
smartprogrammer93 Feb 17, 2026
b97a07d
Update src/Nethermind/Nethermind.Evm.Test/Tracing/GasEstimationTests.cs
smartprogrammer93 Feb 17, 2026
1b6fa2c
fix shared method
svlachakis Feb 17, 2026
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
27 changes: 7 additions & 20 deletions src/Nethermind/Nethermind.Blockchain.Test/TransactionProcessorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,40 +466,27 @@ public void Can_estimate_with_destroy_refund_and_below_intrinsic_pre_berlin()
tracer.CalculateAdditionalGasRequired(tx, releaseSpec).Should().Be(24080);
tracer.GasSpent.Should().Be(35228L);
long estimate = estimator.Estimate(tx, block.Header, tracer, out string? err, 0);
estimate.Should().Be(59307);
estimate.Should().Be(54225);
Copy link
Copy Markdown
Contributor Author

@svlachakis svlachakis Feb 17, 2026

Choose a reason for hiding this comment

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

The Can_estimate_with_destroy_refund_and_below_intrinsic_pre_berlin test previously expected an estimate of 59307. With the nesting-aware OutOfGas fix, it now returns 54225 — a lower, more accurate estimate.

At this gas level, the inner CALL to the SELFDESTRUCT contract runs out of gas, but the top-level CREATE transaction still succeeds. The old estimate was inflated because the binary search rejected 54225 due to the inner OOG, even though the transaction completes successfully.

This matches Geth's behavior. Geth’s binary search implementation:
https://github.com/ethereum/go-ethereum/blob/master/eth/gasestimator/gasestimator.go

uses result.Failed() to decide whether to raise or lower the gas bound. Failed() is defined in: https://github.com/ethereum/go-ethereum/blob/master/core/state_transition.go

as:

result.Err != nil

Err reflects only the top-level EVM outcome. An inner call Out-Of-Gas is handled internally by the CALL opcode (which pushes 0 onto the stack) and does not propagate to result.Err. Therefore, Geth would accept 54225 as a valid estimate.

The test validation was changed from ConfirmEnoughEstimate (which uses GethLikeTxTracer and asserts no OOG at any call depth) to inline CallOutputTracer.StatusCode checks that verify the top-level transaction succeeds at the estimate and fails below it — matching what Geth actually validates.

From Geth’s gasestimator.go:

"It is a bit pointless to return a perfect estimation, as changing network conditions require the caller to bump it up anyway. Since wallets tend to use 20-25% bump, allowing a small approximation error is fine (as long as it's upwards)."

Copy link
Copy Markdown
Contributor

@smartprogrammer93 smartprogrammer93 Feb 17, 2026

Choose a reason for hiding this comment

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

best to confirm this against geth

Copy link
Copy Markdown
Contributor

@smartprogrammer93 smartprogrammer93 Feb 17, 2026

Choose a reason for hiding this comment

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

I can confirm, geth returns the same exact value as the new implementation we have here.

Assert.That(err, Is.Null);

ConfirmEnoughEstimate(tx, block, estimate);
}

private void ConfirmEnoughEstimate(Transaction tx, Block block, long estimate)
{
CallOutputTracer outputTracer = new();
tx.GasLimit = estimate;
TestContext.Out.WriteLine(tx.GasLimit);

GethLikeTxMemoryTracer gethTracer = new(tx, GethTraceOptions.Default);
var blkCtx = new BlockExecutionContext(block.Header, _specProvider.GetSpec(block.Header));
_transactionProcessor.CallAndRestore(tx, blkCtx, gethTracer);
string traceEnoughGas = new EthereumJsonSerializer().Serialize(gethTracer.BuildResult(), true);

CallOutputTracer outputTracer = new();
tx.GasLimit = estimate;
_transactionProcessor.CallAndRestore(tx, blkCtx, outputTracer);
traceEnoughGas.Should().NotContain("OutOfGas");
outputTracer.StatusCode.Should().Be(StatusCode.Success,
$"transaction should succeed at the estimate ({estimate})");

outputTracer = new CallOutputTracer();
tx.GasLimit = Math.Min(estimate - 1, estimate * 63 / 64);
TestContext.Out.WriteLine(tx.GasLimit);

gethTracer = new GethLikeTxMemoryTracer(tx, GethTraceOptions.Default);
_transactionProcessor.CallAndRestore(tx, blkCtx, gethTracer);

string traceOutOfGas = new EthereumJsonSerializer().Serialize(gethTracer.BuildResult(), true);
TestContext.Out.WriteLine(traceOutOfGas);

_transactionProcessor.CallAndRestore(tx, blkCtx, outputTracer);

bool failed = traceEnoughGas.Contains("failed") || traceEnoughGas.Contains("OutOfGas");
failed.Should().BeTrue();
outputTracer.StatusCode.Should().Be(StatusCode.Failure,
$"transaction should fail below the estimate ({tx.GasLimit})");
}

[TestCase]
Expand Down
11 changes: 7 additions & 4 deletions src/Nethermind/Nethermind.Blockchain/Tracing/EstimateGasTracer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,14 @@ public void ReportActionError(EvmExceptionType exceptionType, long gasLeft)

public override void ReportOperationError(EvmExceptionType error)
{
OutOfGas |= error == EvmExceptionType.OutOfGas;

if (error == EvmExceptionType.Revert && _currentNestingLevel == 0)
if (_currentNestingLevel == 0)
{
TopLevelRevert = true;
OutOfGas |= error == EvmExceptionType.OutOfGas;

if (error == EvmExceptionType.Revert)
{
TopLevelRevert = true;
}
}
}

Expand Down
Loading