-
Notifications
You must be signed in to change notification settings - Fork 827
Firehose EVM tracer addition #1344
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
base: main
Are you sure you want to change the base?
Conversation
This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - sei-protocol#1344
0517c62
to
8efef50
Compare
x/evm/keeper/msg_server.go
Outdated
defer func() { | ||
}() |
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.
Oups leftover
This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - sei-protocol#1344
This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - sei-protocol#1344
This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - sei-protocol#1344
24bf4e0
to
61712fe
Compare
This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - sei-protocol#1344
772eb0d
to
8f6324c
Compare
* Update that brings `go-ethereum` with live tracer support This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - #1344 * Added missing instrumentation when state changes in `DBImpl`
8f6324c
to
8d70845
Compare
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.
overall looks okay from the correctness-perspective. Not sure about the performance impact, but if tracers are generally not turned on for validator nodes then it's probably fine (worst case would be that the RPC turning this on would lag behind from time to time if it does affect performance)
8d70845
to
476a2a9
Compare
* Update that brings `go-ethereum` with live tracer support This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - #1344 * Added missing instrumentation when state changes in `DBImpl`
* Update that brings `go-ethereum` with live tracer support This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - #1344 * Added missing instrumentation when state changes in `DBImpl`
* Update that brings `go-ethereum` with live tracer support This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - #1344 * Added missing instrumentation when state changes in `DBImpl`
f1ce12e
to
4763011
Compare
* Update that brings `go-ethereum` with live tracer support This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - #1344 * Added missing instrumentation when state changes in `DBImpl`
* Update that brings `go-ethereum` with live tracer support This only brings the dependency (will need to wait for a merge + tag of the branch before merging this PR) and the required changes to adapt `DBImpl` to support the new interface. Relates to: - sei-protocol/go-ethereum#15 - #1344 * Added missing instrumentation when state changes in `DBImpl`
This reverts commit 7bf5478.
…ei-protocol#1755)" This reverts commit 5ddb9ab.
* checkpoint * fix * fix
This reverts commit f17ec67.
…sei-protocol#1755)" This reverts commit 067ab50.
This reverts commit d9d6b08.
This reverts commit 7bf5478.
This reverts commit fe7f265.
This reverts commit 9289f1d.
This reverts commit d7575be.
This reverts commit 7dee649.
27bc1be
to
42eff8a
Compare
Initial commit to see the general idea of how Firehose tracer is instantiated and configured
Starter
Important This is a WIP PR that I push now so that you guys can see how it looks like and we then we can discuss concrete solutions to the questions I still have unanswered so far.
Here what I see as important decision points I took and open questions I have that will shape the future PR(s).
Ethereum
core.BlockchainLogger
vsx/evm/tracers#BlockchainLogger
The standard Geth has
core.BlockchainLogger
which acts as a "live" tracer that traces the full block execution in addition to the EVM and State tracing.However, the
core.BlockchainLogger
is made for Ethereum Block model in a way that is a bit too rigid to fit in Sei model, for example theOnBlockStart
receives a*types.Block
which is too hard to replicate in Sei model (if it was receivingtypes.Header
, we could maybe have retrofit).Furthermore, the
BlockchainLogger
direct methods (OnBlockStart
,OnBlockEnd
, etc.) are used incore/blockchain.go
, execution point that is not present in Sei. So it was a bit moot to try to usecore.BlockchainLogger
in Sei directly due to required changes to the interface that would be needed.We could have follow that route, e.g. changing the
core.BlockchainLogger
interface to fit Sei, but I judged it was better to limit the changes ingo-ethereum
and instead define our own version ofBlockchainLogger
.For now I kept the same name and its semantics fits only for Ethereum transactions, so right now it's not a general Sei tracer.
I also prefix all methods of this interface with
OnSei...
, the reason for that being that it would enable one to implement both interface from a single struct, something I see could be a possibility for ourFirehoseTracer
.Open questions:
x/evm/tracers#BlockchainLogger
is EVM aware only, should it become a general tracer for Sei with possibility to trace all type of transactions, including EVM? I think this discussion can be post-pone. First, a survey of Cosmos tracing world would be needed as we would probably want to follow current standard. Second, we could want to have this as a general Cosmos/Tendermint piece instead.Tracers Directory and Activation
Right now I enforced the instantiation of the
FirehoseTracer
straight inapp.go
. The Geth tracer PR introduces a registry were standard tracers are added to. The node operator can then activate the tracing by passing the flaggeth ... --vmtrace=<tracer-id>
, e.g.geth ... --vmtrace=firehose
.I plan to offer a similar experience in Sei, so I will add some kind of registry for the tracers +
func init()
registration. We could think of adding aDebuggingTracer
that would print every tracing entry points.If you could give me some hints on the names you would like to use, I'll prepare something.
FirehoseTracer & Inclusion
The
FirehoseTracer
struct is our implementation ofx/evm/tracers@BlockchainLogger
. How it works is relatively simple. When a block start, we instantiate our Ethereum Block Model struct which is a Protobuf generated Go struct, definitions can be seen at https://buf.build/streamingfast/firehose-ethereum/docs/main:sf.ethereum.type.v2#sf.ethereum.type.v2.Block.As the tracer is invoked (
CaptureTxStart
,CaptureStart
, etc..) we decoded and accumulated all information in our block model.CaptureTxStart
lead to a new activeTransactionTracer
which containsCall[]
each call recording the various state changes (Log, Balance, Nonce, etc.)On
OnBlockEnd
, the block is "completed", we serialize it to bytes then to base64 and we then emit in text format onstdout
file descriptor the lineFIRE BLOCK <blocks's metadata> <final_block_ref> <bytes_base64_payload>
. Ourfireeth
binary manages theseid start
process reading itsstdout
pipe and blocks progress through our code at this point.I would like to have
FirehoseTracer
directly builtin insei-chain
. The main reason is to have an easy way for operators to operator Firehose node without having to download a forked binary that contains the Firehose interface. Morevover, the Firehose output format is relatively simply to parse and can be used in any language that supports Protobuf so external system could benefits from it.Parallelism, linearity
Right now the tracer in
app.go#ProcessBlock
is re-instantiated on each block as I noticedProcessBlock
is called within a goroutine which leads me to think that there is a possibility that 2 or moreProcessBlock
could run concurrently.Firehose needs to emit the block by locking the pipe to ensure that a single line is fully emitted before the next one, we wouldn't want concurrent write to
stdout
pipe.I'm unsure where to ensure that linearity and exclusiveness is respected. For example if there is indeed 2 concurrents
ProcessBlock
running, where can I trigger the "emit" sequentially?Finality
Tony mentioned that
ProcessBlock
could be called from two code paths one final the other not final. Firehose handles forks without a problem but needs finality information about the block, we need to know which parent(s) block is final relative to the current block.The idea is in pseudo-code something like this:
Chain(s) usually have some kind of deterministic way to determine the final block. In case of Sei with its instant finality and in regards to
ProcessBlock
, we have multiple avenue:ProcessBlock
is final, would need to see where I can get that information.findFirstFinalBlockStartingFrom(block)
for Sei chain and I use thatRunPrecompiledContract
andRunAndCalculateGas
In
RunPrecompiledContract
there isRunAndCalculateGas
which is an early return. InRunPrecompiledContract
method we track gas viaOnGasChange
which means we need to also instrumentRunAndCalculateGas
which is an interface.What is the correct way to instrument this, this is billing some gas if I understand it right. However
RunAndCalculateGas
is an interface so there is multiple implementation, they all must be aware of the tracer.Sei Address Association
Sei has address association from sei <=> EVM based on the fact the the underlying private key is secp256k1 but the public keys is mapped differently. Is this tracked in some EVM contract/pre-compiled or is it kept in Sei kvdb? Should we track this somehow?
State Snapshot
Our standard Ethereum implementation records the genesis block with the genesis balances, code and storage for the various genesis account. Now it appears that when Sei upgrade to EVM support, the current
usei
balance is carried over to EVM.I see the Firehose tracer as starting from the very block that will enable EVM meaning state prior that point will not be known to Firehose user. This is a problem for advanced technology that are built on the fact that the tracer keeps sees the full state of the node including genesis data.
One possibility I see here is that
x/evm/types/BlockchainLogger
get an extraOnEVMGenesisState
. Now, the exact interface of how to query the state would need to be defined.I sense of the amount of data we talk about will drive the decision of such API. Indeed, if you tell me there is currently 10M Sei users, maybe holding it full in memory is not the best choice.
Would need to know also if there is other state that caries over. I saw some ERC20 likes stuff and I think some mapping could be in the plan so I imagine here also the initial state would be kept in Sei.
Other Changes
Outside of the
RunAndCalculateGas
, in this vein of tracking any state changes happening around the EVM block execution, do you guys see other things that are particular to Sei but fiddles someone with the EVM known state?