Skip to content
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.

Sign up for GitHub

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

Jump to bottom

internal: add eth_batchCall method #25743

Closed
wants to merge 2 commits into from
+777 −36
Closed

internal: add eth_batchCall method #25743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
47a90f7
internal/ethapi: add eth_batchCall method
s1na Sep 7, 2022
bdf0fa7
not abort on revert of one call
s1na Sep 27, 2022
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
5 changes: 4 additions & 1 deletion eth/api_backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,15 @@ func (b *EthAPIBackend) GetTd(ctx context.Context, hash common.Hash) *big.Int {
return nil
}

func (b *EthAPIBackend) GetEVM(ctx context.Context, msg core.Message, state *state.StateDB, header *types.Header, vmConfig *vm.Config) (*vm.EVM, func() error, error) {
func (b *EthAPIBackend) GetEVM(ctx context.Context, msg core.Message, state *state.StateDB, header *types.Header, vmConfig *vm.Config, blockContext *vm.BlockContext) (*vm.EVM, func() error, error) {
if vmConfig == nil {
vmConfig = b.eth.blockchain.GetVMConfig()
}
txContext := core.NewEVMTxContext(msg)
context := core.NewEVMBlockContext(header, b.eth.BlockChain(), nil)
if blockContext != nil {
context = *blockContext
}
return vm.NewEVM(context, txContext, state, b.eth.blockchain.Config(), *vmConfig), state.Error, nil
}

Expand Down
26 changes: 1 addition & 25 deletions eth/tracers/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,34 +93,10 @@ func NewAPI(backend Backend) *API {
return &API{backend: backend}
}

type chainContext struct {
api *API
ctx context.Context
}

func (context *chainContext) Engine() consensus.Engine {
return context.api.backend.Engine()
}

func (context *chainContext) GetHeader(hash common.Hash, number uint64) *types.Header {
header, err := context.api.backend.HeaderByNumber(context.ctx, rpc.BlockNumber(number))
if err != nil {
return nil
}
if header.Hash() == hash {
return header
}
header, err = context.api.backend.HeaderByHash(context.ctx, hash)
if err != nil {
return nil
}
return header
}

// chainContext constructs the context reader which is used by the evm for reading
// the necessary chain context.
func (api *API) chainContext(ctx context.Context) core.ChainContext {
return &chainContext{api: api, ctx: ctx}
return ethapi.NewChainContext(ctx, api.backend)
}

// blockByNumber is the wrapper of the chain access function offered by the backend.
Expand Down
1 change: 0 additions & 1 deletion eth/tracers/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,6 @@ func TestTracingWithOverrides(t *testing.T) {
From: &accounts[0].addr,
// BLOCKNUMBER PUSH1 MSTORE
Input: newRPCBytes(common.Hex2Bytes("4360005260206000f3")),
//&hexutil.Bytes{0x43}, // blocknumber
},
config: &TraceCallConfig{
BlockOverrides: &ethapi.BlockOverrides{Number: (*hexutil.Big)(big.NewInt(0x1337))},
Expand Down
2 changes: 1 addition & 1 deletion ethclient/ethclient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func newTestBackend(t *testing.T) (*node.Node, []*types.Block) {
t.Fatalf("can't create new node: %v", err)
}
// Create Ethereum Service
config := &ethconfig.Config{Genesis: genesis}
config := &ethconfig.Config{Genesis: genesis, RPCGasCap: 50000000}
config.Ethash.PowMode = ethash.ModeFake
ethservice, err := eth.New(n, config)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion ethclient/gethclient/gethclient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func newTestBackend(t *testing.T) (*node.Node, []*types.Block) {
t.Fatalf("can't create new node: %v", err)
}
// Create Ethereum Service
config := &ethconfig.Config{Genesis: genesis}
config := &ethconfig.Config{Genesis: genesis, RPCGasCap: 50000000}
config.Ethash.PowMode = ethash.ModeFake
ethservice, err := eth.New(n, config)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions graphql/graphql_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ func newGQLService(t *testing.T, stack *node.Node, gspec *core.Genesis, genBlock
TrieDirtyCache: 5,
TrieTimeout: 60 * time.Minute,
SnapshotCache: 5,
RPCGasCap: 50000000,
}
ethBackend, err := eth.New(stack, ethConf)
if err != nil {
Expand Down
117 changes: 113 additions & 4 deletions internal/ethapi/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/common/math"
"github.com/ethereum/go-ethereum/consensus"
"github.com/ethereum/go-ethereum/consensus/ethash"
"github.com/ethereum/go-ethereum/consensus/misc"
"github.com/ethereum/go-ethereum/core"
Expand Down Expand Up @@ -946,6 +947,38 @@ func (diff *BlockOverrides) Apply(blockCtx *vm.BlockContext) {
}
}

// ChainContextBackend provides methods required to implement ChainContext.
type ChainContextBackend interface {
Engine() consensus.Engine
HeaderByNumber(context.Context, rpc.BlockNumber) (*types.Header, error)
}

// ChainContext is an implementation of core.ChainContext. It's main use-case
// is instantiating a vm.BlockContext without having access to the BlockChain object.
type ChainContext struct {
b ChainContextBackend
ctx context.Context
}

// NewChainContext creates a new ChainContext object.
func NewChainContext(ctx context.Context, backend ChainContextBackend) *ChainContext {
return &ChainContext{ctx: ctx, b: backend}
}

func (context *ChainContext) Engine() consensus.Engine {
return context.b.Engine()
}

func (context *ChainContext) GetHeader(hash common.Hash, number uint64) *types.Header {
s1na marked this conversation as resolved.
Show resolved Hide resolved
// This method is called to get the hash for a block number when executing the BLOCKHASH
// opcode. Hence no need to search for non-canonical blocks.
header, err := context.b.HeaderByNumber(context.ctx, rpc.BlockNumber(number))
if err != nil || header.Hash() != hash {
return nil
}
return header
}

func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides *StateOverride, timeout time.Duration, globalGasCap uint64) (*core.ExecutionResult, error) {
defer func(start time.Time) { log.Debug("Executing EVM call finished", "runtime", time.Since(start)) }(time.Now())

Expand All @@ -967,13 +1000,16 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash
// Make sure the context is cancelled when the call has completed
// this makes sure resources are cleaned up.
defer cancel()
return doCall(ctx, b, args, state, header, timeout, new(core.GasPool).AddGas(globalGasCap), nil)
}

func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.StateDB, header *types.Header, timeout time.Duration, gp *core.GasPool, blockContext *vm.BlockContext) (*core.ExecutionResult, error) {
// Get a new instance of the EVM.
msg, err := args.ToMessage(globalGasCap, header.BaseFee)
msg, err := args.ToMessage(gp.Gas(), header.BaseFee)
if err != nil {
return nil, err
}
evm, vmError, err := b.GetEVM(ctx, msg, state, header, &vm.Config{NoBaseFee: true})
evm, vmError, err := b.GetEVM(ctx, msg, state, header, &vm.Config{NoBaseFee: true}, blockContext)
if err != nil {
return nil, err
}
Expand All @@ -985,7 +1021,6 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash
}()

