Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

[TT-1490] add gas bumping #40

Merged
merged 10 commits into from
Aug 23, 2024
Merged

[TT-1490] add gas bumping #40

merged 10 commits into from
Aug 23, 2024

Conversation

Tofel
Copy link
Contributor

@Tofel Tofel commented Aug 20, 2024

this PR adds gas bumping for transactions that are slow to confirm. that means that they need to be successfully submitted and cannot be reverted. by default bumping is disabled (retries = 0). there's also an option to specify max gas price in wei.

gas bumping can be used, when deploying contracts via DeployContract() function and inside Decode() function, when waiting for tx to be mined.

following tests were added:

  • (legacy tx) contract deployment: successful bumping with standard function
  • (legacy tx) contract deployment: bumping disabled
  • (legacy tx) contract deployment: failed bumping
  • (legacy tx) contract deployment: successful bumping with custom function
  • (dynamic tx) contract deployment: successful bumping with standard function
  • (dynamic tx) contract deployment: bumping of tx from non-root key
  • (dynamic tx) contract deployment: bumping of tx from unknown/invalid key
  • (legacy tx) contract interaction: successful bumping with standard function
  • (legacy tx) contract interaction: bumping disabled
  • (legacy tx) contract interaction: failed bumping

@Tofel Tofel force-pushed the tt_1490_gas_bumping branch from 84f0490 to 19a46a1 Compare August 21, 2024 08:02
@Tofel Tofel force-pushed the tt_1490_gas_bumping branch from 40eda12 to 76afc61 Compare August 21, 2024 11:27
@Tofel Tofel requested review from skudasov and kalverra August 21, 2024 17:55
README.md Outdated
- for Blob transactions (EIP-4844) it's the sum of gas fee cap and tip cap and max fee per blob
- for AccessList transactions (EIP-2930) it's just the gas price

Please note that Blob and AccessList
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we end the sentence? =)

README.md Outdated
}
```

Currently, the gas bumping mechanism is available for:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove these lines, we've already described them before, or shorten it

retry.DelayType(retry.FixedDelay),
// unless attempts is at least 1 retry.Do won't execute at all
retry.Attempts(func() uint {
if m.Cfg.GasBumpRetries() == 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it strictly 1 attempt still or 2 attempts, 1 initial and 1 after?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's the thing, if attempts is 0 no retry will ever be done


Don't worry if while bumping logic executes previous transaction gets mined. In that case sending replacement transaction with higher gas will fail (because it is using the same nonce as original transaction) and we will retry waiting for the mining of the original transaction.

**Gas bumping is only applied for submitted transaction. If transaction was rejected by the node (e.g. because of too low base fee) we will not bump the gas price nor try to submit it, because original transaction submission happens outside of Seth.**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sadly enough, that's true, haven't found any other possible workarounds?
transact in generated bindings returns nil if it fails to send

	if err := c.transactor.SendTransaction(ensureContext(opts.Context), signedTx); err != nil {
		return nil, err
	}
	return signedTx, nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup so inside Decode() we don't have the transaction so we cannot resubmit it, the only solution I can think of is what I described to you yesterday:

  • implement Backend
  • proxy SendTransaction to original one
  • if err, retry inside our implementation of SendTransaction

retry.go Outdated
// +30%
return func(gasPrice *big.Int) *big.Int {
gasPriceFloat, _ := gasPrice.Float64()
newGasPriceFloat := big.NewFloat(0.0).Mul(big.NewFloat(gasPriceFloat), big.NewFloat(1.5))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you mean 1.3 for 30%?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch!

@skudasov skudasov self-requested a review August 23, 2024 07:30
@Tofel Tofel merged commit 7a35586 into master Aug 23, 2024
12 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants