Skip to content

Support various eth_call invocations post 1559 #3317

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

Closed
MansurovB-source wants to merge 39 commits into from
Closed

Support various eth_call invocations post 1559 #3317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
f5bed91
Added new ExecutionOptions for zero gas price eth_call
Aug 6, 2021
4dcc4f4
Changed Call And Restore to separate zero price transactions
Aug 6, 2021
9065595
Changes to "Execute" to identify transactions with commit and restore…
Aug 6, 2021
5574d5a
Changes to "Execute" to process transactions with commit and restore …
Aug 6, 2021
ab32915
Added a new condition for checking maxFee > maxPriorityFee
Aug 6, 2021
8883dd4
Fixed tests
Aug 6, 2021
6fff2c1
Added and fixed tests
Aug 6, 2021
7611928
Added description to the tests
Aug 6, 2021
0a412ff
Fixed condition for "insufficient max fee per gas for sender balance"
Aug 16, 2021
78a20f7
fixed the reason for the crash of tests
Aug 16, 2021
03e0b0e
Fixed eth_call tests
Aug 17, 2021
d2fc7ba
fixed eth_estimateGas tests
Aug 17, 2021
0385a18
fixed the default gas limit value for eth_call and eth_estimateGas
Aug 17, 2021
a4829c2
Added test for checking account balance after successfully executing …
Aug 18, 2021
9d8ac63
Added test
Aug 19, 2021
c7e8bb2
Removed unnecessary parts of QuickFail (deleting an account)
Aug 19, 2021
646834d
The inappropriate test was commented out
Aug 19, 2021
394c190
Refactoring
Aug 25, 2021
7f46a98
fixed special if-statement for CallAndRestore
Aug 25, 2021
78c8f41
fixed the reason for the test failure, refactoring
Aug 25, 2021
8652d2c
Merge remote-tracking branch 'origin/master' into changes_in_eth_call
kjazgar Aug 30, 2021
4c62c77
small fix
Sep 3, 2021
c91715e
fixup! small fix
Sep 3, 2021
7fff105
Behaviour when specifying the gas price and the gas price of type 155…
Sep 6, 2021
ac8f321
Added support for automatic recognition of the transaction type in et…
Sep 7, 2021
800b083
Get-style error output
Sep 13, 2021
9c19612
Removed support for automatic recognition of the transaction type
Sep 13, 2021
e1993b2
Fixed eth_call tests
Sep 13, 2021
11f7cf4
Fixed eth_estimateGas tests
Sep 13, 2021
b3b8739
fixed some tests, added new tests for eth_estimateGas and small refac…
Sep 14, 2021
1e323ed
Added JsonConverter for Nullable TxType
Sep 22, 2021
cd29e01
Added test case
Sep 22, 2021
f831fe1
Added tests for Nullable TxTypeConverter (JSON)
Sep 22, 2021
b257240
Nullable TxType
Sep 22, 2021
0eac807
fixed outdated tests
Sep 22, 2021
77bdd4d
Added support for automati recognition of the transaction type in eth…
Sep 22, 2021
5175f3c
Fixed tests and add new test cases
Sep 22, 2021
367ab1b
Merge branch 'master' into changes_in_eth_call
kjazgar Jan 18, 2022
5126e27
Merged master and changed tests.
kjazgar Jan 19, 2022
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
39 changes: 39 additions & 0 deletions src/Nethermind/Nethermind.Core.Test/Json/NullableTxTypeConverterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright (c) 2021 Demerzel Solutions Limited
// This file is part of the Nethermind library.
//
// The Nethermind library is free software: you can redistribute it and/or modify
// it under the terms of the GNU Lesser General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// The Nethermind library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with the Nethermind. If not, see <http://www.gnu.org/licenses/>.
//

using Nethermind.Serialization.Json;
using NUnit.Framework;

