Skip to content

account/abi/bind/v2: fix TestDeploymentWithOverrides #32212

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

Merged

Conversation

stevemilk
Copy link
Contributor

@stevemilk stevemilk commented Jul 14, 2025

The root cause of the flaky test was a nonce conflict caused by async contract deployments.

This solution defines a custom deployer with automatic nonce management.

return addr, tx, nil
}
}
}
}
Copy link
Member

@rjl493456442 rjl493456442 Jul 15, 2025

Choose a reason for hiding this comment

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

We can probably define another customized deployer to solve the nonce conflict issue?

// DeployerWithNonceAssignment is basically identical to DefaultDeployer,
// but it additionally tracks the nonce to enable automatic assignment.
func DeployerWithNonceAssignment(opts *TransactOpts, backend ContractBackend) DeployFn {
	var pendingNonce int64
	if opts.Nonce != nil {
		pendingNonce = opts.Nonce.Int64()
	}
	return func(input []byte, deployer []byte) (common.Address, *types.Transaction, error) {
		if pendingNonce != 0 {
			opts.Nonce = big.NewInt(pendingNonce)
		}
		addr, tx, err := DeployContract(opts, deployer, backend, input)
		if err != nil {
			return common.Address{}, nil, err
		}
		pendingNonce = int64(tx.Nonce() + 1)
		return addr, tx, nil
	}
}

cc @jwasinger

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed — I’ve made the update, and this makes more sense, thx!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I was going to comment that we shouldn't change the semantics of DeployContract. Introducing a new deployer here is the right way to go IMO

@rjl493456442 rjl493456442 added this to the 1.16.2 milestone Jul 16, 2025
@rjl493456442 rjl493456442 changed the title account/abi/bind/v2: fix flaky test TestDeploymentWithOverrides account/abi/bind/v2: fix TestDeploymentWithOverrides Jul 16, 2025
@rjl493456442 rjl493456442 merged commit 66df1f2 into ethereum:master Jul 16, 2025
3 of 5 checks passed
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.

3 participants