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.

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

accounts/abi/bind: Do not overwrite TransactOpts in transact #23663

Closed

Conversation

yondonfu
Copy link
Contributor

Motivation

At the moment, the BoundContract.transact() will populate the GasTipCap (i.e. maxPriorityFee) and GasFeeCap (i.e. maxFeePerGas) of a transaction if the TransactOpts passed to the method do not specify a GasTipCap/GasFeeCap by setting the GasTipCap field and GasFeeCap field on the TransactOpts. Since the TransactOpts parameter is a pointer, if the same pointer is passed to another call to BoundContract.transact(), the GasTipCap and GasFeeCap fields will already be populated which means fixed values will be used for these fields instead of values based on the latest suggested GasTipCap and the latest base fee.

The above behavior can be a problem when a contract binding session is used. The first transaction sent using the session will set the GasTipCap and GasFeeCap fields of the TransactOpts stored in the session. Then, subsequent transactions would end up using the same fixed GasTipCap and GasFeeCap.

If EIP-1559 is not enabled and the GasPrice field in TransactOpts is used then the above applies for the GasPrice field as well.

Solution

This PR updates BoundContract.transact() to not set the GasTipCap/GasFeeCap/GasPrice fields on the TransactOpts parameter. The method will only read fields from TransactOpts and will not write fields.

Additional Info

This was not a problem prior to the EIP-1559 related changes to BoundContract at commit 38ea7f2 because the transact() method did not set the GasPrice field on the TransactOpts.

@fjl
Copy link
Contributor

fjl commented Oct 13, 2021

Thanks a lot for this PR! We have enhanced the changes a bit and merged #23719 instead.

@fjl fjl closed this Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants