Skip to content

accounts/abi/bind/v2: modify the default contract deployer to block until the tx is accepted/rejected from the pool #32169

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jwasinger
Copy link
Contributor

@jwasinger jwasinger commented Jul 8, 2025

This aims to fix some flaky tests in this package, and also fixes a valid issue that could affect users.

If the deploy invocation is part of a set of deployments and the nonce is nil in the TransactOpts, the tx nonce will be automatically determined by looking at the pending nonce in the txpool.

If there are subsequent dependent deployment transactions, we want to make sure that prior transactions had a chance to be accepted into the pool so that the nonces on all the transactions are set correctly.

TODO: Also, in the case of a multi-contract deployment where TransactOpts has non-nil nonce, we should not determine the txs' nonces based on the pending from the pool. Instead, we should start incrementing from whatever was provided in the TransactOpts.

…transaction is accepted/rejected from the pool.
@@ -103,6 +103,9 @@ type ContractTransactor interface {

// PendingNonceAt retrieves the current pending nonce associated with an account.
PendingNonceAt(ctx context.Context, account common.Address) (uint64, error)

// TransactionByHash retrieves the transaction corresponding to the given hash.
TransactionByHash(ctx context.Context, txHash common.Hash) (tx *types.Transaction, isPending bool, err error)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's IMO a bit unfortunate that this method is on the ContractTransactor and not the DeployBackend interface.

However, the method signature of DefaultDeployer takes a ContractBackend. In order to not break compatibility, and make sure that this fix can apply to the default deployer, I decided to put TransactionsByHash here.

@jwasinger
Copy link
Contributor Author

hmmm. still seeing failures:

1m5s: 6558 runs so far, 0 failures, 12 active

/var/folders/fz/k1s_1m8n0md3v58rxwzdsb2h0000gn/T/go-stress-20250708T204733-1186997902
--- FAIL: TestDeploymentLibraries (0.02s)
    lib_test.go:85: err: replacement transaction underpriced

Perhaps the fix here isn't sufficient to address the flaky tests.

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.

1 participant