Fix gas fees accounting in TransactionProcessor.Trace #5743
Conversation
src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs
Show resolved
Hide resolved
| @@ -241,7 +241,7 @@ public void Trace(Transaction transaction, BlockHeader block, ITxTracer txTracer | |||
| } | |||
| } | |||
|
|
|||
| UInt256 senderReservedGasPayment = noValidation ? UInt256.Zero : (ulong)gasLimit * effectiveGasPrice; | |||
| UInt256 senderReservedGasPayment = (ulong)gasLimit * effectiveGasPrice; | |||
There was a problem hiding this comment.
Can you write an explanation for TransactionProcessor changes?
There was a problem hiding this comment.
How it works right now:
- If
noValidationsubtract zero(senderReservedGasPayment) as gas fee - Refund gas. So if
noValidationwe refund gas that we didn't take.
After the fix:
- If
noValidationdo not subtract gas fee at all - Refund only if
noValidationis false. This fixes mentioned above issue
Another issue:
Right now Trace function uses noValidation, so we don't charge gas from account during trace. And as we discussed this behavior shouldn't be changed.
When we execute trace_replayBlock or trace_replayTransaction not charging fees causes inconsistency between chain state and stat that we get in trace. So I changed these functions to use Execute instead of Trace.
| public void Trace(Block block, IBlockTracer tracer) => Process(block, tracer, _traceProcessor); | ||
|
|
||
| public void Execute(Block block, IBlockTracer tracer) => Process(block, tracer, _executeProcessor); | ||
|
|
There was a problem hiding this comment.
Why do we need separated processors?
There was a problem hiding this comment.
Wrote under previous comment
| @@ -118,7 +118,7 @@ protected TestAllTracerWithOutput Execute(long blockNumber, long gasLimit, byte[ | |||
| protected (Block block, Transaction transaction) PrepareTx( | |||
| long blockNumber, | |||
| long gasLimit, | |||
| byte[] code, | |||
| byte[] code = null, | |||
| void Trace(Block block, IBlockTracer tracer); | ||
|
|
||
| /// <summary> | ||
| /// Allows to trace and verify arbitrary constructed block. Subtracts gas from sender account | ||
| /// </summary> | ||
| /// <param name="block">Block to trace.</param> | ||
| /// <param name="tracer">Trace to act on block processing events.</param> | ||
| void Execute(Block block, IBlockTracer tracer); |
There was a problem hiding this comment.
Can you add a write-up (maybe in which JSON RPC uses which and why)
deffrian
deleted the
5680-trace_replayblocktransactions-insufficient-funds
branch
Fixes Closes Resolves #
#5680
Changes
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?