diff --git a/src/Nethermind/Nethermind.Blockchain.Test/TransactionProcessorTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/TransactionProcessorTests.cs index 267fad42efd..8b3a9a81acf 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/TransactionProcessorTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/TransactionProcessorTests.cs @@ -331,7 +331,16 @@ public long Should_not_estimate_tx_with_high_value(UInt256 txValue) GasEstimator estimator = new(_transactionProcessor, _stateProvider, _specProvider, blocksConfig); long estimate = estimator.Estimate(tx, block.Header, tracer, out string? err, 0); - Assert.That(err, Is.Null); + + if (txValue > AccountBalance) + { + Assert.That(err, Is.Not.Null); // Should have error + Assert.That(err, Is.EqualTo("Transaction execution fails")); + } + else + { + Assert.That(err, Is.Null); // Should succeed + } return estimate; } diff --git a/src/Nethermind/Nethermind.Blockchain/Tracing/EstimateGasTracer.cs b/src/Nethermind/Nethermind.Blockchain/Tracing/EstimateGasTracer.cs index 1d657c3e794..2a3b74b2d2a 100644 --- a/src/Nethermind/Nethermind.Blockchain/Tracing/EstimateGasTracer.cs +++ b/src/Nethermind/Nethermind.Blockchain/Tracing/EstimateGasTracer.cs @@ -38,6 +38,8 @@ public EstimateGasTracer() public byte StatusCode { get; set; } + public bool OutOfGas { get; private set; } + public override void MarkAsSuccess(Address recipient, GasConsumed gasSpent, byte[] output, LogEntry[] logs, Hash256? stateRoot = null) { @@ -105,6 +107,7 @@ public override void ReportAction(long gas, UInt256 value, Address from, Address { if (_currentNestingLevel == -1) { + OutOfGas = false; IntrinsicGasAt = gas; } @@ -131,14 +134,21 @@ public override void ReportActionEnd(long gas, Address deploymentAddress, ReadOn public override void ReportActionError(EvmExceptionType exceptionType) { + ReportOperationError(exceptionType); UpdateAdditionalGas(); } public void ReportActionError(EvmExceptionType exceptionType, long gasLeft) { + ReportOperationError(exceptionType); UpdateAdditionalGas(gasLeft); } + public override void ReportOperationError(EvmExceptionType error) + { + OutOfGas |= error == EvmExceptionType.OutOfGas || error == EvmExceptionType.Revert; + } + private void UpdateAdditionalGas(long? gasLeft = null) { if (_isInPrecompile) diff --git a/src/Nethermind/Nethermind.Blockchain/Tracing/GasEstimator.cs b/src/Nethermind/Nethermind.Blockchain/Tracing/GasEstimator.cs index 971c4bf1f2c..89ae4bfb8ee 100644 --- a/src/Nethermind/Nethermind.Blockchain/Tracing/GasEstimator.cs +++ b/src/Nethermind/Nethermind.Blockchain/Tracing/GasEstimator.cs @@ -4,7 +4,6 @@ using System.Threading; using Nethermind.Config; using Nethermind.Core; -using Nethermind.Core.Crypto; using Nethermind.Core.Specs; using Nethermind.Evm; using Nethermind.Evm.TransactionProcessing; @@ -62,7 +61,24 @@ public long Estimate(Transaction tx, BlockHeader header, EstimateGasTracer gasTr UInt256 senderBalance = _stateProvider.GetBalance(tx.SenderAddress); if (tx.ValueRef != UInt256.Zero && tx.ValueRef > senderBalance && !tx.IsSystem()) { - return gasTracer.CalculateAdditionalGasRequired(tx, releaseSpec); + long additionalGas = gasTracer.CalculateAdditionalGasRequired(tx, releaseSpec); + if (additionalGas == 0) + { + // If no additional gas can help, it's an insufficient balance error + if (gasTracer.OutOfGas) + { + err = "Gas estimation failed due to out of gas"; + } + else if (gasTracer.StatusCode == StatusCode.Failure) + { + err = gasTracer.Error ?? "Transaction execution fails"; + } + else + { + err = "insufficient balance"; + } + } + return additionalGas; } var lowerBound = IntrinsicGasCalculator.Calculate(tx, releaseSpec).MinimalGas; @@ -82,19 +98,21 @@ public long Estimate(Transaction tx, BlockHeader header, EstimateGasTracer gasTr } // Execute binary search to find the optimal gas estimation. - return BinarySearchEstimate(leftBound, rightBound, tx, header, gasTracer, errorMargin, token); + return BinarySearchEstimate(leftBound, rightBound, tx, header, gasTracer, errorMargin, token, out err); } private long BinarySearchEstimate(long leftBound, long rightBound, Transaction tx, BlockHeader header, - EstimateGasTracer gasTracer, int errorMargin, CancellationToken token) + EstimateGasTracer gasTracer, int errorMargin, CancellationToken token, out string? err) { + err = null; double marginWithDecimals = errorMargin == 0 ? 1 : errorMargin / 10000d + 1; + //This approach is similar to Geth, by starting from an optimistic guess the number of iterations is greatly reduced in most cases long optimisticGasEstimate = (long)((gasTracer.GasSpent + gasTracer.TotalRefund + GasCostOf.CallStipend) * marginWithDecimals); if (optimisticGasEstimate > leftBound && optimisticGasEstimate < rightBound) { - if (TryExecutableTransaction(tx, header, optimisticGasEstimate, token)) + if (TryExecutableTransaction(tx, header, optimisticGasEstimate, token, gasTracer)) rightBound = optimisticGasEstimate; else leftBound = optimisticGasEstimate; @@ -106,7 +124,7 @@ private long BinarySearchEstimate(long leftBound, long rightBound, Transaction t && leftBound + 1 < rightBound) { long mid = (leftBound + rightBound) / 2; - if (!TryExecutableTransaction(tx, header, mid, token)) + if (!TryExecutableTransaction(tx, header, mid, token, gasTracer)) { leftBound = mid; } @@ -116,8 +134,21 @@ private long BinarySearchEstimate(long leftBound, long rightBound, Transaction t } } - if (rightBound == cap && !TryExecutableTransaction(tx, header, rightBound, token)) + if (rightBound == cap && !TryExecutableTransaction(tx, header, rightBound, token, gasTracer)) { + // Set error based on failure reason + if (gasTracer.OutOfGas) + { + err = "Gas estimation failed due to out of gas"; + } + else if (gasTracer.StatusCode == StatusCode.Failure) + { + err = gasTracer.Error ?? "Transaction execution fails"; + } + else + { + err = "Transaction execution fails"; + } return 0; } @@ -125,50 +156,15 @@ private long BinarySearchEstimate(long leftBound, long rightBound, Transaction t } private bool TryExecutableTransaction(Transaction transaction, BlockHeader block, long gasLimit, - CancellationToken token) + CancellationToken token, EstimateGasTracer gasTracer) { - OutOfGasTracer tracer = new(); - Transaction txClone = new Transaction(); transaction.CopyTo(txClone); txClone.GasLimit = gasLimit; _transactionProcessor.SetBlockExecutionContext(new(block, _specProvider.GetSpec(block))); - _transactionProcessor.CallAndRestore(txClone, tracer.WithCancellation(token)); - - return !tracer.OutOfGas; - } - - private class OutOfGasTracer : TxTracer - { - public OutOfGasTracer() - { - OutOfGas = false; - } - - public override bool IsTracingReceipt => true; - public override bool IsTracingInstructions => true; - public override bool IsTracingActions => true; - public bool OutOfGas { get; private set; } + TransactionResult result = _transactionProcessor.CallAndRestore(txClone, gasTracer.WithCancellation(token)); - public override void MarkAsSuccess(Address recipient, GasConsumed gasSpent, byte[] output, LogEntry[] logs, - Hash256? stateRoot = null) - { - } - - public override void MarkAsFailed(Address recipient, GasConsumed gasSpent, byte[] output, string? error, - Hash256? stateRoot = null) - { - } - - public override void ReportActionError(EvmExceptionType error) - { - OutOfGas |= error == EvmExceptionType.OutOfGas; - } - - public override void ReportOperationError(EvmExceptionType error) - { - OutOfGas |= error == EvmExceptionType.OutOfGas; - } + return result.TransactionExecuted && gasTracer.StatusCode == StatusCode.Success && !gasTracer.OutOfGas; } } diff --git a/src/Nethermind/Nethermind.Evm.Test/Tracing/GasEstimationTests.cs b/src/Nethermind/Nethermind.Evm.Test/Tracing/GasEstimationTests.cs index 6dab4255e23..51bcd2cfc4c 100644 --- a/src/Nethermind/Nethermind.Evm.Test/Tracing/GasEstimationTests.cs +++ b/src/Nethermind/Nethermind.Evm.Test/Tracing/GasEstimationTests.cs @@ -7,6 +7,7 @@ using Nethermind.Blockchain.Tracing; using Nethermind.Config; using Nethermind.Core; +using Nethermind.Core.Crypto; using Nethermind.Core.Extensions; using Nethermind.Core.Specs; using Nethermind.Core.Test; @@ -51,7 +52,7 @@ public void Does_not_take_into_account_precompiles() testEnvironment.tracer.ReportActionEnd(600, Array.Empty()); testEnvironment.estimator.Estimate(tx, block.Header, testEnvironment.tracer, out string? err).Should().Be(0); - Assert.That(err, Is.Null); + Assert.That(err, Is.EqualTo("Transaction execution fails")); } [Test] @@ -81,7 +82,7 @@ public void Handles_well_top_level() testEnvironment.tracer.ReportActionEnd(600, Array.Empty()); testEnvironment.estimator.Estimate(tx, block.Header, testEnvironment.tracer, out string? err).Should().Be(0); - Assert.That(err, Is.Null); + Assert.That(err, Is.EqualTo("Transaction execution fails")); } [Test] @@ -202,7 +203,6 @@ public void Handles_well_precompile_out_of_gas() reportError.Should().NotThrow(); } - [Test] public void Handles_well_nested_calls_where_most_nested_defines_excess() { @@ -284,7 +284,6 @@ public void Estimate_UseErrorMarginOutsideBounds_ThrowArgumentOutOfRangeExceptio Assert.That(err, Is.Not.Null); } - [TestCase(Transaction.BaseTxGasCost, GasEstimator.DefaultErrorMargin, false)] [TestCase(Transaction.BaseTxGasCost, 100, false)] [TestCase(Transaction.BaseTxGasCost, 1000, false)] @@ -341,7 +340,7 @@ public void Estimate_UseErrorMargin_EstimationResultIsNotExact() MainnetSpecProvider.Instance, new BlocksConfig()); - long result = sut.Estimate(tx, block.Header, tracer, out string? err, GasEstimator.DefaultErrorMargin); + long result = sut.Estimate(tx, block.Header, tracer, out string? err); using (Assert.EnterMultipleScope()) { @@ -375,6 +374,244 @@ public void Estimate_UseZeroErrorMargin_EstimationResultIsExact() } } + [Test] + public void Should_return_zero_when_out_of_gas_detected_during_estimation() + { + TestEnvironment testEnvironment = new(); + Transaction tx = Build.A.Transaction.WithGasLimit(100000).TestObject; + Block block = Build.A.Block.WithNumber(1).WithTransactions(tx).TestObject; + + testEnvironment.tracer.ReportAction(1000, 0, Address.Zero, Address.Zero, Array.Empty(), + ExecutionType.TRANSACTION, false); + + testEnvironment.tracer.ReportActionError(EvmExceptionType.OutOfGas); + + testEnvironment.tracer.MarkAsSuccess(Address.Zero, 500, Array.Empty(), Array.Empty()); + + long estimate = testEnvironment.estimator.Estimate(tx, block.Header, testEnvironment.tracer, out string? err); + + estimate.Should().Be(0, "Should return 0 when OutOfGas is detected"); + err.Should().NotBeNull("Error message should be provided when OutOfGas is detected"); + testEnvironment.tracer.OutOfGas.Should().BeTrue("OutOfGas should be set to true"); + } + + [Test] + public void Should_return_zero_when_status_code_is_failure() + { + TestEnvironment testEnvironment = new(); + Transaction tx = Build.A.Transaction.WithGasLimit(100000).TestObject; + Block block = Build.A.Block.WithNumber(1).WithTransactions(tx).TestObject; + + testEnvironment.tracer.ReportAction(1000, 0, Address.Zero, Address.Zero, Array.Empty(), + ExecutionType.TRANSACTION, false); + + testEnvironment.tracer.MarkAsFailed(Address.Zero, 500, Array.Empty(), "execution failed"); + + long estimate = testEnvironment.estimator.Estimate(tx, block.Header, testEnvironment.tracer, out string? err); + + estimate.Should().Be(0, "Should return 0 when StatusCode is Failure"); + err.Should().NotBeNull("Error message should be provided when transaction always fails"); + testEnvironment.tracer.StatusCode.Should().Be(StatusCode.Failure); + } + + [Test] + public void Should_return_positive_estimate_when_no_failure_conditions() + { + TestEnvironment testEnvironment = new(); + Transaction tx = Build.A.Transaction.WithGasLimit(128).TestObject; + Block block = Build.A.Block.WithNumber(1).WithTransactions(tx).TestObject; + + testEnvironment.tracer.ReportAction(128, 0, Address.Zero, Address.Zero, Array.Empty(), + ExecutionType.TRANSACTION, false); + testEnvironment.tracer.ReportAction(100, 0, Address.Zero, Address.Zero, Array.Empty(), ExecutionType.CALL, false); + testEnvironment.tracer.ReportActionEnd(63, Array.Empty()); + testEnvironment.tracer.ReportActionEnd(65, Array.Empty()); + + testEnvironment.tracer.MarkAsSuccess(Address.Zero, 63, Array.Empty(), Array.Empty()); + + long estimate = testEnvironment.estimator.Estimate(tx, block.Header, testEnvironment.tracer, out string? err); + estimate.Should().Be(1, "Should match the Easy_one_level_case result"); + err.Should().BeNull("No error should occur"); + testEnvironment.tracer.OutOfGas.Should().BeFalse("No OutOfGas should be detected"); + testEnvironment.tracer.StatusCode.Should().Be(StatusCode.Success, "StatusCode should be Success"); + } + + [Test] + public void Should_return_zero_with_insufficient_balance_error_when_sender_is_address_zero_with_value_transfer() + { + TestEnvironment testEnvironment = new(); + Transaction tx = Build.A.Transaction + .WithGasLimit(100000) + .WithSenderAddress(Address.Zero) + .WithValue(1.Ether()) // Value transfer with zero balance + .TestObject; + Block block = Build.A.Block.WithNumber(1).WithTransactions(tx).TestObject; + + // Address.Zero has zero balance by default in test environment + EstimateGasTracer tracer = new(); + tracer.MarkAsFailed(Address.Zero, 0, Array.Empty(), "insufficient balance"); + + long estimate = testEnvironment.estimator.Estimate(tx, block.Header, tracer, out string? err); + + estimate.Should().Be(0, "Should return 0 when Address.Zero has insufficient balance for value transfer"); + err.Should().Be("insufficient balance", "Should provide insufficient balance error message"); + } + + [Test] + public void Should_return_zero_with_out_of_gas_error_when_address_zero_runs_out_of_gas() + { + TestEnvironment testEnvironment = new(); + Transaction tx = Build.A.Transaction + .WithGasLimit(100000) + .WithSenderAddress(Address.Zero) + .WithValue(1.Ether()) + .TestObject; + Block block = Build.A.Block.WithNumber(1).WithTransactions(tx).TestObject; + + EstimateGasTracer tracer = new(); + tracer.ReportAction(100000, 0, Address.Zero, Address.Zero, Array.Empty(), ExecutionType.TRANSACTION, false); + tracer.ReportActionError(EvmExceptionType.OutOfGas); + tracer.MarkAsFailed(Address.Zero, 100000, Array.Empty(), "out of gas"); + + long estimate = testEnvironment.estimator.Estimate(tx, block.Header, tracer, out string? err); + + estimate.Should().Be(0, "Should return 0 when Address.Zero transaction runs out of gas"); + err.Should().Be("Gas estimation failed due to out of gas", "Should provide out of gas error message"); + } + + [Test] + public void Should_return_zero_with_execution_failure_when_address_zero_transaction_always_fails() + { + TestEnvironment testEnvironment = new(); + Transaction tx = Build.A.Transaction + .WithGasLimit(100000) + .WithSenderAddress(Address.Zero) + .WithValue(1.Ether()) + .TestObject; + Block block = Build.A.Block.WithNumber(1).WithTransactions(tx).TestObject; + + EstimateGasTracer tracer = new(); + tracer.ReportAction(100000, 0, Address.Zero, Address.Zero, Array.Empty(), ExecutionType.TRANSACTION, false); + tracer.MarkAsFailed(Address.Zero, 50000, Array.Empty(), "execution reverted"); + + long estimate = testEnvironment.estimator.Estimate(tx, block.Header, tracer, out string? err); + + estimate.Should().Be(0, "Should return 0 when Address.Zero transaction always fails"); + err.Should().Be("execution reverted", "Should provide the specific execution failure message"); + } + + [Test] + public void Should_succeed_when_address_zero_has_no_value_transfer() + { + TestEnvironment testEnvironment = new(); + Transaction tx = Build.A.Transaction + .WithGasLimit(100000) + .WithSenderAddress(Address.Zero) + .WithValue(0) // No value transfer - should work even with zero balance + .TestObject; + Block block = Build.A.Block.WithNumber(1).WithTransactions(tx).TestObject; + + testEnvironment.tracer.ReportAction(100000, 0, Address.Zero, Address.Zero, Array.Empty(), ExecutionType.TRANSACTION, false); + testEnvironment.tracer.ReportActionEnd(79000, Array.Empty()); + testEnvironment.tracer.MarkAsSuccess(Address.Zero, 21000, Array.Empty(), Array.Empty()); + + long estimate = testEnvironment.estimator.Estimate(tx, block.Header, testEnvironment.tracer, out string? err); + + estimate.Should().BeGreaterThan(0, "Should succeed when Address.Zero has no value transfer"); + err.Should().BeNull("No error should occur for Address.Zero with no value transfer"); + } + + [Test] + public void Should_return_zero_when_address_zero_exceeds_gas_limits() + { + TestEnvironment testEnvironment = new(); + Transaction tx = Build.A.Transaction + .WithGasLimit(21000) // Very low gas limit + .WithSenderAddress(Address.Zero) + .WithValue(0) + .TestObject; + Block block = Build.A.Block.WithNumber(1).WithTransactions(tx).WithGasLimit(21000).TestObject; + + EstimateGasTracer tracer = new(); + // Simulate gas spent exceeding available limits + tracer.ReportAction(21000, 0, Address.Zero, Address.Zero, Array.Empty(), ExecutionType.TRANSACTION, false); + tracer.MarkAsSuccess(Address.Zero, 25000, Array.Empty(), Array.Empty()); + + long estimate = testEnvironment.estimator.Estimate(tx, block.Header, tracer, out string? err); + + estimate.Should().Be(0, "Should return 0 when gas spent exceeds limits"); + err.Should().Be("Cannot estimate gas, gas spent exceeded transaction and block gas limit", + "Should provide gas limit exceeded error message"); + } + + [Test] + public void Should_estimate_gas_successfully_ignoring_precompile_costs() + { + TestEnvironment testEnvironment = new(); + Transaction tx = Build.A.Transaction.WithGasLimit(30000).WithSenderAddress(TestItem.AddressA).TestObject; + Block block = Build.A.Block.WithNumber(1).WithTransactions(tx).TestObject; + + testEnvironment.tracer.ReportAction(30000, 0, TestItem.AddressA, Address.Zero, Array.Empty(), + ExecutionType.TRANSACTION, false); + testEnvironment.tracer.ReportAction(28000, 0, TestItem.AddressA, Address.Zero, Array.Empty(), + ExecutionType.CALL, true); + testEnvironment.tracer.ReportActionEnd(26000, Array.Empty()); + testEnvironment.tracer.ReportActionEnd(25000, Array.Empty()); + + long result = testEnvironment.estimator.Estimate(tx, block.Header, testEnvironment.tracer, out string? err); + result.Should().BeGreaterThan(0, "Should estimate positive gas, ignoring precompile costs"); + Assert.That(err, Is.Null); + } + + [Test] + public void Should_estimate_gas_successfully_for_simple_transaction() + { + TestEnvironment testEnvironment = new(); + Transaction tx = Build.A.Transaction.WithGasLimit(30000).WithSenderAddress(TestItem.AddressA).TestObject; + Block block = Build.A.Block.WithNumber(1).WithTransactions(tx).TestObject; + + testEnvironment.tracer.ReportAction(30000, 0, TestItem.AddressA, Address.Zero, Array.Empty(), + ExecutionType.TRANSACTION, false); + testEnvironment.tracer.ReportActionEnd(28000, Array.Empty()); + + long result = testEnvironment.estimator.Estimate(tx, block.Header, testEnvironment.tracer, out string? err); + result.Should().BeGreaterThan(0, "Should estimate positive gas for successful transaction"); + Assert.That(err, Is.Null); + } + + [TestCase(50_000, false)] + [TestCase(500_000, false)] + [TestCase(1_000_000, false)] + [TestCase(1_100_000, true)] + public void Should_estimate_gas_for_explicit_gas_check_and_revert(long gasLimit, bool shouldSucceed) + { + TestEnvironment testEnvironment = new(); + Address contractAddress = TestItem.AddressB; + var check = 1_000_000; + byte[] contractCode = Bytes.FromHexString($"0x62{check:x6}5a10600f576001600055005b6000806000fd"); + testEnvironment.InsertContract(contractAddress, contractCode); + + Transaction tx = Build.A.Transaction + .WithGasLimit(gasLimit) + .WithTo(contractAddress) + .WithSenderAddress(TestItem.AddressA) + .TestObject; + Block block = Build.A.Block + .WithNumber(MainnetSpecProvider.ByzantiumBlockNumber + 1) // Ensure opcode `REVERT` is available + .WithTransactions(tx).TestObject; + long result = testEnvironment.estimator.Estimate(tx, block.Header, testEnvironment.tracer, out string? err); + + if (shouldSucceed) + { + result.Should().BeGreaterThan(1_000_000, "Gas estimation should account for the gas threshold in the contract"); + err.Should().BeNull(); + } + else + { + err.Should().NotBeNull("Gas estimation should fail when the gas limit is too low"); + } + } + private class TestEnvironment : IDisposable { public ISpecProvider _specProvider; @@ -405,6 +642,14 @@ public TestEnvironment() estimator = new(_transactionProcessor, _stateProvider, _specProvider, blocksConfig); } + public void InsertContract(Address contractAddress, byte[] code) + { + _stateProvider.CreateAccount(contractAddress, 0); + _stateProvider.InsertCode(contractAddress, ValueKeccak.Compute(code), code, _specProvider.GenesisSpec); + _stateProvider.Commit(_specProvider.GenesisSpec); + _stateProvider.CommitTree(0); + } + public void Dispose() { _closer.Dispose(); diff --git a/src/Nethermind/Nethermind.Facade.Test/BlockchainBridgeTests.cs b/src/Nethermind/Nethermind.Facade.Test/BlockchainBridgeTests.cs index 7fafed8be99..bc57ead6406 100644 --- a/src/Nethermind/Nethermind.Facade.Test/BlockchainBridgeTests.cs +++ b/src/Nethermind/Nethermind.Facade.Test/BlockchainBridgeTests.cs @@ -586,7 +586,7 @@ public void Call_tx_returns_noError() } [Test] - public void EstimateGas_tx_returns_noError() + public void EstimateGas_invalid_tx_returns_error() { BlockHeader header = Build.A.BlockHeader .TestObject; @@ -597,7 +597,7 @@ public void EstimateGas_tx_returns_noError() CallOutput callOutput = _blockchainBridge.EstimateGas(header, tx, 1); - Assert.That(callOutput.Error, Is.Null); + Assert.That(callOutput.Error, Is.Not.Null); } [Test]