Closed eelanagaraj closed 1 year ago
Coverage from tests in ./e2e_test/...
for ./consensus/istanbul/...
at commit 87d69dcc02ae2f9525a8355db52141e6f131ca64
coverage: 63.2% of statements in consensus/istanbul coverage: 42.7% of statements in consensus/istanbul/announce coverage: 55.8% of statements in consensus/istanbul/backend coverage: 0.0% of statements in consensus/istanbul/backend/backendtest coverage: 24.3% of statements in consensus/istanbul/backend/internal/replica coverage: 64.9% of statements in consensus/istanbul/core coverage: 50.0% of statements in consensus/istanbul/db coverage: 0.0% of statements in consensus/istanbul/proxy coverage: 64.4% of statements in consensus/istanbul/uptime coverage: 51.8% of statements in consensus/istanbul/validator coverage: 79.2% of statements in consensus/istanbul/validator/random
Test failures: |
---|
TestTransactionPoolUnderpricing: core
|
This test report was produced by the test-summary action. Made with ❤️ in Cambridge. |
Patch coverage: 14.28%
and project coverage change: +0.09%
:tada:
Comparison is base (
0ed594e
) 55.10% compared to head (a358ea9
) 55.19%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Description
Fixes transaction tracing bug where the tracer did not include the block randomness for block whose transaction was being traced. For
(tx_i, block_n)
, it would fetch the state forblock_n-1
and then reexecute all txs up untiltx_i
, but would omit the random commitment at the very start ofblock_n
.This affects the following endpoints:
TraceTransaction
TraceBlock*
StandardTraceBlock*
IntermediateRoots
TraceChain
Note:
TraceCall
performs correctly since this applies the tx at the end of the requested block, so the block randomness of the next block is irrelevant.Implementation note
eth/tracers/api.go
) but this results in even more messiness because theeth/state_accessor/stateAtTransaction
function needs to have the solution duplicatedOne alternative here is just directly updating the
stateAtTransaction
function and then updating the API to usestateAtTransaction
with the block & 0th index transaction to set up the state properly pre-tracing. I thought this was potentially a bit unclear + more likely to be accidentally overwritten in future merges but happy to reconsider!Factors out
ApplyBlockRandomnessTx
fromcore/state_processor/Process
so that it can be used bystate_accessor
as well.Tested
Unfortunately, testing this via an e2e/unit test is extremely involved, since tracing a simple interaction with
Random.sol
will not suffice. It appears necessary to trace a transaction where a contract interacts withRandom.sol
(tracing a non-view call), and deploying a contract manually in the test feels unnecessary.TraceChain
& not using thelive
database for multiple blocks 🤔Backwards compatibility
This changes the behavior of the tracing API endpoints but is not expected to be a consensus-relevant breaking change.