-
Notifications
You must be signed in to change notification settings - Fork 495
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.
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
Live tracing experimental MVP support for arbitrum nitro #2934
base: master
Are you sure you want to change the base?
Conversation
|
} | ||
|
||
defer func() { | ||
getVMConfig().Tracer.OnBlockEnd(outError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hook was missing a call, so I've added it manually here
@@ -192,6 +218,16 @@ func ProduceBlockAdvanced( | |||
} | |||
|
|||
header := createNewHeader(lastBlockHeader, l1Info, arbState, chainConfig) | |||
|
|||
getVMConfig().Tracer.OnBlockStart( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hook was missing a call, so I've added it manually here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution!
This is a very initial review, but should help get you on the right track. I Didn't yet see geth-part.
[submodule "go-ethereum"] | ||
path = go-ethereum | ||
url = https://github.com/Tenderly/arb-go-ethereum.git | ||
branch = live-tracing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this change.
You should have a separate PR to our go-ethereum fork, point to it from your comment ("this pulls in + link"), and add a commit which sets go-ethereum to the specific commit of your geth-change.
We have merge checks that will not allow nitro commit to be merged before the geth PR is also merged.
cacheConfig *core.CacheConfig, | ||
chainConfig *params.ChainConfig, | ||
txLookupLimit uint64, | ||
tracingConfig json.RawMessage, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should not need to chainge blockchain at all. However, if you do - seems better to get here *tracing.Hooks that a json.RawMessage.
} | ||
|
||
// TODO: IMPORTANT !!! | ||
// This will cause a tracer to be called multiple times for the same block concurrently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably not the only thing that'll cause tracer to be called multiple times.
In normal case we have no reorgs, but a node might reexecute the same block multiple times for ultiple reasons.
I didn't view geth side yet, but I think a better solution will be to:
- remove all changes to blockchain
- Pass a vm.Config to ProduceBlock which could enable tracing
- have a config that controls tracing and feed it to ExecutionEngine (it currently does not have a config.. have a look how we use configs for e.g. SyncMonitorConfig)
- Inside func (s *ExecutionEngine) createBlockFromNextMessage - pass to ProduceBlock a vm.Config that enables tracing if it was enabled in config and only if isMsgForPrefetch == false
@@ -172,7 +184,21 @@ func ProduceBlockAdvanced( | |||
sequencingHooks *SequencingHooks, | |||
isMsgForPrefetch bool, | |||
runMode core.MessageRunMode, | |||
) (*types.Block, types.Receipts, error) { | |||
blockChain *core.BlockChain, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems better to get a VMConfig than a BlockChain.
Integrating Tenderly Live Tracer into Arbitrum Nitro
Description:
This PR demonstrates our attempt to integrate live tracing into Arbitrum Nitro.
While we have a more complex system in place with the actual live tracer and a full build system around it, this PR highlights the core changes necessary to enable live tracing for Arb Nitro.
It serves as a foundation for understanding the key modifications required for this integration to happen.