-
Notifications
You must be signed in to change notification settings - Fork 20.9k
internal: add eth_batchCall method #25743
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
|
@@ -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 { | ||
// 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()) | ||
|
||
|
@@ -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 | ||
} | ||
|
@@ -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 | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An example of simple use-case: Problem: Solution: if 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 But the use-case is valid IMO and warrants a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it makes sense. So if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Micah also asked a similar question here: ethereum/execution-apis#312 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. However, it might also be a footgun. If you want to simulate a sequence where
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.... ? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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:
Currently, geth does the third option, since the 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.... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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
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 |
||
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 ( | ||
|
@@ -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 | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.