Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

fix(rpc): cache back access list within same block #1585

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (rpc) [#1503](https://github.com/evmos/ethermint/pull/1503) Fix block hashes returned on JSON-RPC filter `eth_newBlockFilter`.
* (rpc) [#1557](https://github.com/evmos/ethermint/pull/1557) Patch GasUsed before the fix of revert gas refund logic when transaction reverted for `eth_getTransactionReceipt`.
* (ante) [#1566](https://github.com/evmos/ethermint/pull/1566) Fix `gasWanted` on `EthGasConsumeDecorator` ante handler when running transaction in `ReCheckMode`
* (rpc) [#1585](https://github.com/evmos/ethermint/pull/1585) Patch gas before the fix of clear access list before processing each transaction for `debug_traceTransaction`.

## [v0.19.3] - 2022-10-14

Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ require (
github.com/tyler-smith/go-bip39 v1.1.0
golang.org/x/net v0.5.0
golang.org/x/text v0.6.0
golang.org/x/text v0.6.0
google.golang.org/genproto v0.0.0-20221118155620-16455021b5e6
google.golang.org/grpc v1.52.0
sigs.k8s.io/yaml v1.3.0
Expand Down
20 changes: 10 additions & 10 deletions gomod2nix.toml
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,8 @@ schema = 3
version = "v0.0.5"
hash = "sha256-/5i70IkH/qSW5KjGzv8aQNKh9tHoz98tqtL0K2DMFn4="
[mod."github.com/onsi/ginkgo/v2"]
version = "v2.6.1"
hash = "sha256-OEiWYKCGPCaqL3vzSrHquHGm+Q8URT2anpanAVK5hRo="
version = "v2.7.0"
hash = "sha256-BKqQKCsPA73FaQwYpAY+QsWFHIncrG5jgRhC2IiNmCk="
[mod."github.com/onsi/gomega"]
version = "v1.24.2"
hash = "sha256-iascSzzBT1Uv/XybezSblIwwrq78BU4a9BVB5MvK6MM="
Expand Down Expand Up @@ -496,23 +496,23 @@ schema = 3
version = "v0.0.0-20220722155223-a9213eeb770e"
hash = "sha256-kNgzydWRpjm0sZl4uXEs3LX5L0xjJtJRAFf/CTlYUN4="
[mod."golang.org/x/net"]
version = "v0.4.0"
hash = "sha256-7IwGZh/xg4mQz88cJio2Ov5d3jGRXKj1itlAja/EAbQ="
version = "v0.5.0"
hash = "sha256-HpbIAiLs7S1+tVsaSSdbCPw1IK43A0bFFuSzPSyjLbo="
[mod."golang.org/x/oauth2"]
version = "v0.0.0-20221014153046-6fdb5e3db783"
hash = "sha256-IoygidVNqyAZmN+3macDeIefK8hhJToygpcqlwehdYQ="
[mod."golang.org/x/sync"]
version = "v0.1.0"
hash = "sha256-Hygjq9euZ0qz6TvHYQwOZEjNiTbTh1nSLRAWZ6KFGR8="
[mod."golang.org/x/sys"]
version = "v0.3.0"
hash = "sha256-TIHhfYbZ99sCU1ZMikxwomXH5AEtD/lA1VMMW+UAhbU="
version = "v0.4.0"
hash = "sha256-jchMzHCH5dg+IL/F+LqaX/fyAcB/nvHQpfBjqwaRJH0="
[mod."golang.org/x/term"]
version = "v0.3.0"
hash = "sha256-NKv2o8wz8DB/2W2h/muGEIHb+S06mBXZxhG254RpQ5s="
version = "v0.4.0"
hash = "sha256-wQKxHV10TU4vCU8Re2/hFmAbur/jRWEOB8QXBzgTFNY="
[mod."golang.org/x/text"]
version = "v0.5.0"
hash = "sha256-ztH+xQyM/clOcQl+y/UEPcfNKbc3xApMbEPDDZ9up0o="
version = "v0.6.0"
hash = "sha256-+bpeRWR3relKACdal6NPj+eP5dnWCplTViArSN7/qA4="
[mod."golang.org/x/xerrors"]
version = "v0.0.0-20220907171357-04be3eba64a2"
hash = "sha256-6+zueutgefIYmgXinOflz8qGDDDj0Zhv+2OkGhBTKno="
Expand Down
6 changes: 6 additions & 0 deletions proto/ethermint/evm/v1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,9 @@ message QueryTraceTxRequest {
bytes proposer_address = 8 [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.ConsAddress"];
// chain_id is the the eip155 chain id parsed from the requested block header
int64 chain_id = 9;
// fix_clear_access_list_height defines the upgrade height for fix clear access list before processing each
// transaction
int64 fix_clear_access_list_height = 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this constant have to be included in the .proto file for this request? Why can't it be included in the code elsewhere?

}

// QueryTraceTxResponse defines TraceTx response
Expand All @@ -279,6 +282,9 @@ message QueryTraceBlockRequest {
bytes proposer_address = 8 [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.ConsAddress"];
// chain_id is the eip155 chain id parsed from the requested block header
int64 chain_id = 9;
// fix_clear_access_list_height defines the upgrade height for fix clear access list before processing each
// transaction
int64 fix_clear_access_list_height = 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question as above.

}

// QueryTraceBlockResponse defines TraceBlock response
Expand Down
30 changes: 16 additions & 14 deletions rpc/backend/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,14 @@ func (b *Backend) TraceTransaction(hash common.Hash, config *evmtypes.TraceConfi
}

traceTxRequest := evmtypes.QueryTraceTxRequest{
Msg: ethMessage,
Predecessors: predecessors,
BlockNumber: blk.Block.Height,
BlockTime: blk.Block.Time,
BlockHash: common.Bytes2Hex(blk.BlockID.Hash),
ProposerAddress: sdk.ConsAddress(blk.Block.ProposerAddress),
ChainId: b.chainID.Int64(),
Msg: ethMessage,
Predecessors: predecessors,
BlockNumber: blk.Block.Height,
BlockTime: blk.Block.Time,
BlockHash: common.Bytes2Hex(blk.BlockID.Hash),
ProposerAddress: sdk.ConsAddress(blk.Block.ProposerAddress),
ChainId: b.chainID.Int64(),
FixClearAccessListHeight: b.cfg.JSONRPC.FixClearAccessListHeight,
}

if config != nil {
Expand Down Expand Up @@ -172,13 +173,14 @@ func (b *Backend) TraceBlock(height rpctypes.BlockNumber,
ctxWithHeight := rpctypes.ContextWithHeight(int64(contextHeight))

traceBlockRequest := &evmtypes.QueryTraceBlockRequest{
Txs: txsMessages,
TraceConfig: config,
BlockNumber: block.Block.Height,
BlockTime: block.Block.Time,
BlockHash: common.Bytes2Hex(block.BlockID.Hash),
ProposerAddress: sdk.ConsAddress(block.Block.ProposerAddress),
ChainId: b.chainID.Int64(),
Txs: txsMessages,
TraceConfig: config,
BlockNumber: block.Block.Height,
BlockTime: block.Block.Time,
BlockHash: common.Bytes2Hex(block.BlockID.Hash),
ProposerAddress: sdk.ConsAddress(block.Block.ProposerAddress),
ChainId: b.chainID.Int64(),
FixClearAccessListHeight: b.cfg.JSONRPC.FixClearAccessListHeight,
}

res, err := b.queryClient.TraceBlock(ctxWithHeight, traceBlockRequest)
Expand Down
3 changes: 3 additions & 0 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ type JSONRPCConfig struct {
MetricsAddress string `mapstructure:"metrics-address"`
// FixRevertGasRefundHeight defines the upgrade height for fix of revert gas refund logic when transaction reverted
FixRevertGasRefundHeight int64 `mapstructure:"fix-revert-gas-refund-height"`
// FixClearAccessListHeight defines the upgrade height for fix clear access list before processing each transaction
FixClearAccessListHeight int64 `mapstructure:"fix-clear-access-list-height"`
}

// TLSConfig defines the certificate and matching private key for the server.
Expand Down Expand Up @@ -351,6 +353,7 @@ func GetConfig(v *viper.Viper) (Config, error) {
EnableIndexer: v.GetBool("json-rpc.enable-indexer"),
MetricsAddress: v.GetString("json-rpc.metrics-address"),
FixRevertGasRefundHeight: v.GetInt64("json-rpc.fix-revert-gas-refund-height"),
FixClearAccessListHeight: v.GetInt64("json-rpc.fix-clear-access-list-height"),
},
TLS: TLSConfig{
CertificatePath: v.GetString("tls.certificate-path"),
Expand Down
3 changes: 3 additions & 0 deletions server/config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ metrics-address = "{{ .JSONRPC.MetricsAddress }}"
# Upgrade height for fix of revert gas refund logic when transaction reverted.
fix-revert-gas-refund-height = {{ .JSONRPC.FixRevertGasRefundHeight }}

# Upgrade height for fix clear access list before processing each transaction.
fix-clear-access-list-height = {{ .JSONRPC.FixClearAccessListHeight }}

###############################################################################
### TLS Configuration ###
###############################################################################
Expand Down
1 change: 1 addition & 0 deletions server/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const (
// https://github.com/ethereum/go-ethereum/blob/master/metrics/metrics.go#L35-L55
JSONRPCEnableMetrics = "metrics"
JSONRPCFixRevertGasRefundHeight = "json-rpc.fix-revert-gas-refund-height"
JSONRPCFixClearAccessListHeight = "json-rpc.fix-clear-access-list-height"
)

// EVM flags
Expand Down
1 change: 1 addition & 0 deletions tests/integration_tests/configs/default.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
'block-range-cap': 10000,
'logs-cap': 10000,
'fix-revert-gas-refund-height': 1,
'fix-clear-access-list-height': 1,
},
},
validators: [{
Expand Down
113 changes: 84 additions & 29 deletions x/evm/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,38 @@ func (k Keeper) EstimateGas(c context.Context, req *types.EthCallRequest) (*type
return &types.EstimateGasResponse{Gas: hi}, nil
}

// GetTxTraceResultForTx returns statedb with cached address list when need patch
func (k Keeper) GetTxTraceResultForTx(
ctx sdk.Context,
tx *types.MsgEthereumTx,
signer ethtypes.Signer,
cfg *statedb.EVMConfig,
txConfig statedb.TxConfig,
patch bool,
lastDB *statedb.StateDB,
) (*statedb.StateDB, error) {
ethTx := tx.AsTransaction()
msg, err := ethTx.AsMessage(signer, cfg.BaseFee)
if err != nil {
return lastDB, err
}
txConfig.TxHash = ethTx.Hash()
var stateDB *statedb.StateDB
if patch {
stateDB = statedb.New(ctx, &k, txConfig)
if lastDB != nil {
stateDB.SetAddressToAccessList(lastDB.GetAddressToAccessList())
}
lastDB = stateDB
}
rsp, err := k.ApplyMessageWithStateDB(ctx, msg, types.NewNoOpTracer(), true, cfg, txConfig, stateDB)
if err != nil {
return lastDB, err
}
txConfig.LogIndex += uint(len(rsp.Logs))
return lastDB, nil
}

// TraceTx configures a new tracer according to the provided configuration, and
// executes the given message in the provided environment. The return value will
// be tracer dependent.
Expand Down Expand Up @@ -421,22 +453,14 @@ func (k Keeper) TraceTx(c context.Context, req *types.QueryTraceTxRequest) (*typ
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to load evm config: %s", err.Error())
}
signer := ethtypes.MakeSigner(cfg.ChainConfig, big.NewInt(ctx.BlockHeight()))

height := ctx.BlockHeight()
patch := height < req.FixClearAccessListHeight
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, I see that you are accessing the FixClearAccessListHeight from the request itself. Similar to the question above: why does the constant have to come from the request? Why can't it be stored elsewhere, ex. in the code for the evm keeper itself, to avoid changing the proto files and json-rpc configuration overall?

Copy link
Contributor Author

@mmsqe mmsqe Jan 6, 2023

Choose a reason for hiding this comment

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

Cut off height config might be different based on chain, and not sure if env variable is better way than JSONRPCConfig for keeper to access?

signer := ethtypes.MakeSigner(cfg.ChainConfig, big.NewInt(height))
txConfig := statedb.NewEmptyTxConfig(common.BytesToHash(ctx.HeaderHash().Bytes()))
var lastDB *statedb.StateDB
for i, tx := range req.Predecessors {
ethTx := tx.AsTransaction()
msg, err := ethTx.AsMessage(signer, cfg.BaseFee)
if err != nil {
continue
}
txConfig.TxHash = ethTx.Hash()
txConfig.TxIndex = uint(i)
rsp, err := k.ApplyMessageWithConfig(ctx, msg, types.NewNoOpTracer(), true, cfg, txConfig)
if err != nil {
continue
}
txConfig.LogIndex += uint(len(rsp.Logs))
lastDB, _ = k.GetTxTraceResultForTx(ctx, tx, signer, cfg, txConfig, patch, lastDB)
4rgon4ut marked this conversation as resolved.
Show resolved Hide resolved

Check warning

Code scanning / gosec

Returned error is not propagated up the stack.

Returned error is not propagated up the stack.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
lastDB, _ = k.GetTxTraceResultForTx(ctx, tx, signer, cfg, txConfig, patch, lastDB)
//#nosec G703 -- if an error is returned, just continue with the next iteration
lastDB, _ = k.GetTxTraceResultForTx(ctx, tx, signer, cfg, txConfig, patch, lastDB)

}

tx := req.Msg.AsTransaction()
Expand All @@ -450,8 +474,11 @@ func (k Keeper) TraceTx(c context.Context, req *types.QueryTraceTxRequest) (*typ
// ignore error. default to no traceConfig
_ = json.Unmarshal([]byte(req.TraceConfig.TracerJsonConfig), &tracerConfig)
}

result, _, err := k.traceTx(ctx, cfg, txConfig, signer, tx, req.TraceConfig, false, tracerConfig)
stateDB := statedb.New(ctx, &k, txConfig)
if lastDB != nil {
stateDB.SetAddressToAccessList(lastDB.GetAddressToAccessList())
}
result, _, err := k.traceTx(ctx, cfg, txConfig, stateDB, signer, tx, req.TraceConfig, false, tracerConfig)
if err != nil {
// error will be returned with detail status from traceTx
return nil, err
Expand All @@ -467,6 +494,39 @@ func (k Keeper) TraceTx(c context.Context, req *types.QueryTraceTxRequest) (*typ
}, nil
}

// GetTxTraceResultForBlock returns TxTraceResult and
// statedb with cached address list when need patch and
Comment on lines +497 to +498
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you complete the sentence you have started here?

func (k Keeper) GetTxTraceResultForBlock(
ctx sdk.Context,
tx *types.MsgEthereumTx,
signer ethtypes.Signer,
cfg *statedb.EVMConfig,
txConfig statedb.TxConfig,
traceConfig *types.TraceConfig,
patch bool,
lastDB *statedb.StateDB,
) (*statedb.StateDB, *types.TxTraceResult) {
result := new(types.TxTraceResult)
ethTx := tx.AsTransaction()
txConfig.TxHash = ethTx.Hash()
var stateDB *statedb.StateDB
if patch {
stateDB = statedb.New(ctx, &k, txConfig)
if lastDB != nil {
stateDB.SetAddressToAccessList(lastDB.GetAddressToAccessList())
}
lastDB = stateDB
}
traceResult, logIndex, err := k.traceTx(ctx, cfg, txConfig, stateDB, signer, ethTx, traceConfig, true, nil)
if err != nil {
result.Error = err.Error()
} else {
txConfig.LogIndex = logIndex
Copy link
Contributor

Choose a reason for hiding this comment

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

Why assign txConfig.LogIndex here when it doesn't get returned or used in the rest of the function?

result.Result = traceResult
}
return lastDB, result
}

// TraceBlock configures a new tracer according to the provided configuration, and
// executes the given message in the provided environment for all the transactions in the queried block.
// The return value will be tracer dependent.
Expand Down Expand Up @@ -499,24 +559,18 @@ func (k Keeper) TraceBlock(c context.Context, req *types.QueryTraceBlockRequest)
if err != nil {
return nil, status.Error(codes.Internal, "failed to load evm config")
}
signer := ethtypes.MakeSigner(cfg.ChainConfig, big.NewInt(ctx.BlockHeight()))
height := ctx.BlockHeight()
patch := height < req.FixClearAccessListHeight
signer := ethtypes.MakeSigner(cfg.ChainConfig, big.NewInt(height))
txsLength := len(req.Txs)
results := make([]*types.TxTraceResult, 0, txsLength)

txConfig := statedb.NewEmptyTxConfig(common.BytesToHash(ctx.HeaderHash().Bytes()))
var lastDB *statedb.StateDB
for i, tx := range req.Txs {
result := types.TxTraceResult{}
ethTx := tx.AsTransaction()
txConfig.TxHash = ethTx.Hash()
txConfig.TxIndex = uint(i)
traceResult, logIndex, err := k.traceTx(ctx, cfg, txConfig, signer, ethTx, req.TraceConfig, true, nil)
if err != nil {
result.Error = err.Error()
} else {
txConfig.LogIndex = logIndex
result.Result = traceResult
}
results = append(results, &result)
var result *types.TxTraceResult
lastDB, result = k.GetTxTraceResultForBlock(ctx, tx, signer, cfg, txConfig, req.TraceConfig, patch, lastDB)
results = append(results, result)
}

resultData, err := json.Marshal(results)
Expand All @@ -534,6 +588,7 @@ func (k *Keeper) traceTx(
ctx sdk.Context,
cfg *statedb.EVMConfig,
txConfig statedb.TxConfig,
stateDB *statedb.StateDB,
signer ethtypes.Signer,
tx *ethtypes.Transaction,
traceConfig *types.TraceConfig,
Expand Down Expand Up @@ -602,7 +657,7 @@ func (k *Keeper) traceTx(
}
}()

res, err := k.ApplyMessageWithConfig(ctx, msg, tracer, commitMessage, cfg, txConfig)
res, err := k.ApplyMessageWithStateDB(ctx, msg, tracer, commitMessage, cfg, txConfig, stateDB)
if err != nil {
return nil, 0, status.Error(codes.Internal, err.Error())
}
Expand Down
Loading