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 9 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 @@ -113,6 +113,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (rpc) [#1484](https://github.com/evmos/ethermint/pull/1484) Align empty account result for old blocks as ethereum instead of return account not found error.
* (rpc) [#1503](https://github.com/evmos/ethermint/pull/1503) Fix block hashes returned on JSON-RPC filter `eth_newBlockFilter`.
* (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
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
35 changes: 19 additions & 16 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ type JSONRPCConfig struct {
EnableIndexer bool `mapstructure:"enable-indexer"`
// MetricsAddress defines the metrics server to listen on
MetricsAddress string `mapstructure:"metrics-address"`
// 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 @@ -325,22 +327,23 @@ func GetConfig(v *viper.Viper) (Config, error) {
MaxTxGasWanted: v.GetUint64("evm.max-tx-gas-wanted"),
},
JSONRPC: JSONRPCConfig{
Enable: v.GetBool("json-rpc.enable"),
API: v.GetStringSlice("json-rpc.api"),
Address: v.GetString("json-rpc.address"),
WsAddress: v.GetString("json-rpc.ws-address"),
GasCap: v.GetUint64("json-rpc.gas-cap"),
FilterCap: v.GetInt32("json-rpc.filter-cap"),
FeeHistoryCap: v.GetInt32("json-rpc.feehistory-cap"),
TxFeeCap: v.GetFloat64("json-rpc.txfee-cap"),
EVMTimeout: v.GetDuration("json-rpc.evm-timeout"),
LogsCap: v.GetInt32("json-rpc.logs-cap"),
BlockRangeCap: v.GetInt32("json-rpc.block-range-cap"),
HTTPTimeout: v.GetDuration("json-rpc.http-timeout"),
HTTPIdleTimeout: v.GetDuration("json-rpc.http-idle-timeout"),
MaxOpenConnections: v.GetInt("json-rpc.max-open-connections"),
EnableIndexer: v.GetBool("json-rpc.enable-indexer"),
MetricsAddress: v.GetString("json-rpc.metrics-address"),
Enable: v.GetBool("json-rpc.enable"),
API: v.GetStringSlice("json-rpc.api"),
Address: v.GetString("json-rpc.address"),
WsAddress: v.GetString("json-rpc.ws-address"),
GasCap: v.GetUint64("json-rpc.gas-cap"),
FilterCap: v.GetInt32("json-rpc.filter-cap"),
FeeHistoryCap: v.GetInt32("json-rpc.feehistory-cap"),
TxFeeCap: v.GetFloat64("json-rpc.txfee-cap"),
EVMTimeout: v.GetDuration("json-rpc.evm-timeout"),
LogsCap: v.GetInt32("json-rpc.logs-cap"),
BlockRangeCap: v.GetInt32("json-rpc.block-range-cap"),
HTTPTimeout: v.GetDuration("json-rpc.http-timeout"),
HTTPIdleTimeout: v.GetDuration("json-rpc.http-idle-timeout"),
MaxOpenConnections: v.GetInt("json-rpc.max-open-connections"),
EnableIndexer: v.GetBool("json-rpc.enable-indexer"),
MetricsAddress: v.GetString("json-rpc.metrics-address"),
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 @@ -92,6 +92,9 @@ enable-indexer = {{ .JSONRPC.EnableIndexer }}
# Prometheus metrics path: /debug/metrics/prometheus
metrics-address = "{{ .JSONRPC.MetricsAddress }}"

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

###############################################################################
### TLS Configuration ###
###############################################################################
Expand Down
3 changes: 2 additions & 1 deletion server/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ const (
// JSONRPCEnableMetrics enables EVM RPC metrics server.
// Set to `metrics` which is hardcoded flag from go-ethereum.
// https://github.com/ethereum/go-ethereum/blob/master/metrics/metrics.go#L35-L55
JSONRPCEnableMetrics = "metrics"
JSONRPCEnableMetrics = "metrics"
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 @@ -19,6 +19,7 @@
'feehistory-cap': 100,
'block-range-cap': 10000,
'logs-cap': 10000,
'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