Skip to content

Commit 79b727b

Browse files
MariusVanDerWijdenyondonfufjl
authored
accounts/abi/bind: refactor transact method (#23719)
This fixes a bug where gas-related fields of the TransactOpts passed to transaction methods would be modified, skipping gas estimation for subsequent transactions. Co-authored-by: Yondon Fu <[email protected]> Co-authored-by: Felix Lange <[email protected]>
1 parent 778ff94 commit 79b727b

File tree

2 files changed

+217
-81
lines changed

2 files changed

+217
-81
lines changed

‎accounts/abi/bind/base.go‎

Lines changed: 131 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -231,108 +231,158 @@ func (c *BoundContract) Transfer(opts *TransactOpts) (*types.Transaction, error)
231231
return c.transact(opts, &c.address, nil)
232232
}
233233

234-
// transact executes an actual transaction invocation, first deriving any missing
235-
// authorization fields, and then scheduling the transaction for execution.
236-
func (c *BoundContract) transact(opts *TransactOpts, contract *common.Address, input []byte) (*types.Transaction, error) {
237-
var err error
238-
239-
// Ensure a valid value field and resolve the account nonce
234+
func (c *BoundContract) createDynamicTx(opts *TransactOpts, contract *common.Address, input []byte, head *types.Header) (*types.Transaction, error) {
235+
// Normalize value
240236
value := opts.Value
241237
if value == nil {
242238
value = new(big.Int)
243239
}
244-
var nonce uint64
245-
if opts.Nonce == nil {
246-
nonce, err = c.transactor.PendingNonceAt(ensureContext(opts.Context), opts.From)
240+
// Estimate TipCap
241+
gasTipCap := opts.GasTipCap
242+
if gasTipCap == nil {
243+
tip, err := c.transactor.SuggestGasTipCap(ensureContext(opts.Context))
247244
if err != nil {
248-
return nil, fmt.Errorf("failed to retrieve account nonce: %v", err)
245+
return nil, err
249246
}
250-
} else {
251-
nonce = opts.Nonce.Uint64()
247+
gasTipCap = tip
252248
}
253-
// Figure out reasonable gas price values
254-
if opts.GasPrice != nil && (opts.GasFeeCap != nil || opts.GasTipCap != nil) {
255-
return nil, errors.New("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified")
249+
// Estimate FeeCap
250+
gasFeeCap := opts.GasFeeCap
251+
if gasFeeCap == nil {
252+
gasFeeCap = new(big.Int).Add(
253+
gasTipCap,
254+
new(big.Int).Mul(head.BaseFee, big.NewInt(2)),
255+
)
256256
}
257-
head, err := c.transactor.HeaderByNumber(ensureContext(opts.Context), nil)
257+
if gasFeeCap.Cmp(gasTipCap) < 0 {
258+
return nil, fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", gasFeeCap, gasTipCap)
259+
}
260+
// Estimate GasLimit
261+
gasLimit := opts.GasLimit
262+
if opts.GasLimit == 0 {
263+
var err error
264+
gasLimit, err = c.estimateGasLimit(opts, contract, input, nil, gasTipCap, gasFeeCap, value)
265+
if err != nil {
266+
return nil, err
267+
}
268+
}
269+
// create the transaction
270+
nonce, err := c.getNonce(opts)
258271
if err != nil {
259272
return nil, err
260273
}
261-
if head.BaseFee != nil && opts.GasPrice == nil {
262-
if opts.GasTipCap == nil {
263-
tip, err := c.transactor.SuggestGasTipCap(ensureContext(opts.Context))
264-
if err != nil {
265-
return nil, err
266-
}
267-
opts.GasTipCap = tip
268-
}
269-
if opts.GasFeeCap == nil {
270-
gasFeeCap := new(big.Int).Add(
271-
opts.GasTipCap,
272-
new(big.Int).Mul(head.BaseFee, big.NewInt(2)),
273-
)
274-
opts.GasFeeCap = gasFeeCap
275-
}
276-
if opts.GasFeeCap.Cmp(opts.GasTipCap) < 0 {
277-
return nil, fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", opts.GasFeeCap, opts.GasTipCap)
278-
}
279-
} else {
280-
if opts.GasFeeCap != nil || opts.GasTipCap != nil {
281-
return nil, errors.New("maxFeePerGas or maxPriorityFeePerGas specified but london is not active yet")
282-
}
283-
if opts.GasPrice == nil {
284-
price, err := c.transactor.SuggestGasPrice(ensureContext(opts.Context))
285-
if err != nil {
286-
return nil, err
287-
}
288-
opts.GasPrice = price
274+
baseTx := &types.DynamicFeeTx{
275+
To: contract,
276+
Nonce: nonce,
277+
GasFeeCap: gasFeeCap,
278+
GasTipCap: gasTipCap,
279+
Gas: gasLimit,
280+
Value: value,
281+
Data: input,
282+
}
283+
return types.NewTx(baseTx), nil
284+
}
285+
286+
func (c *BoundContract) createLegacyTx(opts *TransactOpts, contract *common.Address, input []byte) (*types.Transaction, error) {
287+
if opts.GasFeeCap != nil || opts.GasTipCap != nil {
288+
return nil, errors.New("maxFeePerGas or maxPriorityFeePerGas specified but london is not active yet")
289+
}
290+
// Normalize value
291+
value := opts.Value
292+
if value == nil {
293+
value = new(big.Int)
294+
}
295+
// Estimate GasPrice
296+
gasPrice := opts.GasPrice
297+
if gasPrice == nil {
298+
price, err := c.transactor.SuggestGasPrice(ensureContext(opts.Context))
299+
if err != nil {
300+
return nil, err
289301
}
302+
gasPrice = price
290303
}
304+
// Estimate GasLimit
291305
gasLimit := opts.GasLimit
292-
if gasLimit == 0 {
293-
// Gas estimation cannot succeed without code for method invocations
294-
if contract != nil {
295-
if code, err := c.transactor.PendingCodeAt(ensureContext(opts.Context), c.address); err != nil {
296-
return nil, err
297-
} else if len(code) == 0 {
298-
return nil, ErrNoCode
299-
}
300-
}
301-
// If the contract surely has code (or code is not needed), estimate the transaction
302-
msg := ethereum.CallMsg{From: opts.From, To: contract, GasPrice: opts.GasPrice, GasTipCap: opts.GasTipCap, GasFeeCap: opts.GasFeeCap, Value: value, Data: input}
303-
gasLimit, err = c.transactor.EstimateGas(ensureContext(opts.Context), msg)
306+
if opts.GasLimit == 0 {
307+
var err error
308+
gasLimit, err = c.estimateGasLimit(opts, contract, input, gasPrice, nil, nil, value)
304309
if err != nil {
305-
return nil, fmt.Errorf("failed to estimate gas needed: %v", err)
310+
return nil, err
306311
}
307312
}
308-
// Create the transaction, sign it and schedule it for execution
309-
var rawTx *types.Transaction
310-
if opts.GasFeeCap == nil {
311-
baseTx := &types.LegacyTx{
312-
Nonce: nonce,
313-
GasPrice: opts.GasPrice,
314-
Gas: gasLimit,
315-
Value: value,
316-
Data: input,
317-
}
318-
if contract != nil {
319-
baseTx.To = &c.address
313+
// create the transaction
314+
nonce, err := c.getNonce(opts)
315+
if err != nil {
316+
return nil, err
317+
}
318+
baseTx := &types.LegacyTx{
319+
To: contract,
320+
Nonce: nonce,
321+
GasPrice: gasPrice,
322+
Gas: gasLimit,
323+
Value: value,
324+
Data: input,
325+
}
326+
return types.NewTx(baseTx), nil
327+
}
328+
329+
func (c *BoundContract) estimateGasLimit(opts *TransactOpts, contract *common.Address, input []byte, gasPrice, gasTipCap, gasFeeCap, value *big.Int) (uint64, error) {
330+
if contract != nil {
331+
// Gas estimation cannot succeed without code for method invocations.
332+
if code, err := c.transactor.PendingCodeAt(ensureContext(opts.Context), c.address); err != nil {
333+
return 0, err
334+
} else if len(code) == 0 {
335+
return 0, ErrNoCode
320336
}
321-
rawTx = types.NewTx(baseTx)
337+
}
338+
msg := ethereum.CallMsg{
339+
From: opts.From,
340+
To: contract,
341+
GasPrice: gasPrice,
342+
GasTipCap: gasTipCap,
343+
GasFeeCap: gasFeeCap,
344+
Value: value,
345+
Data: input,
346+
}
347+
return c.transactor.EstimateGas(ensureContext(opts.Context), msg)
348+
}
349+
350+
func (c *BoundContract) getNonce(opts *TransactOpts) (uint64, error) {
351+
if opts.Nonce == nil {
352+
return c.transactor.PendingNonceAt(ensureContext(opts.Context), opts.From)
322353
} else {
323-
baseTx := &types.DynamicFeeTx{
324-
Nonce: nonce,
325-
GasFeeCap: opts.GasFeeCap,
326-
GasTipCap: opts.GasTipCap,
327-
Gas: gasLimit,
328-
Value: value,
329-
Data: input,
330-
}
331-
if contract != nil {
332-
baseTx.To = &c.address
354+
return opts.Nonce.Uint64(), nil
355+
}
356+
}
357+
358+
// transact executes an actual transaction invocation, first deriving any missing
359+
// authorization fields, and then scheduling the transaction for execution.
360+
func (c *BoundContract) transact(opts *TransactOpts, contract *common.Address, input []byte) (*types.Transaction, error) {
361+
if opts.GasPrice != nil && (opts.GasFeeCap != nil || opts.GasTipCap != nil) {
362+
return nil, errors.New("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified")
363+
}
364+
// Create the transaction
365+
var (
366+
rawTx *types.Transaction
367+
err error
368+
)
369+
if opts.GasPrice != nil {
370+
rawTx, err = c.createLegacyTx(opts, contract, input)
371+
} else {
372+
// Only query for basefee if gasPrice not specified
373+
if head, errHead := c.transactor.HeaderByNumber(ensureContext(opts.Context), nil); err != nil {
374+
return nil, errHead
375+
} else if head.BaseFee != nil {
376+
rawTx, err = c.createDynamicTx(opts, contract, input, head)
377+
} else {
378+
// Chain is not London ready -> use legacy transaction
379+
rawTx, err = c.createLegacyTx(opts, contract, input)
333380
}
334-
rawTx = types.NewTx(baseTx)
335381
}
382+
if err != nil {
383+
return nil, err
384+
}
385+
// Sign the transaction and schedule it for execution
336386
if opts.Signer == nil {
337387
return nil, errors.New("no signer to authorize the transaction with")
338388
}

‎accounts/abi/bind/base_test.go‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,49 @@ import (
3131
"github.com/ethereum/go-ethereum/core/types"
3232
"github.com/ethereum/go-ethereum/crypto"
3333
"github.com/ethereum/go-ethereum/rlp"
34+
"github.com/stretchr/testify/assert"
3435
)
3536

37+
func mockSign(addr common.Address, tx *types.Transaction) (*types.Transaction, error) { return tx, nil }
38+
39+
type mockTransactor struct {
40+
baseFee *big.Int
41+
gasTipCap *big.Int
42+
gasPrice *big.Int
43+
suggestGasTipCapCalled bool
44+
suggestGasPriceCalled bool
45+
}
46+
47+
func (mt *mockTransactor) HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error) {
48+
return &types.Header{BaseFee: mt.baseFee}, nil
49+
}
50+
51+
func (mt *mockTransactor) PendingCodeAt(ctx context.Context, account common.Address) ([]byte, error) {
52+
return []byte{1}, nil
53+
}
54+
55+
func (mt *mockTransactor) PendingNonceAt(ctx context.Context, account common.Address) (uint64, error) {
56+
return 0, nil
57+
}
58+
59+
func (mt *mockTransactor) SuggestGasPrice(ctx context.Context) (*big.Int, error) {
60+
mt.suggestGasPriceCalled = true
61+
return mt.gasPrice, nil
62+
}
63+
64+
func (mt *mockTransactor) SuggestGasTipCap(ctx context.Context) (*big.Int, error) {
65+
mt.suggestGasTipCapCalled = true
66+
return mt.gasTipCap, nil
67+
}
68+
69+
func (mt *mockTransactor) EstimateGas(ctx context.Context, call ethereum.CallMsg) (gas uint64, err error) {
70+
return 0, nil
71+
}
72+
73+
func (mt *mockTransactor) SendTransaction(ctx context.Context, tx *types.Transaction) error {
74+
return nil
75+
}
76+
3677
type mockCaller struct {
3778
codeAtBlockNumber *big.Int
3879
callContractBlockNumber *big.Int
@@ -226,6 +267,51 @@ func TestUnpackIndexedBytesTyLogIntoMap(t *testing.T) {
226267
unpackAndCheck(t, bc, expectedReceivedMap, mockLog)
227268
}
228269

270+
func TestTransactGasFee(t *testing.T) {
271+
assert := assert.New(t)
272+
273+
// GasTipCap and GasFeeCap
274+
// When opts.GasTipCap and opts.GasFeeCap are nil
275+
mt := &mockTransactor{baseFee: big.NewInt(100), gasTipCap: big.NewInt(5)}
276+
bc := bind.NewBoundContract(common.Address{}, abi.ABI{}, nil, mt, nil)
277+
opts := &bind.TransactOpts{Signer: mockSign}
278+
tx, err := bc.Transact(opts, "")
279+
assert.Nil(err)
280+
assert.Equal(big.NewInt(5), tx.GasTipCap())
281+
assert.Equal(big.NewInt(205), tx.GasFeeCap())
282+
assert.Nil(opts.GasTipCap)
283+
assert.Nil(opts.GasFeeCap)
284+
assert.True(mt.suggestGasTipCapCalled)
285+
286+
// Second call to Transact should use latest suggested GasTipCap
287+
mt.gasTipCap = big.NewInt(6)
288+
mt.suggestGasTipCapCalled = false
289+
tx, err = bc.Transact(opts, "")
290+
assert.Nil(err)
291+
assert.Equal(big.NewInt(6), tx.GasTipCap())
292+
assert.Equal(big.NewInt(206), tx.GasFeeCap())
293+
assert.True(mt.suggestGasTipCapCalled)
294+
295+
// GasPrice
296+
// When opts.GasPrice is nil
297+
mt = &mockTransactor{gasPrice: big.NewInt(5)}
298+
bc = bind.NewBoundContract(common.Address{}, abi.ABI{}, nil, mt, nil)
299+
opts = &bind.TransactOpts{Signer: mockSign}
300+
tx, err = bc.Transact(opts, "")
301+
assert.Nil(err)
302+
assert.Equal(big.NewInt(5), tx.GasPrice())
303+
assert.Nil(opts.GasPrice)
304+
assert.True(mt.suggestGasPriceCalled)
305+
306+
// Second call to Transact should use latest suggested GasPrice
307+
mt.gasPrice = big.NewInt(6)
308+
mt.suggestGasPriceCalled = false
309+
tx, err = bc.Transact(opts, "")
310+
assert.Nil(err)
311+
assert.Equal(big.NewInt(6), tx.GasPrice())
312+
assert.True(mt.suggestGasPriceCalled)
313+
}
314+
229315
func unpackAndCheck(t *testing.T, bc *bind.BoundContract, expected map[string]interface{}, mockLog types.Log) {
230316
received := make(map[string]interface{})
231317
if err := bc.UnpackLogIntoMap(received, "received", mockLog); err != nil {

0 commit comments

Comments
 (0)