From 682875adff760a29a2bb0024190883e4b4dd5d72 Mon Sep 17 00:00:00 2001 From: Péter Szilágyi Date: Sat, 21 Jan 2017 00:39:16 +0200 Subject: accounts/abi/bind, internal/ethapi: binary search gas estimation (#3587) Gas estimation currently mostly works, but can underestimate for more funky refunds. This is because various ops (e.g. CALL) need more gas to run than they actually consume (e.g. 2300 stipend that is refunded if not used). With more intricate contract interplays, it becomes almost impossible to return a proper value to the user. This commit swaps out the simplistic gas estimation to a binary search approach, honing in on the correct gas use. This does mean that gas estimation needs to rerun the transaction log(max-price) times to measure whether it fails or not, but it's a price paid by the transaction issuer, and it should be worth it to support proper estimates. --- accounts/abi/bind/backends/simulated.go | 28 ++++++++++++++++-- accounts/abi/bind/bind_test.go | 51 ++++++++++++++++++++++++++++++--- internal/ethapi/api.go | 30 +++++++++++++++++-- 3 files changed, 100 insertions(+), 9 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index c26662ea1..1c34ba0e7 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -201,10 +201,32 @@ func (b *SimulatedBackend) SuggestGasPrice(ctx context.Context) (*big.Int, error func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMsg) (*big.Int, error) { b.mu.Lock() defer b.mu.Unlock() - defer b.pendingState.RevertToSnapshot(b.pendingState.Snapshot()) - _, gas, err := b.callContract(ctx, call, b.pendingBlock, b.pendingState) - return gas, err + // Binary search the gas requirement, as it may be higher than the amount used + var lo, hi uint64 + if call.Gas != nil { + hi = call.Gas.Uint64() + } else { + hi = b.pendingBlock.GasLimit().Uint64() + } + for lo+1 < hi { + // Take a guess at the gas, and check transaction validity + mid := (hi + lo) / 2 + call.Gas = new(big.Int).SetUint64(mid) + + snapshot := b.pendingState.Snapshot() + _, gas, err := b.callContract(ctx, call, b.pendingBlock, b.pendingState) + b.pendingState.RevertToSnapshot(snapshot) + + // If the transaction became invalid or used all the gas (failed), raise the gas limit + if err != nil || gas.Cmp(call.Gas) == 0 { + lo = mid + continue + } + // Otherwise assume the transaction succeeded, lower the gas limit + hi = mid + } + return new(big.Int).SetUint64(hi), nil } // callContract implemens common code between normal and pending contract calls. diff --git a/accounts/abi/bind/bind_test.go b/accounts/abi/bind/bind_test.go index 6ebc8ea0a..eb46bc081 100644 --- a/accounts/abi/bind/bind_test.go +++ b/accounts/abi/bind/bind_test.go @@ -341,11 +341,11 @@ var bindTests = []struct { { `NonExistent`, ` - contract NonExistent { - function String() constant returns(string) { - return "I don't exist"; + contract NonExistent { + function String() constant returns(string) { + return "I don't exist"; + } } - } `, `6060604052609f8060106000396000f3606060405260e060020a6000350463f97a60058114601a575b005b600060605260c0604052600d60809081527f4920646f6e27742065786973740000000000000000000000000000000000000060a052602060c0908152600d60e081905281906101009060a09080838184600060046012f15050815172ffffffffffffffffffffffffffffffffffffff1916909152505060405161012081900392509050f3`, `[{"constant":true,"inputs":[],"name":"String","outputs":[{"name":"","type":"string"}],"type":"function"}]`, @@ -365,6 +365,49 @@ var bindTests = []struct { } `, }, + // Tests that gas estimation works for contracts with weird gas mechanics too. + { + `FunkyGasPattern`, + ` + contract FunkyGasPattern { + string public field; + + function SetField(string value) { + // This check will screw gas estimation! Good, good! + if (msg.gas < 100000) { + throw; + } + field = value; + } + } + `, + `606060405261021c806100126000396000f3606060405260e060020a600035046323fcf32a81146100265780634f28bf0e1461007b575b005b6040805160206004803580820135601f8101849004840285018401909552848452610024949193602493909291840191908190840183828082843750949650505050505050620186a05a101561014e57610002565b6100db60008054604080516020601f600260001961010060018816150201909516949094049384018190048102820181019092528281529291908301828280156102145780601f106101e957610100808354040283529160200191610214565b60405180806020018281038252838181518152602001915080519060200190808383829060006004602084601f0104600302600f01f150905090810190601f16801561013b5780820380516001836020036101000a031916815260200191505b509250505060405180910390f35b505050565b8060006000509080519060200190828054600181600116156101000203166002900490600052602060002090601f016020900481019282601f106101b557805160ff19168380011785555b506101499291505b808211156101e557600081556001016101a1565b82800160010185558215610199579182015b828111156101995782518260005055916020019190600101906101c7565b5090565b820191906000526020600020905b8154815290600101906020018083116101f757829003601f168201915b50505050508156`, + `[{"constant":false,"inputs":[{"name":"value","type":"string"}],"name":"SetField","outputs":[],"type":"function"},{"constant":true,"inputs":[],"name":"field","outputs":[{"name":"","type":"string"}],"type":"function"}]`, + ` + // Generate a new random account and a funded simulator + key, _ := crypto.GenerateKey() + auth := bind.NewKeyedTransactor(key) + sim := backends.NewSimulatedBackend(core.GenesisAccount{Address: auth.From, Balance: big.NewInt(10000000000)}) + + // Deploy a funky gas pattern contract + _, _, limiter, err := DeployFunkyGasPattern(auth, sim) + if err != nil { + t.Fatalf("Failed to deploy funky contract: %v", err) + } + sim.Commit() + + // Set the field with automatic estimation and check that it succeeds + auth.GasLimit = nil + if _, err := limiter.SetField(auth, "automatic"); err != nil { + t.Fatalf("Failed to call automatically gased transaction: %v", err) + } + sim.Commit() + + if field, _ := limiter.Field(nil); field != "automatic" { + t.Fatalf("Field mismatch: have %v, want %v", field, "automatic") + } + `, + }, } // Tests that packages generated by the binder can be successfully compiled and diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 4359181c8..925f547b6 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -559,8 +559,34 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNr r // EstimateGas returns an estimate of the amount of gas needed to execute the given transaction. func (s *PublicBlockChainAPI) EstimateGas(ctx context.Context, args CallArgs) (*hexutil.Big, error) { - _, gas, err := s.doCall(ctx, args, rpc.PendingBlockNumber) - return (*hexutil.Big)(gas), err + // Binary search the gas requirement, as it may be higher than the amount used + var lo, hi uint64 + if (*big.Int)(&args.Gas).BitLen() > 0 { + hi = (*big.Int)(&args.Gas).Uint64() + } else { + // Retrieve the current pending block to act as the gas ceiling + block, err := s.b.BlockByNumber(ctx, rpc.PendingBlockNumber) + if err != nil { + return nil, err + } + hi = block.GasLimit().Uint64() + } + for lo+1 < hi { + // Take a guess at the gas, and check transaction validity + mid := (hi + lo) / 2 + (*big.Int)(&args.Gas).SetUint64(mid) + + _, gas, err := s.doCall(ctx, args, rpc.PendingBlockNumber) + + // If the transaction became invalid or used all the gas (failed), raise the gas limit + if err != nil || gas.Cmp((*big.Int)(&args.Gas)) == 0 { + lo = mid + continue + } + // Otherwise assume the transaction succeeded, lower the gas limit + hi = mid + } + return (*hexutil.Big)(new(big.Int).SetUint64(hi)), nil } // ExecutionResult groups all structured logs emitted by the EVM -- cgit