Use Remote State to generate traces for Debug API #567
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
m-Peter
force-pushed
the
traces-api-use-remote-state
branch
4 times, most recently
from
f79eb77 to
3e97e28
Compare
m-Peter
force-pushed
the
feature/remote-state
branch
from
68d27a8 to
8ccf6d5
Compare
m-Peter
force-pushed
the
traces-api-use-remote-state
branch
from
3e97e28 to
7749f32
Compare
m-Peter
force-pushed
the
traces-api-use-remote-state
branch
from
7749f32 to
06d260e
Compare
m-Peter
force-pushed
the
traces-api-use-remote-state
branch
from
06d260e to
dd0196a
Compare
api/debug.go
Outdated
| tracerConfig := json.RawMessage(tracerConfig) | ||
| tracerCtx := &tracers.Context{ | ||
| BlockHash: receipt.BlockHash, | ||
| BlockNumber: receipt.BlockNumber, | ||
| TxIndex: int(receipt.TransactionIndex), | ||
| TxHash: receipt.TxHash, | ||
| } | ||
| tracer, err := tracers.DefaultDirectory.New(tracerName, tracerCtx, tracerConfig) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| block, err := d.blocks.GetByHeight(receipt.BlockNumber.Uint64()) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| // We need to re-execute the given transaction and all the | ||
| // transactions that precede it in the same block, based on | ||
| // the previous block state, to generate the correct trace. | ||
| previousBlock, err := d.blocks.GetByHeight(block.Height - 1) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| previousBlockHeight := previousBlock.Height | ||
|
|
||
| client, err := d.client.GetClientForHeight(previousBlockHeight) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| exeClient, ok := client.(*grpc.Client) | ||
| if !ok { | ||
| return nil, fmt.Errorf("could not convert to execution client") | ||
| } | ||
|
|
||
| ledger, err := evm.NewRemoteLedger(exeClient.ExecutionDataRPCClient(), previousBlockHeight) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("could not create remote ledger for height: %d, with: %w", previousBlockHeight, err) | ||
| } | ||
|
|
||
| blockExecutor, err := evm.NewBlockExecutor( | ||
| previousBlock, | ||
| ledger, | ||
| d.config.FlowNetworkID, | ||
| d.blocks, | ||
| d.receipts, | ||
| d.logger, | ||
| ) |
There was a problem hiding this comment.
we should extract all this logic into a tracer component. I don't like for API layer to have too much logic, it starts to get duplicated and it's harder to test in isolation. I suggest extracting the above into a tracer component in the services that just takes in a config of the tracer.
There was a problem hiding this comment.
at that point we should also write unit tests with just the tracer component
m-Peter
force-pushed
the
traces-api-use-remote-state
branch
3 times, most recently
from
26dae9f to
713ab98
Compare
m-Peter
force-pushed
the
feature/remote-state
branch
from
8ccf6d5 to
a0e6820
Compare
m-Peter
force-pushed
the
traces-api-use-remote-state
branch
3 times, most recently
from
29188dd to
738ef71
Compare
m-Peter
force-pushed
the
traces-api-use-remote-state
branch
from
738ef71 to
4aff62b
Compare
| var fixedHash *string | ||
| if payload.PrevRandao == gethCommon.HexToHash("0x0") { | ||
| hash := payload.Hash.String() | ||
| fixedHash = &hash |
There was a problem hiding this comment.
not sure why we populate it using this heuristic?
There was a problem hiding this comment.
This change is not really related to the objective of the PR, but it is required due to upgrading the flow-go versioned.
This heuristic allows us to decode block events prior to the addition of the PrevRandao field. The decoding of legacy block events was moved in flow-go (https://github.com/onflow/flow-go/blob/master/fvm/evm/events/events.go#L188-L190), so we no longer have a way to distinguish them, hence the usage of this heuristic. I'll add a comment to describe this though, as it is for sure not clear why it's needed 😅
|
Closing in favor of: #635 |
Closes: #530 #583
Description
For contributor use:
masterbranchFiles changedin the Github PR explorer