Firehose EVM tracer addition #1344

maoueh · 2024-02-10T14:16:21Z

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` vs `x/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 the OnBlockStart receives a *types.Block which is too hard to replicate in Sei model (if it was receiving types.Header, we could maybe have retrofit).

Furthermore, the BlockchainLogger direct methods (OnBlockStart, OnBlockEnd, etc.) are used in core/blockchain.go, execution point that is not present in Sei. So it was a bit moot to try to use core.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 in go-ethereum and instead define our own version of BlockchainLogger.

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 our FirehoseTracer.

Open questions:

Right now the 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 in app.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 flag geth ... --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 a DebuggingTracer 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 of x/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 active TransactionTracer which contains Call[] 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 on stdout file descriptor the line FIRE BLOCK <blocks's metadata> <final_block_ref> <bytes_base64_payload>. Our fireeth binary manages the seid start process reading its stdout pipe and blocks progress through our code at this point.

I would like to have FirehoseTracer directly builtin in sei-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 noticed ProcessBlock is called within a goroutine which leads me to think that there is a possibility that 2 or more ProcessBlock 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:

ProcessBlock(block Block):
  OnBlockStart(block, findFirstFinalBlockStartingFrom(block))
  ...

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:

Maybe the final/non-final execution is true only for a miner node and is not relevant for a full node that is simply syncing with the network. If it's the case in Sei, then I would simply use the current block as being final (and add a sanity check if the block is non-final and a tracer is active).
The tracer is only called if the execution of ProcessBlock is final, would need to see where I can get that information.
We determine what is the right version of findFirstFinalBlockStartingFrom(block) for Sei chain and I use that

`RunPrecompiledContract` and `RunAndCalculateGas`

In RunPrecompiledContract there is RunAndCalculateGas which is an early return. In RunPrecompiledContract method we track gas via OnGasChange which means we need to also instrument RunAndCalculateGas 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 extra OnEVMGenesisState. 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?

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

maoueh · 2024-02-28T00:39:12Z

x/evm/keeper/msg_server.go

+ defer func() {
+ }()


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

* 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`

precompiles/bank/bank.go

codchen

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)

app/app.go

* 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`

x/evm/tracers/firehose.go

+ "net/url"
+ "os"
+ "regexp"
+ "runtime"


* 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`

# Conflicts: # x/evm/keeper/msg_server.go # x/evm/module.go # x/evm/module_test.go

# Conflicts: # evmrpc/config.go # evmrpc/config_test.go

# Conflicts: # evmrpc/config.go # evmrpc/config_test.go # precompiles/bank/bank.go # precompiles/wasmd/wasmd.go # x/evm/keeper/msg_server.go # x/evm/module.go

…irehose panics

… running Firehose tracer

…OrDefault` otherwise we generate app mistmatches

# Conflicts: # app/app.go # x/evm/keeper/msg_server.go # x/evm/module_test.go

# Conflicts: # precompiles/wasmd/wasmd.go

codchen · 2024-07-03T06:05:40Z

app/app.go