// Execute the message.
gp := new(core.GasPool).AddGas(math.MaxUint64)
result, err := core.ApplyMessage(evm, msg, gp)
if err := vmError(); err != nil {
return nil, err
Expand Down Expand Up @@ -1049,6 +1084,80 @@ func (s *BlockChainAPI) Call(ctx context.Context, args TransactionArgs, blockNrO
return result.Return(), result.Err
}

// BatchCallConfig is the config object to be passed to eth_batchCall.
type BatchCallConfig struct {
Block rpc.BlockNumberOrHash
StateOverrides *StateOverride
Calls []BatchCallArgs
}

// BatchCallArgs is the object specifying each call within eth_batchCall. It
// extends TransactionArgs with the list of block metadata overrides.
type BatchCallArgs struct {
TransactionArgs
BlockOverrides *BlockOverrides
}

// CallResult is the result of one call.
type CallResult struct {
Return hexutil.Bytes
Error error
}
Comment on lines +1101 to +1105
Copy link

Choose a reason for hiding this comment

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

I really like that this is finally being implemented in the core protocol since it makes it very easy to do a number of things.

It'd be really great if this also includes gas consumed by the particular call. It'd help to set more appropriate gas limits when someone needs to sign the next transactions while the initial state-changing transactions are pending/not broadcasted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's useful we can easily return the gas used here. But can you please elaborate on your use-case? I didn't quite understand.

Copy link

@zemse zemse Sep 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

An example of simple use-case: erc20 approve + defi interaction.

Problem: eth_estimateGas done before the erc20 approve getting confirmed would usually revert. Hence the normal flow of UX is to send approve tx and wait for it to confirm then do the defi interaction tx. This is well known to be not good UX because of increased user waiting times. Using huge input gas limit is not the good solution because wallets show max eth fees (gas price * gas limit) which looks costly and some users might not have enough eth.

Solution: if eth_batchCall includes gasUsed, then it can be used instead of eth_estimateGas where the erc20 approve tx is mentioned as first call and then the defi interaction as second call and it's gasUsed can be used to very accurately estimate gas even before prervious transaction is confirmed. The improved UX for this becomes: click on button in dapp once, hit confirm on metamask/wallet twice, check back in like few mins if both txs are confirmed. Similarly, if the user had to do a lot of steps like approve + deposit + stake + what not, a dapp could use eth_batchCall to simulate the UI state after a user interaction and create list of txs to submit and get them signed at once and accurately estimate the gas limit (similar to github PR reviews where we can add lot of comments while scrolling at our convenience and it gets submited all at once). This saves a lot of user's waiting time, and hence has the potential to improve the UX considerably.

TLDR including gasUsed basically enables estimating gas on a state updated after a series of calls. I hope the usecase makes sense.

Edit: I just came across a project (created by Uniswap engineer) that exposes an endpoint for batch estimateGas using mainnet fork (link), use-case mentioned in their README beginning is exactly what I am trying to explain above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok now I understand the use-case, thanks for extensive description. I think adding gasUsed to the result is not a good solution. If you look, logic of eth_estimateGas is more complicated than simply doing a eth_call and reporting the gasUsed. This AFAIK is because the gaslimit provided to the tx can change the flow of the tx itself (GAS opcode).

But the use-case is valid IMO and warrants a eth_batchEstimateGas or something of the sort.

Copy link
Member

Choose a reason for hiding this comment

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

@s1na I had a implementation for batchEstimateGas way way back ago #21268

But I think your approach is much better, would be appreciate you can also take over this with the same design(e.g. state overrides, block overrides, etc)

Copy link

Choose a reason for hiding this comment

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

Yeah, it makes sense. So if gas is not specified in a call object it'd assume a large value, in order to ensure complex state-changing calls follow a successful execution path if there exists any, wouldn't users need to use eth_batchEstimateGas (or something like that) for setting the gas field in the calls prior to using eth_batchCall?

Copy link

Choose a reason for hiding this comment

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

An example of simple use-case: erc20 approve + defi interaction.

Problem: eth_estimateGas done before the erc20 approve getting confirmed would usually revert. Hence the normal flow of UX is to send approve tx and wait for it to confirm then do the defi interaction tx. This is well known to be not good UX because of increased user waiting times. Using huge input gas limit is not the good solution because wallets show max eth fees (gas price * gas limit) which looks costly and some users might not have enough eth.

Solution: if eth_batchCall includes gasUsed, then it can be used instead of eth_estimateGas where the erc20 approve tx is mentioned as first call and then the defi interaction as second call and it's gasUsed can be used to very accurately estimate gas even before prervious transaction is confirmed. The improved UX for this becomes: click on button in dapp once, hit confirm on metamask/wallet twice, check back in like few mins if both txs are confirmed. Similarly, if the user had to do a lot of steps like approve + deposit + stake + what not, a dapp could use eth_batchCall to simulate the UI state after a user interaction and create list of txs to submit and get them signed at once and accurately estimate the gas limit (similar to github PR reviews where we can add lot of comments while scrolling at our convenience and it gets submited all at once). This saves a lot of user's waiting time, and hence has the potential to improve the UX considerably.

TLDR including gasUsed basically enables estimating gas on a state updated after a series of calls. I hope the usecase makes sense.

Edit: I just came across a project (created by Uniswap engineer) that exposes an endpoint for batch estimateGas using mainnet fork (link), use-case mentioned in their README beginning is exactly what I am trying to explain above.

gas used through a wrapper contract is not accurate with Multicall due to EIP-2929, so should be avoided FYI (this is why Uniswap made this endpoint i think)


// BatchCall executes a series of transactions on the state of a given block as base.
// The base state can be overridden once before transactions are executed.
//
// Additionally, each call can override block context fields such as number.
//
// Note, this function doesn't make any changes in the state/blockchain and is
// useful to execute and retrieve values.
func (s *BlockChainAPI) BatchCall(ctx context.Context, config BatchCallConfig) ([]CallResult, error) {
Copy link
Member

Choose a reason for hiding this comment

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

It feels weird that we put all arguments in a config object. I can get the point that it's way more flexible and can be easily extended in the future.

Maybe we can put Block rpc.BlockNumberOrHash and Calls []BatchCallArgs as standalone parameters and with a config object for specifying the additional configurations(state overrides, etc)? Just a braindump though.

state, header, err := s.b.StateAndHeaderByNumberOrHash(ctx, config.Block)
if state == nil || err != nil {
return nil, err
}
// State overrides are applied once before all calls
if err := config.StateOverrides.Apply(state); err != nil {
return nil, err
}
// Setup context so it may be cancelled before the calls completed
// or, in case of unmetered gas, setup a context with a timeout.
var (
cancel context.CancelFunc
timeout = s.b.RPCEVMTimeout()
)
if timeout > 0 {
ctx, cancel = context.WithTimeout(ctx, timeout)
} else {
ctx, cancel = context.WithCancel(ctx)
}
// Make sure the context is cancelled when the call has completed
// this makes sure resources are cleaned up.
defer cancel()
var (
results []CallResult
// Each tx and all the series of txes shouldn't consume more gas than cap
globalGasCap = s.b.RPCGasCap()
gp = new(core.GasPool).AddGas(globalGasCap)
)
for _, call := range config.Calls {
blockContext := core.NewEVMBlockContext(header, NewChainContext(ctx, s.b), nil)
if call.BlockOverrides != nil {
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand why we have call-level block overrides. In practice these calls usually have the same block context if they want to be put in a single block by intention?

Copy link
Member

Choose a reason for hiding this comment

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

Because users can simulate the case that transactions are included in different blocks? If so I think this design makes sense.

Copy link
Member

Choose a reason for hiding this comment

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

Micah also asked a similar question here: ethereum/execution-apis#312 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

On one hand, it makes sense. For example, if you want to experiment with a time-locked contract. First you create it, then two years pass, now you want to interact again.
It opens up a lot of potential uses which does not fit inside a single block.

However, it might also be a footgun. If you want to simulate a sequence where

  1. Block n: A contract X is selfdestructed,
  2. Block n+1, contrat X is resurrected.

The two steps can never happen in one block. The question is: what happens in the batch-call? Is it possible to make the two calls execute correctly, or will it be some form of "time-shifted single block", where you can override the time and number, but state-processing-wise it's still the same block.... ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I also am concerned that not all clients will have an easy time implementing this as currently specified. I suspect all clients could have two distinct blocks that they execute against some existing block's post-state, but not all clients may be able to simulate a series of transcations against a cohesive state when the transaction's don't share block fields.

Would be great to get other client feedback on this to verify, but without any feedback I would assume the worst that this will be "hard" to implement in some clients. Having writing some Nethermind plugins, my gut suggests that this would be challenging to do with Nethermind for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about this some more, and I think it would be better to just allow the user to multicall and have multiple blocks, each with different transactions. The model may look something like:

[ { block_n_details, block_n_transactions }, { block_m_details, block_m_transactions }, ... ]

We would still require normal rules to be respected between blocks (like block numbers are incrementing, timestamp in future blocks must be higher number than previous blocks, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or will it be some form of "time-shifted single block", where you can override the time and number, but state-processing-wise it's still the same block.... ?

This is a good point. As the implementation stands there are differences to how a sequence of blocks are executed (one being coinbase fee). As I mentioned here ethereum/execution-apis#312 (comment) I would like to proceed with "only" the single-block-multi-call variant. This would already be a big improvement for users and I would prefer not to delay that for something more complicated at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

This would already be a big improvement for users and I would prefer not to delay that for something more complicated at the moment.

I think it is only notably more complicated if you try to do different overrides of transaction details within a single block. My proposal is to actually have multiple blocks, each which would follow most consensus rules (like timestamps must increase, block number must increase, etc.). I believe the complexity that Martin is referring to is specifically related to how the original proposal was designed where you have one "block" but each transaction had different block properties reflected in it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about this some more, ...
We would still require normal rules to be respected between blocks (like block numbers are incrementing, timestamp in future blocks must be higher number than previous blocks, etc.

I've also been thinking about this some more - and I reached a different conclusion :) In fact, kind of the opposite. I was thinking that we could make this call very much up to the caller. We would not do any form of sanity checks. If the user wants to do block 1, 500, 498, 1M, 3 in sequence, while letting timestamp go backwards, then fine. It's up to the caller to use this thing "correctly".

In that sense, I don't see any need to enforce "separate blocks". (To be concrete, I think that only means shipping the fees to the coinbase, so that is not a biggie really. )

I do foresee a couple of problems that maybe should be agreed with the other clients:

  1. When EVM invokes the BLOCKHASH(number) opcode. How should we 'resolve' blockhash when the block number is overridden. Possible semantics
    • Always return emptyhash
    • Always return keccak256(num)
    • Return as if it were executed on the current block, ignoring overrides.

Currently, geth does the third option, since the blockcontext.GetHash function is set before any block overrides:

	vmctx := core.NewEVMBlockContext(block.Header(), api.chainContext(ctx), nil)
	// Apply the customization rules if required.
	if config != nil {
		if err := config.StateOverrides.Apply(statedb); err != nil {
			return nil, err
		}
		config.BlockOverrides.Apply(&vmctx)
	}

.... I think there was one more thing I meant to write, but I've forgotten now....

Copy link
Contributor

Choose a reason for hiding this comment

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

I've also been thinking about this some more - and I reached a different conclusion :) In fact, kind of the opposite. I was thinking that we could make this call very much up to the caller. We would not do any form of sanity checks. If the user wants to do block 1, 500, 498, 1M, 3 in sequence, while letting timestamp go backwards, then fine. It's up to the caller to use this thing "correctly".

My concern with this strategy is that some clients (or possibly future clients) may be architected such that disabling basic validation checks like "block number go up" in an area that is harder to override during calls. This is, of course, speculation on my part but it aligns with my general preference toward keeping the multicall as close to actual block building as possible. I also can't think of any good use cases where having block number or time go backwards would help someone, so it feels like unnecessary leniency.

When EVM invokes the BLOCKHASH(number) opcode. How should we 'resolve' blockhash when the block number is overridden. Possible semantics

* Always return emptyhash

* Always return `keccak256(num)`

* Return as if it were executed on the current block, ignoring overrides.

I think there is a fourth option to include it in the potential overrides, so the caller would say "when you execute this block and BLOCKHASH(n) is called, return this value". The caller could provide an array of n to blockhash values (they presumably know what set they need). We could then fallback to one of the "reasonable defaults" that you have listed.

call.BlockOverrides.Apply(&blockContext)
}
result, err := doCall(ctx, s.b, call.TransactionArgs, state, header, timeout, gp, &blockContext)
if err != nil {
return nil, err
}
// If the result contains a revert reason, try to unpack it.
if len(result.Revert()) > 0 {
result.Err = newRevertError(result)
}
results = append(results, CallResult{Return: result.Return(), Error: result.Err})
}
return results, nil
}

func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap uint64) (hexutil.Uint64, error) {
// Binary search the gas requirement, as it may be higher than the amount used
var (
Expand Down Expand Up @@ -1459,7 +1568,7 @@ func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrH
// Apply the transaction with the access list tracer
tracer := logger.NewAccessListTracer(accessList, args.from(), to, precompiles)
config := vm.Config{Tracer: tracer, Debug: true, NoBaseFee: true}
vmenv, _, err := b.GetEVM(ctx, msg, statedb, header, &config)
vmenv, _, err := b.GetEVM(ctx, msg, statedb, header, &config, nil)
if err != nil {
return nil, 0, nil, err
}
Expand Down
Loading