namespace Nethermind.Core.Test.Json
{
[TestFixture]
public class NullableTxTypeConverterTests : ConverterTestBase<TxType?>
{
[TestCase(null)]
[TestCase((TxType)0)]
[TestCase((TxType)15)]
[TestCase((TxType)16)]
[TestCase((TxType)255)]
[TestCase(TxType.Legacy)]
[TestCase(TxType.AccessList)]
[TestCase(TxType.EIP1559)]
public void Test_roundtrip(TxType? arg)
{
TestConverter(arg, (before, after) => before.Equals(after), new NullableTxTypeConverter());
}
}
}
1 change: 1 addition & 0 deletions src/Nethermind/Nethermind.Core.Test/Json/TxTypeConverterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public class TxTypeConverterTests : ConverterTestBase<TxType>
[TestCase((TxType)255)]
[TestCase(TxType.Legacy)]
[TestCase(TxType.AccessList)]
[TestCase(TxType.EIP1559)]
public void Test_roundtrip(TxType arg)
{
TestConverter(arg, (before, after) => before.Equals(after), new TxTypeConverter());
Expand Down
150 changes: 112 additions & 38 deletions src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,17 @@ private enum ExecutionOptions
/// <summary>
/// Commit and later restore state also skip validation, use for CallAndRestore
/// </summary>
CommitAndRestore = Commit | Restore | NoValidation
CommitAndRestore = Commit | Restore | NoValidation,

/// <summary>
/// Zero Gas price
/// </summary>
SkipGasPricingValidation = 8,

/// <summary>
/// Commit and restore with zero gas price
/// </summary>
CommitAndRestoreWithSkippingGasPricingValidation = CommitAndRestore | SkipGasPricingValidation
}

