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

ethapi: add block override to estimateGas #30695

Merged
merged 6 commits into from
Dec 12, 2024
Merged

ethapi: add block override to estimateGas #30695

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
abab344
ethapi: add block override to estimateGas
antonydenyer Oct 17, 2024
b304b13
Inline packRevert function as only used in test
antonydenyer Nov 1, 2024
c5af175
Move gasestimator into the ethapi package
antonydenyer Nov 6, 2024
6475f5a
Revert "Move gasestimator into the ethapi package"
s1na Nov 27, 2024
1d6637e
move overrides to own package
s1na Nov 27, 2024
3def46a
state override no precompile
s1na Nov 27, 2024
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 .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ profile.cov

# IdeaIDE
.idea
*.iml

# VS Code
.vscode
Expand Down
4 changes: 2 additions & 2 deletions graphql/graphql.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,7 @@ func (b *Block) Call(ctx context.Context, args struct {
func (b *Block) EstimateGas(ctx context.Context, args struct {
Data ethapi.TransactionArgs
}) (hexutil.Uint64, error) {
return ethapi.DoEstimateGas(ctx, b.r.backend, args.Data, *b.numberOrHash, nil, b.r.backend.RPCGasCap())
return ethapi.DoEstimateGas(ctx, b.r.backend, args.Data, *b.numberOrHash, nil, nil, b.r.backend.RPCGasCap())
}

type Pending struct {
Expand Down Expand Up @@ -1272,7 +1272,7 @@ func (p *Pending) EstimateGas(ctx context.Context, args struct {
Data ethapi.TransactionArgs
}) (hexutil.Uint64, error) {
latestBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.LatestBlockNumber)
return ethapi.DoEstimateGas(ctx, p.r.backend, args.Data, latestBlockNr, nil, p.r.backend.RPCGasCap())
return ethapi.DoEstimateGas(ctx, p.r.backend, args.Data, latestBlockNr, nil, nil, p.r.backend.RPCGasCap())
}

// Resolver is the top-level object in the GraphQL hierarchy.
Expand Down
20 changes: 14 additions & 6 deletions internal/ethapi/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,20 +973,28 @@ func (api *BlockChainAPI) SimulateV1(ctx context.Context, opts simOpts, blockNrO
// successfully at block `blockNrOrHash`. It returns error if the transaction would revert, or if
// there are unexpected failures. The gas limit is capped by both `args.Gas` (if non-nil &
// non-zero) and `gasCap` (if non-zero).
func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides *StateOverride, gasCap uint64) (hexutil.Uint64, error) {
func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides *StateOverride, blockOverrides *BlockOverrides, gasCap uint64) (hexutil.Uint64, error) {
// Retrieve the base state and mutate it with any overrides
state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash)
if state == nil || err != nil {
return 0, err
}
if err := overrides.Apply(state, nil); err != nil {
chainContext := NewChainContext(ctx, b)
blockCtx := core.NewEVMBlockContext(header, chainContext, nil)
if blockOverrides != nil {
blockOverrides.Apply(&blockCtx)
}

rules := b.ChainConfig().Rules(blockCtx.BlockNumber, blockCtx.Random != nil, blockCtx.Time)
precompiles := maps.Clone(vm.ActivePrecompiledContracts(rules))
if err := overrides.Apply(state, precompiles); err != nil {
return 0, err
}
// Construct the gas estimator option from the user input
opts := &gasestimator.Options{
Config: b.ChainConfig(),
Chain: NewChainContext(ctx, b),
Header: header,
Chain: chainContext,
Header: blockOverrides.MakeHeader(header),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this line. The EVM doesn't have access to the header directly anyway. It only accesses block fields via the block context object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it took me a while to find this, I was taking most of the code fromDoCall path which doesn't do this. But the problem with the estimate path is that it uses the header in https://github.com/ethereum/go-ethereum/blob/master/eth/gasestimator/gasestimator.go#L221

Without this line the block overrides are not available in solidity code when running the unit test; this could be because is the test isn't setup correctly or it could genuinely be needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Riighhtt this made me realize we should do this differently. Right now you're effectively only applying the block overrides to mutate the header and pass that to the gasestimator.

IMO we should pass in the overrides to gasestimator and apply them there just before execution. But that will create an import cycle. Hmm...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was surprised by the number of differences between the two endpoints. They are basically the same thing, except when they are not!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another option is to move the gasestimator package up into the ethapi package to prevent this.

let me know what you think ...
c5af175

Copy link
Contributor

Choose a reason for hiding this comment

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

This was a deliberate change to move gas estimator out of rpc: #28600. let me think about it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, I could pass it in on the options, but that didn't feel right.

State: state,
ErrorRatio: estimateGasErrorRatio,
}
Expand Down Expand Up @@ -1017,12 +1025,12 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
// value is capped by both `args.Gas` (if non-nil & non-zero) and the backend's RPCGasCap
// configuration (if non-zero).
// Note: Required blob gas is not computed in this method.
func (api *BlockChainAPI) EstimateGas(ctx context.Context, args TransactionArgs, blockNrOrHash *rpc.BlockNumberOrHash, overrides *StateOverride) (hexutil.Uint64, error) {
func (api *BlockChainAPI) EstimateGas(ctx context.Context, args TransactionArgs, blockNrOrHash *rpc.BlockNumberOrHash, overrides *StateOverride, blockOverrides *BlockOverrides) (hexutil.Uint64, error) {
bNrOrHash := rpc.BlockNumberOrHashWithNumber(rpc.LatestBlockNumber)
if blockNrOrHash != nil {
bNrOrHash = *blockNrOrHash
}
return DoEstimateGas(ctx, api.b, args, bNrOrHash, overrides, api.b.RPCGasCap())
return DoEstimateGas(ctx, api.b, args, bNrOrHash, overrides, blockOverrides, api.b.RPCGasCap())
}

// RPCMarshalHeader converts the given header to the RPC output .
Expand Down
125 changes: 111 additions & 14 deletions internal/ethapi/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import (
"testing"
"time"

"github.com/ethereum/go-ethereum/accounts/abi"

"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/accounts"
"github.com/ethereum/go-ethereum/accounts/keystore"
Expand Down Expand Up @@ -641,6 +643,17 @@ func TestEstimateGas(t *testing.T) {
signer = types.HomesteadSigner{}
randomAccounts = newAccounts(2)
)
packRevert := func(revertMessage string) []byte {
var revertSelector = crypto.Keccak256([]byte("Error(string)"))[:4]
stringType, _ := abi.NewType("string", "", nil)
args := abi.Arguments{
{Type: stringType},
}
encodedMessage, _ := args.Pack(revertMessage)

return append(revertSelector, encodedMessage...)
}

api := NewBlockChainAPI(newTestBackend(t, genBlocks, genesis, beacon.New(ethash.NewFaker()), func(i int, b *core.BlockGen) {
// Transfer from account[0] to account[1]
// value: 1000 wei
Expand All @@ -649,14 +662,16 @@ func TestEstimateGas(t *testing.T) {
b.AddTx(tx)
b.SetPoS()
}))

var testSuite = []struct {
blockNumber rpc.BlockNumber
call TransactionArgs
overrides StateOverride
expectErr error
want uint64
blockNumber rpc.BlockNumber
call TransactionArgs
overrides StateOverride
blockOverrides BlockOverrides
expectErr error
want uint64
}{
// simple transfer on latest block
//simple transfer on latest block
{
blockNumber: rpc.LatestBlockNumber,
call: TransactionArgs{
Expand Down Expand Up @@ -759,16 +774,65 @@ func TestEstimateGas(t *testing.T) {
},
want: 21000,
},
// // SPDX-License-Identifier: GPL-3.0
//pragma solidity >=0.8.2 <0.9.0;
//
//contract BlockOverridesTest {
// function call() public view returns (uint256) {
// return block.number;
// }
//
// function estimate() public view {
// revert(string.concat("block ", uint2str(block.number)));
// }
//
// function uint2str(uint256 _i) internal pure returns (string memory str) {
// if (_i == 0) {
// return "0";
// }
// uint256 j = _i;
// uint256 length;
// while (j != 0) {
// length++;
// j /= 10;
// }
// bytes memory bstr = new bytes(length);
// uint256 k = length;
// j = _i;
// while (j != 0) {
// bstr[--k] = bytes1(uint8(48 + (j % 10)));
// j /= 10;
// }
// str = string(bstr);
// }
//}
{
blockNumber: rpc.LatestBlockNumber,
call: TransactionArgs{
From: &accounts[0].addr,
To: &accounts[1].addr,
Data: hex2Bytes("0x3592d016"), //estimate
},
overrides: StateOverride{
accounts[1].addr: OverrideAccount{
Code: hex2Bytes("608060405234801561000f575f5ffd5b5060043610610034575f3560e01c806328b5e32b146100385780633592d0161461004b575b5f5ffd5b4360405190815260200160405180910390f35b610053610055565b005b61005e4361009d565b60405160200161006e91906101a5565b60408051601f198184030181529082905262461bcd60e51b8252610094916004016101cd565b60405180910390fd5b6060815f036100c35750506040805180820190915260018152600360fc1b602082015290565b815f5b81156100ec57806100d681610216565b91506100e59050600a83610242565b91506100c6565b5f8167ffffffffffffffff81111561010657610106610255565b6040519080825280601f01601f191660200182016040528015610130576020820181803683370190505b508593509050815b831561019c57610149600a85610269565b61015490603061027c565b60f81b8261016183610295565b92508281518110610174576101746102aa565b60200101906001600160f81b03191690815f1a905350610195600a85610242565b9350610138565b50949350505050565b650313637b1b5960d51b81525f82518060208501600685015e5f920160060191825250919050565b602081525f82518060208401528060208501604085015e5f604082850101526040601f19601f83011684010191505092915050565b634e487b7160e01b5f52601160045260245ffd5b5f6001820161022757610227610202565b5060010190565b634e487b7160e01b5f52601260045260245ffd5b5f826102505761025061022e565b500490565b634e487b7160e01b5f52604160045260245ffd5b5f826102775761027761022e565b500690565b8082018082111561028f5761028f610202565b92915050565b5f816102a3576102a3610202565b505f190190565b634e487b7160e01b5f52603260045260245ffdfea2646970667358221220a253cad1e2e3523b8c053c1d0cd1e39d7f3bafcedd73440a244872701f05dab264736f6c634300081c0033"),
},
},
blockOverrides: BlockOverrides{Number: (*hexutil.Big)(big.NewInt(11))},
expectErr: newRevertError(packRevert("block 11")),
},
}
for i, tc := range testSuite {
result, err := api.EstimateGas(context.Background(), tc.call, &rpc.BlockNumberOrHash{BlockNumber: &tc.blockNumber}, &tc.overrides)
result, err := api.EstimateGas(context.Background(), tc.call, &rpc.BlockNumberOrHash{BlockNumber: &tc.blockNumber}, &tc.overrides, &tc.blockOverrides)
if tc.expectErr != nil {
if err == nil {
t.Errorf("test %d: want error %v, have nothing", i, tc.expectErr)
continue
}
if !errors.Is(err, tc.expectErr) {
t.Errorf("test %d: error mismatch, want %v, have %v", i, tc.expectErr, err)
if !reflect.DeepEqual(err, tc.expectErr) {
t.Errorf("test %d: error mismatch, want %v, have %v", i, tc.expectErr, err)
}
}
continue
}
Expand Down Expand Up @@ -929,16 +993,49 @@ func TestCall(t *testing.T) {
},
want: "0x000000000000000000000000000000000000000000000000000000000000007b",
},
// Block overrides should work
// // SPDX-License-Identifier: GPL-3.0
//pragma solidity >=0.8.2 <0.9.0;
//
//contract BlockOverridesTest {
// function call() public view returns (uint256) {
// return block.number;
// }
//
// function estimate() public view {
// revert(string.concat("block ", uint2str(block.number)));
// }
//
// function uint2str(uint256 _i) internal pure returns (string memory str) {
// if (_i == 0) {
// return "0";
// }
// uint256 j = _i;
// uint256 length;
// while (j != 0) {
// length++;
// j /= 10;
// }
// bytes memory bstr = new bytes(length);
// uint256 k = length;
// j = _i;
// while (j != 0) {
// bstr[--k] = bytes1(uint8(48 + (j % 10)));
// j /= 10;
// }
// str = string(bstr);
// }
//}
{
name: "block-override",
name: "block-override-with-state-override",
blockNumber: rpc.LatestBlockNumber,
call: TransactionArgs{
From: &accounts[1].addr,
Input: &hexutil.Bytes{
0x43, // NUMBER
0x60, 0x00, 0x52, // MSTORE offset 0
0x60, 0x20, 0x60, 0x00, 0xf3,
To: &accounts[2].addr,
Data: hex2Bytes("0x28b5e32b"), //call
},
overrides: StateOverride{
accounts[2].addr: OverrideAccount{
Code: hex2Bytes("608060405234801561000f575f5ffd5b5060043610610034575f3560e01c806328b5e32b146100385780633592d0161461004b575b5f5ffd5b4360405190815260200160405180910390f35b610053610055565b005b61005e4361009d565b60405160200161006e91906101a5565b60408051601f198184030181529082905262461bcd60e51b8252610094916004016101cd565b60405180910390fd5b6060815f036100c35750506040805180820190915260018152600360fc1b602082015290565b815f5b81156100ec57806100d681610216565b91506100e59050600a83610242565b91506100c6565b5f8167ffffffffffffffff81111561010657610106610255565b6040519080825280601f01601f191660200182016040528015610130576020820181803683370190505b508593509050815b831561019c57610149600a85610269565b61015490603061027c565b60f81b8261016183610295565b92508281518110610174576101746102aa565b60200101906001600160f81b03191690815f1a905350610195600a85610242565b9350610138565b50949350505050565b650313637b1b5960d51b81525f82518060208501600685015e5f920160060191825250919050565b602081525f82518060208401528060208501604085015e5f604082850101526040601f19601f83011684010191505092915050565b634e487b7160e01b5f52601160045260245ffd5b5f6001820161022757610227610202565b5060010190565b634e487b7160e01b5f52601260045260245ffd5b5f826102505761025061022e565b500490565b634e487b7160e01b5f52604160045260245ffd5b5f826102775761027761022e565b500690565b8082018082111561028f5761028f610202565b92915050565b5f816102a3576102a3610202565b505f190190565b634e487b7160e01b5f52603260045260245ffdfea2646970667358221220a253cad1e2e3523b8c053c1d0cd1e39d7f3bafcedd73440a244872701f05dab264736f6c634300081c0033"),
},
},
blockOverrides: BlockOverrides{Number: (*hexutil.Big)(big.NewInt(11))},
Expand Down
2 changes: 1 addition & 1 deletion internal/ethapi/transaction_args.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend, skipGas
BlobHashes: args.BlobHashes,
}
latestBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.LatestBlockNumber)
estimated, err := DoEstimateGas(ctx, b, callArgs, latestBlockNr, nil, b.RPCGasCap())
estimated, err := DoEstimateGas(ctx, b, callArgs, latestBlockNr, nil, nil, b.RPCGasCap())
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions internal/web3ext/web3ext.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,8 @@ web3._extend({
new web3._extend.Method({
name: 'estimateGas',
call: 'eth_estimateGas',
params: 3,
inputFormatter: [web3._extend.formatters.inputCallFormatter, web3._extend.formatters.inputBlockNumberFormatter, null],
params: 4,
inputFormatter: [web3._extend.formatters.inputCallFormatter, web3._extend.formatters.inputBlockNumberFormatter, null, null],
outputFormatter: web3._extend.utils.toDecimal
}),
new web3._extend.Method({
Expand Down