feat(fortuna): configurable escalation policy for transactions. #2244
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-Authored-By: Jayant Krishnamurthy <[email protected]>
…event Co-Authored-By: Jayant Krishnamurthy <[email protected]>
Co-Authored-By: Jayant Krishnamurthy <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
jayantk
reviewed
Jan 12, 2025
apps/fortuna/src/keeper.rs
Outdated
| let gas_estimate = gas_estimate.min((gas_limit * DEFAULT_GAS_ESTIMATE_MULTIPLIER_PCT) / 100); | ||
| // Apply the configurable cap from backoff_gas_multiplier_cap_pct | ||
| let max_gas_estimate = gas_limit.saturating_mul(chain_config.backoff_gas_multiplier_cap_pct.into()) / 100; | ||
| let gas_estimate = gas_estimate.min(max_gas_estimate); |
There was a problem hiding this comment.
the correct change is to delete this check and not replace it with another one -- this is a duplicate check now
Co-Authored-By: Jayant Krishnamurthy <[email protected]>
jayantk
changed the title
jayantk
reviewed
Jan 13, 2025
| # Multiply successive callback transaction's fees by 10% until the cap is reached. | ||
| # All numbers are expressed as percentages where 100 = no change. | ||
| # (See also priority_fee_multiplier_pct above to generically adjust the priority fee estimates for the chain -- | ||
| # adjusting that parameter will influence the fee of the first transaction, in addition to other things) |
There was a problem hiding this comment.
note there isn't an initial value here. Instead you adjust the priority_fee_multiplier_pct above. The reason for that is because the other parameter affects all gas estimates for the chain, including the one that is used to determine the keeper's fee
jayantk
reviewed
Jan 13, 2025
| // While we could scale up this value based on the padding, that ends up overcharging users as most transactions cost nowhere | ||
| // near the maximum gas limit. | ||
| // In the unlikely event that the keeper fees aren't sufficient, the solution to this is to configure the target | ||
| // fee percentage to be higher on that specific chain. |
There was a problem hiding this comment.
reverted the change in the previous PR as it was making the fees aggressively high
jayantk
reviewed
Jan 13, 2025
| // Pad the gas estimate after checking it against the simulation gas limit, ensuring that | ||
| // the padded gas estimate doesn't exceed the maximum amount of gas we are willing to use. | ||
| let gas_estimate = gas_estimate.saturating_mul(gas_estimate_multiplier_pct.into()) / 100; | ||
| let gas_estimate = gas_estimate.min((gas_limit * DEFAULT_GAS_ESTIMATE_MULTIPLIER_PCT) / 100); |
There was a problem hiding this comment.
removing the cap on the gas estimate here as it now lives in the escalation policy.
m30m
approved these changes
Jan 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(Jayant: i stole this from devin and did a bunch of work on top, so ignore devin)
This PR adds a configurable escalation policy to fortuna that ramps up both the gas and priority fees on each successive retry. I moved the various escalation control parameters into a struct for tidyness.
I also made a couple of minor changes to other logic that are called out in comments. (some of this is subtle)
Link to Devin run: https://app.devin.ai/sessions/c37cdcd255e449cc97a5423c7b0beaf9