Skip to content
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

JsonRpc standardization: change MinerPremiumNegative error to FeeCapTooLow #8128

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

JsonRpc standardization: change MinerPremiumNegative error to FeeCapTooLow #8128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.Consensus.AuRa/Contracts/TransactionPermissionContractV3.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ protected override object[] GetAllowedTxTypesParameters(Transaction tx, BlockHea

long number = (parentHeader?.Number ?? 0) + 1;
bool isEip1559Enabled = _specProvider.GetSpecFor1559(number).IsEip1559Enabled;
UInt256 gasPrice = isEip1559Enabled && tx.Supports1559 ? tx.MaxFeePerGas : tx.GasPrice;
UInt256 gasPrice = tx.GetGasFeeCap(isEip1559Enabled);

return new object[]
{
Expand Down
5 changes: 5 additions & 0 deletions src/Nethermind/Nethermind.Core/Transaction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,11 @@ public string ToString(string indent)

public bool MayHaveNetworkForm => Type is TxType.Blob;

public UInt256 GetGasFeeCap(bool isEip1559Enabled = true)
{
return isEip1559Enabled && Supports1559 ? MaxFeePerGas : GasPrice;
}

public class PoolPolicy : IPooledObjectPolicy<Transaction>
{
public Transaction Create()
Expand Down
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.Core/TransactionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public static bool IsSystem(this Transaction tx) =>
public static bool TryCalculatePremiumPerGas(this Transaction tx, in UInt256 baseFeePerGas, out UInt256 premiumPerGas)
{
bool freeTransaction = tx.IsFree();
UInt256 feeCap = tx.Supports1559 ? tx.MaxFeePerGas : tx.GasPrice;
UInt256 feeCap = tx.GetGasFeeCap();
if (baseFeePerGas > feeCap)
{
premiumPerGas = UInt256.Zero;
Expand Down
4 changes: 2 additions & 2 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 @@ -462,7 +462,7 @@ protected virtual TransactionResult BuyGas(Transaction tx, BlockHeader header, I
if (!tx.TryCalculatePremiumPerGas(header.BaseFeePerGas, out premiumPerGas))
{
TraceLogInvalidTx(tx, "MINER_PREMIUM_IS_NEGATIVE");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should change MINER_PREMIUM_IS_NEGATIVE log also?

Copy link
Contributor

@alexb5dh alexb5dh Jan 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Does geth show same "max fee per gas less than block base fee..." error message in logs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it is:
WARN [02-03|17:35:21.985] Served eth_call conn=127.0.0.1:51596 reqid=1 duration="395.958µs" err="err: max fee per gas less than block base fee: address 0x32E4E4c7c5d1CEa5db5F9202a9E4D99E56c91a24, maxFeePerGas: 0, baseFee: 0 (supplied gas 50000000)"

Copy link
Contributor

@alexb5dh alexb5dh Feb 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

My suggestion would be to log the same or similar message

return TransactionResult.MinerPremiumNegative;
return TransactionResult.FeeCapTooLow(tx, header.BaseFeePerGas);
}

UInt256 senderBalance = WorldState.GetBalance(tx.SenderAddress!);
Expand Down Expand Up @@ -819,7 +819,7 @@ public readonly struct TransactionResult(string? error) : IEquatable<Transaction
public static readonly TransactionResult InsufficientMaxFeePerGasForSenderBalance = "insufficient MaxFeePerGas for sender balance";
public static readonly TransactionResult InsufficientSenderBalance = "insufficient sender balance";
public static readonly TransactionResult MalformedTransaction = "malformed";
public static readonly TransactionResult MinerPremiumNegative = "miner premium is negative";
public static TransactionResult FeeCapTooLow(Transaction tx, UInt256 baseFee) => $"err: max fee per gas less than block base fee: address {tx.SenderAddress}, maxFeePerGas: {tx.GetGasFeeCap()}, baseFee: {baseFee} (supplied gas {tx.GasLimit})";
public static readonly TransactionResult NonceOverflow = "nonce overflow";
public static readonly TransactionResult SenderHasDeployedCode = "sender has deployed code";
public static readonly TransactionResult SenderNotSpecified = "sender not specified";
Expand Down
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.Optimism/OptimismTransactionProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ protected override TransactionResult BuyGas(Transaction tx, BlockHeader header,
if (!tx.TryCalculatePremiumPerGas(header.BaseFeePerGas, out premiumPerGas))
{
TraceLogInvalidTx(tx, "MINER_PREMIUM_IS_NEGATIVE");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should change MINER_PREMIUM_IS_NEGATIVE log also?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, we can change it

return TransactionResult.MinerPremiumNegative;
return TransactionResult.FeeCapTooLow(tx, header.BaseFeePerGas);
}

if (UInt256.SubtractUnderflow(senderBalance, tx.Value, out UInt256 balanceLeft))
Expand Down
Loading