@@ -967,6 +975,15 @@ func New(
 app.HardForkManager = upgrades.NewHardForkManager(app.ChainID)
 app.HardForkManager.RegisterHandler(v0upgrade.NewHardForkUpgradeHandler(100_000, upgrades.ChainIDSeiHardForkTest, app.WasmKeeper))

+ if app.evmRPCConfig.LiveEVMTracer != "" {


let's put this config under either https://github.com/sei-protocol/sei-chain/blob/main/x/evm/querier/config.go or a new config section, since it's used in the actual processing and not an RPC-side thing

What about:

... [evm_tracing] live_tracer = <name> ...

Does that sounds good?

codchen · 2024-07-03T06:06:47Z

precompiles/bank/bank.go

@@ -235,11 +236,17 @@ func (p Precompile) sendNative(ctx sdk.Context, method *abi.Method, args []inter
 return nil, err
 }

- usei, wei, err := pcommon.HandlePaymentUseiWei(ctx, p.evmKeeper.GetSeiAddressOrDefault(ctx, p.address), senderSeiAddr, value, p.bankKeeper)
+ precompiledSeiAddr := p.evmKeeper.GetSeiAddressOrDefault(ctx, p.address)


nit: precompileSeiAddr

codchen · 2024-07-03T06:09:06Z

precompiles/wasmd/wasmd.go

@@ -147,6 +147,10 @@ func (p Precompile) RunAndCalculateGas(evm *vm.EVM, caller common.Address, calli
 }
 ctx = ctx.WithGasMeter(sdk.NewGasMeterWithMultiplier(ctx, gasLimitBigInt.Uint64()))

+ if logger != nil && logger.OnGasChange != nil {


you might want to rebase; we recently refactored RunAndCalculateGas logics and these new lines should probably go under https://github.com/sei-protocol/sei-chain/blob/main/precompiles/common/precompiles.go#L143

Yeah I'll bring in latest changes and perform the necessary requested changes.

codchen · 2024-07-03T06:11:07Z

x/evm/keeper/msg_server.go

@@ -347,6 +417,36 @@ func (server msgServer) AssociateContractAddress(goCtx context.Context, msg *typ
 return &types.MsgAssociateContractAddressResponse{}, nil
 }

+func getEthReceipt(ctx sdk.Context, tx *ethtypes.Transaction, msg *core.Message, res *core.ExecutionResult, stateDB *state.DBImpl) *ethtypes.Receipt {


this duplicates logic in writeReceipt. If this happens before writeReceipt, can you make it so that the result is passed over to writeReceipt so that we don't allocate memory/calculate bloom/etc. twice?

Yeah indeed, I wasn't sure what you guys would have preferred.

I'll check into refactoring this to share the computing.

codchen · 2024-07-03T06:14:15Z

x/evm/module.go

+
+ if evmHooks != nil && evmHooks.OnBalanceChange != nil && (surplusUsei.GT(sdk.ZeroInt()) || surplusWei.GT(sdk.ZeroInt())) {
+ evmModuleAddress := am.keeper.AccountKeeper().GetModuleAddress(types.ModuleName)
+ tracers.TraceBlockReward(ctx, evmHooks, am.keeper.BankKeeper(), evmModuleAddress, am.keeper.GetEVMAddressOrDefault(ctx, evmModuleAddress), surplusUsei, surplusWei)


the nature of surplus here is closer to burnt actually, since a surplus is resulted from a debit and a smaller credit. The difference between the two iiuc, in ethereum's world, would be considered as burnt

# Conflicts: # app/app.go # precompiles/bank/bank.go # precompiles/wasmd/wasmd.go # x/evm/module.go # x/evm/module_test.go

…ture/firehose-tracer # Conflicts: # precompiles/wasmd/wasmd.go

# Conflicts: # app/app.go # go.sum # x/evm/module.go

…rallel execution engine Some transactions are executed in parallel but not for an EVM transaction. In those cases, it can happen that system calls are performed by the WASM land. We must porperly handle those cases too. To handle those cases, we had to modify how the re-ordering of the ordinal is performed. With new interweave system calls relative to normal transactions, we needed to make the re-ordering "inline" as transaction and/or system calls are added to the block. By doing it like this, the global tracer is always aware of the currently populated ordinal. So changes happening after all other transaction will have the correct ordinal.

…ture/firehose-tracer

maoueh marked this pull request as draft February 10, 2024 14:16

maoueh mentioned this pull request Feb 23, 2024

Update that brings go-ethereum with live tracer support #1375

Merged

maoueh force-pushed the feature/firehose-tracer branch from 0517c62 to 8efef50 Compare February 27, 2024 19:07

maoueh commented Feb 28, 2024

View reviewed changes

x/evm/keeper/msg_server.go Outdated

Comment on lines 69 to 70

defer func() {

}()

Copy link

Contributor Author

maoueh Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oups leftover

maoueh force-pushed the feature/firehose-tracer branch from 24bf4e0 to 61712fe Compare March 22, 2024 15:35

maoueh force-pushed the feature/firehose-tracer branch 2 times, most recently from 772eb0d to 8f6324c Compare March 28, 2024 14:15

maoueh force-pushed the feature/firehose-tracer branch from 8f6324c to 8d70845 Compare April 10, 2024 16:19

maoueh changed the base branch from evm to seiv2 April 10, 2024 16:20

codchen reviewed Apr 15, 2024

View reviewed changes

precompiles/bank/bank.go Outdated Show resolved Hide resolved

codchen reviewed Apr 15, 2024

View reviewed changes

app/app.go Outdated Show resolved Hide resolved

maoueh force-pushed the feature/firehose-tracer branch from 8d70845 to 476a2a9 Compare April 16, 2024 18:06

maoueh marked this pull request as ready for review April 16, 2024 18:23

github-advanced-security bot found potential problems Apr 17, 2024

View reviewed changes

x/evm/tracers/firehose.go

"net/url"

"os"

"regexp"

"runtime"

Check notice

Code scanning / CodeQL

Sensitive package import

Certain system packages contain functions which may be a possible source of non-determinism

maoueh force-pushed the feature/firehose-tracer branch 2 times, most recently from f1ce12e to 4763011 Compare April 17, 2024 20:46

udpatil force-pushed the seiv2 branch from 9403876 to df899bb Compare April 18, 2024 14:05

udpatil force-pushed the seiv2 branch from aa0aca9 to 4eea060 Compare April 19, 2024 19:44

maoueh force-pushed the feature/firehose-tracer branch from a19ceb5 to 6e2102c Compare April 23, 2024 19:29

Firehose tracer addition

e73963a

maoueh force-pushed the feature/firehose-tracer branch from 6e2102c to 4f66962 Compare May 2, 2024 17:54

Merge tag 'v5.1.2' into release/firehose

aa512a8

# Conflicts: # x/evm/keeper/msg_server.go # x/evm/module.go # x/evm/module_test.go

maoueh force-pushed the feature/firehose-tracer branch from 4f66962 to 6761bc6 Compare May 2, 2024 18:13

maoueh added 2 commits May 3, 2024 16:40

Merge tag 'v5.2.0' into release/firehose

e6bdc65

# Conflicts: # evmrpc/config.go # evmrpc/config_test.go

Firehose tracer addition

e05ef46

maoueh force-pushed the feature/firehose-tracer branch from 6761bc6 to e05ef46 Compare May 3, 2024 20:46

maoueh added 9 commits May 27, 2024 09:40

Merge tag 'v5.5.2' into feature/firehose-tracer

ad9f914

# Conflicts: # evmrpc/config.go # evmrpc/config_test.go # precompiles/bank/bank.go # precompiles/wasmd/wasmd.go # x/evm/keeper/msg_server.go # x/evm/module.go

Merge branch 'feature/firehose-tracer' into release/firehose

16f73c4

# Conflicts: # evmrpc/config.go # evmrpc/config_test.go # precompiles/bank/bank.go # precompiles/wasmd/wasmd.go # x/evm/keeper/msg_server.go # x/evm/module.go

Improved panic handling ensuring we always print a stack trace when F…

cccdf48

…irehose panics

Improved caller reporting when there is an invalid state

ab01128

Fixed bug with system call not correctly wired leading to panics when…

67bd0af

… running Firehose tracer

The tracing in precompiles/bank/bank.go needs to use `GetEVMAddress…

8418897

…OrDefault` otherwise we generate app mistmatches

Merge tag 'v5.5.5' into feature/firehose-tracer

ec72741

# Conflicts: # app/app.go # x/evm/keeper/msg_server.go # x/evm/module_test.go

Merge remote-tracking branch 'origin/main' into feature/firehose-tracer

794b793

# Conflicts: # precompiles/wasmd/wasmd.go

Merge branch 'main' into feature/firehose-tracer

8fafa3d

codchen reviewed Jul 3, 2024

View reviewed changes

yzang2019 and others added 12 commits July 3, 2024 14:35

Fix seidb default config value for KeepLastVersion

98994b7

Fix unit test

9612ef1

Update go mod

9dd002e

Add upgrade handler for v5.6.2

6bad612

Merge tag 'v5.6.2' into feature/firehose-tracer-at-latest-release-tag

2cc7a28

# Conflicts: # app/app.go # precompiles/bank/bank.go # precompiles/wasmd/wasmd.go # x/evm/module.go # x/evm/module_test.go

Merge branch 'feature/firehose-tracer-at-latest-release-tag' into fea…

8c44484

…ture/firehose-tracer # Conflicts: # precompiles/wasmd/wasmd.go

Merge remote-tracking branch 'origin/main' into feature/firehose-tracer

23c925f

# Conflicts: # app/app.go # go.sum # x/evm/module.go

Fixed a bug on parallel execution

e7096af

Fixed bug when re-ordering multiple system calls

dbd9193

Go mod tidy

8a8244d

Merge branch 'feature/firehose-tracer-at-latest-release-tag' into fea…

53cecd5

…ture/firehose-tracer

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Firehose EVM tracer addition #1344

Firehose EVM tracer addition #1344

maoueh commented Feb 10, 2024

maoueh Feb 28, 2024

codchen left a comment

codchen Jul 3, 2024

maoueh Jul 8, 2024

maoueh Jul 9, 2024

codchen Jul 3, 2024

codchen Jul 3, 2024

maoueh Jul 8, 2024

codchen Jul 3, 2024

maoueh Jul 8, 2024

codchen Jul 3, 2024

Firehose EVM tracer addition #1344

Are you sure you want to change the base?

Firehose EVM tracer addition #1344

Conversation

maoueh commented Feb 10, 2024

Starter

Ethereum core.BlockchainLogger vs x/evm/tracers#BlockchainLogger

Tracers Directory and Activation

FirehoseTracer & Inclusion

Parallelism, linearity

Finality

RunPrecompiledContract and RunAndCalculateGas

Sei Address Association

State Snapshot

Other Changes

Choose a reason for hiding this comment

codchen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ethereum `core.BlockchainLogger` vs `x/evm/tracers#BlockchainLogger`

`RunPrecompiledContract` and `RunAndCalculateGas`