public TransactionProcessor(
Expand Down Expand Up @@ -95,7 +105,14 @@ public TransactionProcessor(

public void CallAndRestore(Transaction transaction, BlockHeader block, ITxTracer txTracer)
{
Execute(transaction, block, txTracer, ExecutionOptions.CommitAndRestore);
bool skipGasPricing = _specProvider.GetSpec(block.Number).IsEip1559Enabled
Copy link
Contributor

@MarekM25 MarekM25 Sep 8, 2021

Choose a reason for hiding this comment

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

Do other clients skip validation when we have GasPrice = 0 or when it is set to null in TransactionForRpc?

? (transaction.IsEip1559
? (transaction.MaxFeePerGas.IsZero && transaction.MaxPriorityFeePerGas.IsZero)
: transaction.GasPrice.IsZero)
: transaction.GasPrice.IsZero;

Execute(transaction, block, txTracer,
skipGasPricing ? ExecutionOptions.CommitAndRestoreWithSkippingGasPricingValidation : ExecutionOptions.CommitAndRestore);
}

public void BuildUp(Transaction transaction, BlockHeader block, ITxTracer txTracer)
Expand All @@ -116,25 +133,51 @@ public void Trace(Transaction transaction, BlockHeader block, ITxTracer txTracer
Execute(transaction, block, txTracer, ExecutionOptions.NoValidation);
}

private void QuickFail(Transaction tx, BlockHeader block, ITxTracer txTracer, bool eip658NotEnabled,
string? reason)
private void QuickFail(Transaction tx, BlockHeader block, ITxTracer txTracer, bool eip658NotEnabled, string? reason, bool deleteCallerAccount = false)
{
block.GasUsed += tx.GasLimit;

Address recipient = tx.To ?? ContractAddress.From(
tx.SenderAddress ?? Address.Zero,
_stateProvider.GetNonce(tx.SenderAddress ?? Address.Zero));
try
{
// block.GasUsed += tx.GasLimit;
//
// Address recipient = tx.To ?? ContractAddress.From(
// tx.SenderAddress ?? Address.Zero,
// _stateProvider.GetNonce(tx.SenderAddress ?? Address.Zero));
//
// if (txTracer.IsTracingReceipt)
// {
// Keccak? stateRoot = null;
// if (eip658NotEnabled)
// {
// _stateProvider.RecalculateStateRoot();
// stateRoot = _stateProvider.StateRoot;
// }
//
// txTracer.MarkAsFailed(recipient, tx.GasLimit, Array.Empty<byte>(), reason ?? "invalid", stateRoot);

block.GasUsed += tx.GasLimit;

Address recipient = tx.To ?? ContractAddress.From(
tx.SenderAddress ?? Address.Zero,
_stateProvider.GetNonce(tx.SenderAddress ?? Address.Zero));

if (txTracer.IsTracingReceipt)
{
Keccak? stateRoot = null;
if (eip658NotEnabled)
{
_stateProvider.RecalculateStateRoot();
stateRoot = _stateProvider.StateRoot;
}

if (txTracer.IsTracingReceipt)
txTracer.MarkAsFailed(recipient, tx.GasLimit, Array.Empty<byte>(), reason ?? "invalid", stateRoot);
}
}
finally
{
Keccak? stateRoot = null;
if (eip658NotEnabled)
if (deleteCallerAccount)
{
_stateProvider.RecalculateStateRoot();
stateRoot = _stateProvider.StateRoot;
_stateProvider.DeleteAccount(tx.SenderAddress ?? throw new InvalidOperationException());
}

txTracer.MarkAsFailed(recipient, tx.GasLimit, Array.Empty<byte>(), reason ?? "invalid", stateRoot);
}
}

Expand All @@ -144,12 +187,16 @@ private void Execute(Transaction transaction, BlockHeader block, ITxTracer txTra
bool eip658NotEnabled = !_specProvider.GetSpec(block.Number).IsEip658Enabled;

// restore is CallAndRestore - previous call, we will restore state after the execution
//bool restore = (executionOptions & ExecutionOptions.Restore) != ExecutionOptions.None;
bool restore = (executionOptions & ExecutionOptions.Restore) == ExecutionOptions.Restore;
bool noValidation = (executionOptions & ExecutionOptions.NoValidation) == ExecutionOptions.NoValidation;
// commit - is for standard execute, we will commit thee state after execution
bool commit = (executionOptions & ExecutionOptions.Commit) == ExecutionOptions.Commit || eip658NotEnabled;
//!commit - is for build up during block production, we won't commit state after each transaction to support rollbacks
//we commit only after all block is constructed
//we commit only after all block is constructed

bool skipGasPricing = (executionOptions & ExecutionOptions.SkipGasPricingValidation) == ExecutionOptions.SkipGasPricingValidation;

bool notSystemTransaction = !transaction.IsSystem();
bool deleteCallerAccount = false;

Expand All @@ -161,10 +208,26 @@ private void Execute(Transaction transaction, BlockHeader block, ITxTracer txTra

UInt256 value = transaction.Value;

if (!transaction.TryCalculatePremiumPerGas(block.BaseFeePerGas, out UInt256 premiumPerGas) && !noValidation)
// if (!transaction.TryCalculatePremiumPerGas(block.BaseFeePerGas, out UInt256 premiumPerGas) && !noValidation)
// {
// TraceLogInvalidTx(transaction, "MINER_PREMIUM_IS_NEGATIVE");
// QuickFail(transaction, block, txTracer, eip658NotEnabled, "miner premium is negative");
// return;
// }
if (!skipGasPricing && restore && transaction.MaxPriorityFeePerGas > transaction.MaxFeePerGas)
{
TraceLogInvalidTx(transaction, "MAX PRIORITY FEE PER GAS HIGHER THAN MAX FEE PER GAS");
QuickFail(transaction, block, txTracer, eip658NotEnabled,
$"max priority fee per gas higher than max fee per gas: address {transaction.SenderAddress}, maxPriorityFeePerGas: {transaction.MaxPriorityFeePerGas}, maxFeePerGas: {transaction.MaxFeePerGas}");
return;
}

if (!transaction.TryCalculatePremiumPerGas(block.BaseFeePerGas, out UInt256 premiumPerGas) && !skipGasPricing && !restore)
{
TraceLogInvalidTx(transaction, "MINER_PREMIUM_IS_NEGATIVE");
QuickFail(transaction, block, txTracer, eip658NotEnabled, "miner premium is negative");
// max fee per gas (feeCap) less than block base fee
TraceLogInvalidTx(transaction, "MAX FEE PER GAS LESS THAN BLOCK BASE FEE");
QuickFail(transaction, block, txTracer, eip658NotEnabled,
$"max fee per gas less than block base fee: address {transaction.SenderAddress}, maxFeePerGas: {transaction.MaxFeePerGas}, baseFee {block.BaseFeePerGas}");
return;
}

Expand Down Expand Up @@ -200,15 +263,16 @@ private void Execute(Transaction transaction, BlockHeader block, ITxTracer txTra
if (gasLimit < intrinsicGas)
{
TraceLogInvalidTx(transaction, $"GAS_LIMIT_BELOW_INTRINSIC_GAS {gasLimit} < {intrinsicGas}");
QuickFail(transaction, block, txTracer, eip658NotEnabled, "gas limit below intrinsic gas");
QuickFail(transaction, block, txTracer, eip658NotEnabled, $"gas limit below intrinsic gas: have {gasLimit}, want {intrinsicGas}");
return;
}

// if (!restore && gasLimit > block.GasLimit - block.GasUsed)
if (!noValidation && gasLimit > block.GasLimit - block.GasUsed)
{
TraceLogInvalidTx(transaction,
$"BLOCK_GAS_LIMIT_EXCEEDED {gasLimit} > {block.GasLimit} - {block.GasUsed}");
QuickFail(transaction, block, txTracer, eip658NotEnabled, "block gas limit exceeded");
QuickFail(transaction, block, txTracer, eip658NotEnabled, $"block gas limit exceeded: gasLimit {gasLimit} > block gasLimit {block.GasLimit} - block gasUsed {block.GasUsed}");
return;
}
}
Expand All @@ -231,6 +295,7 @@ private void Execute(Transaction transaction, BlockHeader block, ITxTracer txTra
else
{
TraceLogInvalidTx(transaction, $"SENDER_ACCOUNT_DOES_NOT_EXIST {caller}");
// if (!commit || restore || effectiveGasPrice == UInt256.Zero)
if (!commit || noValidation || effectiveGasPrice == UInt256.Zero)
{
deleteCallerAccount = !commit || restore;
Expand All @@ -244,36 +309,45 @@ private void Execute(Transaction transaction, BlockHeader block, ITxTracer txTra
$"Failed to recover sender address on tx {transaction.Hash} when previously recovered sender account did not exist.");
}
}
UInt256 senderReservedGasPayment = skipGasPricing ? UInt256.Zero : (ulong) gasLimit * effectiveGasPrice;

UInt256 senderReservedGasPayment = noValidation ? UInt256.Zero : (ulong)gasLimit * effectiveGasPrice;
// UInt256 senderReservedGasPayment = noValidation ? UInt256.Zero : (ulong)gasLimit * effectiveGasPrice;

if (notSystemTransaction)
{
UInt256 senderBalance = _stateProvider.GetBalance(caller);
if (!noValidation && ((ulong)intrinsicGas * effectiveGasPrice + value > senderBalance ||
senderReservedGasPayment + value > senderBalance))
// if (!noValidation && ((ulong)intrinsicGas * effectiveGasPrice + value > senderBalance ||
// senderReservedGasPayment + value > senderBalance))
if (!skipGasPricing && ((ulong) intrinsicGas * effectiveGasPrice + value > senderBalance || senderReservedGasPayment + value > senderBalance))
{
TraceLogInvalidTx(transaction,
$"INSUFFICIENT_SENDER_BALANCE: ({caller})_BALANCE = {senderBalance}");
QuickFail(transaction, block, txTracer, eip658NotEnabled, "insufficient sender balance");
// TraceLogInvalidTx(transaction,
// $"INSUFFICIENT_SENDER_BALANCE: ({caller})_BALANCE = {senderBalance}");
// QuickFail(transaction, block, txTracer, eip658NotEnabled, "insufficient sender balance");
TraceLogInvalidTx(transaction, $"INSUFFICIENT_SENDER_BALANCE: ({caller})_BALANCE = {senderBalance}");
QuickFail(transaction, block, txTracer, eip658NotEnabled, $"insufficient funds for gas * price + value: address {caller}, have {senderBalance}, want {senderReservedGasPayment + value}", deleteCallerAccount && restore);
return;
}

if (!noValidation && spec.IsEip1559Enabled && !transaction.IsFree() &&
senderBalance < (UInt256)transaction.GasLimit * transaction.MaxFeePerGas + value)
// if (!noValidation && spec.IsEip1559Enabled && !transaction.IsFree() &&
// senderBalance < (UInt256)transaction.GasLimit * transaction.MaxFeePerGas + value)
if (!skipGasPricing && spec.IsEip1559Enabled && !transaction.IsServiceTransaction && senderBalance < (UInt256)transaction.GasLimit * transaction.MaxFeePerGas + value)
{
TraceLogInvalidTx(transaction,
$"INSUFFICIENT_MAX_FEE_PER_GAS_FOR_SENDER_BALANCE: ({caller})_BALANCE = {senderBalance}, MAX_FEE_PER_GAS: {transaction.MaxFeePerGas}");
QuickFail(transaction, block, txTracer, eip658NotEnabled,
"insufficient MaxFeePerGas for sender balance");
// TraceLogInvalidTx(transaction,
// $"INSUFFICIENT_MAX_FEE_PER_GAS_FOR_SENDER_BALANCE: ({caller})_BALANCE = {senderBalance}, MAX_FEE_PER_GAS: {transaction.MaxFeePerGas}");
// QuickFail(transaction, block, txTracer, eip658NotEnabled,
// "insufficient MaxFeePerGas for sender balance");
TraceLogInvalidTx(transaction, $"INSUFFICIENT_MAX_FEE_PER_GAS_FOR_SENDER_BALANCE: ({caller})_BALANCE = {senderBalance}, MAX_FEE_PER_GAS: {transaction.MaxFeePerGas}");
QuickFail(transaction, block, txTracer, eip658NotEnabled, $"insufficient MaxFeePerGas for sender balance: address {{caller}}, sender_balance = {senderBalance}, maxFeePerGas: {transaction.MaxFeePerGas}", deleteCallerAccount && restore);
return;
}

if (transaction.Nonce != _stateProvider.GetNonce(caller))
{
TraceLogInvalidTx(transaction,
$"WRONG_TRANSACTION_NONCE: {transaction.Nonce} (expected {_stateProvider.GetNonce(caller)})");
QuickFail(transaction, block, txTracer, eip658NotEnabled, "wrong transaction nonce");
// TraceLogInvalidTx(transaction,
// $"WRONG_TRANSACTION_NONCE: {transaction.Nonce} (expected {_stateProvider.GetNonce(caller)})");
// QuickFail(transaction, block, txTracer, eip658NotEnabled, "wrong transaction nonce");
TraceLogInvalidTx(transaction, $"WRONG_TRANSACTION_NONCE: {transaction.Nonce} (expected {_stateProvider.GetNonce(caller)})");
QuickFail(transaction, block, txTracer, eip658NotEnabled, "wrong transaction nonce", deleteCallerAccount && restore);
return;
}

Expand Down Expand Up @@ -358,7 +432,7 @@ private void Execute(Transaction transaction, BlockHeader block, ITxTracer txTra
}
else
{
// tks: there is similar code fo contract creation from init and from CREATE
// tks: there is similar code for contract creation from init and from CREATE
// this may lead to inconsistencies (however it is tested extensively in blockchain tests)
if (transaction.IsContractCreation)
{
Expand Down
8 changes: 4 additions & 4 deletions src/Nethermind/Nethermind.JsonRpc.Test/Data/Eip2930Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,17 +180,17 @@ public void can_deserialize_not_provided_txType()
{
_transactionForRpc = _serializer.Deserialize<TransactionForRpc>("{\"nonce\":\"0x0\",\"blockHash\":null,\"blockNumber\":null,\"transactionIndex\":null,\"to\":null,\"value\":\"0x0\",\"gasPrice\":\"0x0\",\"gas\":\"0x0\",\"input\":null}");

// if there is not TxType provided, default value should be TxType.Legacy equal 0
_transactionForRpc.Type.Should().Be(0);
// if there is not TxType provided, default value should be null
_transactionForRpc.Type.Should().Be(null);
}

[Test]
public void can_deserialize_direct_null_txType()
{
_transactionForRpc = _serializer.Deserialize<TransactionForRpc>("{\"nonce\":\"0x0\",\"blockHash\":null,\"blockNumber\":null,\"transactionIndex\":null,\"to\":null,\"value\":\"0x0\",\"gasPrice\":\"0x0\",\"gas\":\"0x0\",\"input\":null,\"type\":null}");

// if there is null TxType provided, still default value should be TxType.Legacy equal 0
_transactionForRpc.Type.Should().Be(0);
// if there is null TxType provided, still default value should be null
_transactionForRpc.Type.Should().Be(null);
}

[Test]
Expand Down
Loading