-
Notifications
You must be signed in to change notification settings - Fork 201
feat(bot-oo): detect nonce backlog and attempt self-tx cancellation #4915
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,132 @@ | ||
| import { delay, waitForLogger, GasEstimator } from "@uma/financial-templates-lib"; | ||
| import { BotModes, initMonitoringParams, Logger, startupLogLevel } from "./common"; | ||
| import { BigNumber } from "ethers"; | ||
| import { BotModes, MonitoringParams, initMonitoringParams, Logger, startupLogLevel } from "./common"; | ||
| import { settleRequests } from "./SettleRequests"; | ||
|
|
||
| const logger = Logger; | ||
| const DEFAULT_REPLACEMENT_BUMP_NUMERATOR = 12; | ||
| const DEFAULT_REPLACEMENT_BUMP_DENOMINATOR = 10; | ||
| const DEFAULT_REPLACEMENT_ATTEMPTS = 3; | ||
|
|
||
| type NonceBacklogConfig = { | ||
| replacementBumpNumerator: number; | ||
| replacementBumpDenominator: number; | ||
| replacementAttempts: number; | ||
| }; | ||
|
|
||
| const parsePositiveInt = (value: string | undefined, defaultValue: number): number => { | ||
| const parsed = Number(value); | ||
| return Number.isFinite(parsed) && parsed > 0 ? Math.floor(parsed) : defaultValue; | ||
| }; | ||
|
|
||
| const getNonceBacklogConfig = (env: NodeJS.ProcessEnv): NonceBacklogConfig => ({ | ||
| replacementBumpNumerator: parsePositiveInt(env.NONCE_REPLACEMENT_BUMP_NUMERATOR, DEFAULT_REPLACEMENT_BUMP_NUMERATOR), | ||
| replacementBumpDenominator: parsePositiveInt( | ||
|
Comment on lines
+23
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no check that resulting numerator is larger than denominator that one would normally expect with fee bumps. Also, do we really need such high precision here? It might be simpler to have single integer percentage points bump (e.g. value of "20" would bump the fee up by 20%) |
||
| env.NONCE_REPLACEMENT_BUMP_DENOMINATOR, | ||
| DEFAULT_REPLACEMENT_BUMP_DENOMINATOR | ||
| ), | ||
| replacementAttempts: parsePositiveInt(env.NONCE_REPLACEMENT_ATTEMPTS, DEFAULT_REPLACEMENT_ATTEMPTS), | ||
| }); | ||
|
|
||
| function bumpFeeData( | ||
| feeData: ReturnType<GasEstimator["getCurrentFastPriceEthers"]>, | ||
| bumps: number, | ||
| config: NonceBacklogConfig | ||
| ): ReturnType<GasEstimator["getCurrentFastPriceEthers"]> { | ||
| if (bumps === 0) return feeData; | ||
|
|
||
| const bumpValue = (value: BigNumber) => { | ||
| let bumped = value; | ||
| for (let i = 0; i < bumps; i++) { | ||
| bumped = bumped.mul(config.replacementBumpNumerator).div(config.replacementBumpDenominator); | ||
| } | ||
| return bumped; | ||
| }; | ||
|
|
||
| if ("gasPrice" in feeData) { | ||
| return { gasPrice: bumpValue(feeData.gasPrice) }; | ||
| } | ||
|
|
||
| return { | ||
| maxFeePerGas: bumpValue(feeData.maxFeePerGas), | ||
| maxPriorityFeePerGas: bumpValue(feeData.maxPriorityFeePerGas), | ||
| }; | ||
| } | ||
|
|
||
| async function handleNonceBacklog( | ||
| params: MonitoringParams, | ||
| gasEstimator: GasEstimator, | ||
| config: NonceBacklogConfig | ||
| ): Promise<boolean> { | ||
| const botAddress = await params.signer.getAddress(); | ||
| const [latestNonce, pendingNonce] = await Promise.all([ | ||
| params.provider.getTransactionCount(botAddress, "latest"), | ||
| params.provider.getTransactionCount(botAddress, "pending"), | ||
| ]); | ||
|
|
||
| if (pendingNonce <= latestNonce) return false; | ||
|
|
||
| logger.warn({ | ||
| at: "OracleBot", | ||
| message: "Nonce backlog detected, skipping settlements for this run", | ||
| botAddress, | ||
| latestNonce, | ||
| pendingNonce, | ||
| }); | ||
|
|
||
| await gasEstimator.update(); | ||
| const baseFeeData = gasEstimator.getCurrentFastPriceEthers(); | ||
|
|
||
| for (let attempt = 1; attempt <= config.replacementAttempts; attempt++) { | ||
| const feeData = bumpFeeData(baseFeeData, attempt - 1, config); | ||
| try { | ||
| const tx = await params.signer.sendTransaction({ | ||
| to: botAddress, | ||
| value: 0, | ||
| nonce: latestNonce, | ||
| gasLimit: 21_000, | ||
| ...feeData, | ||
| }); | ||
|
|
||
| logger.info({ | ||
| at: "OracleBot", | ||
| message: "Submitted nonce backlog cancellation transaction", | ||
| tx: tx.hash, | ||
| nonce: latestNonce, | ||
| attempt, | ||
| feeData, | ||
| }); | ||
|
|
||
| await tx.wait(1); | ||
|
|
||
| logger.info({ | ||
| at: "OracleBot", | ||
| message: "Nonce backlog cancellation transaction mined", | ||
| tx: tx.hash, | ||
| nonce: latestNonce, | ||
| attempt, | ||
| }); | ||
| return true; | ||
| } catch (error) { | ||
| logger.warn({ | ||
| at: "OracleBot", | ||
| message: "Nonce backlog cancellation transaction failed", | ||
| attempt, | ||
| nonce: latestNonce, | ||
| error, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| logger.error({ | ||
| at: "OracleBot", | ||
| message: "Failed to clear nonce backlog, exiting early", | ||
| nonce: latestNonce, | ||
| pendingNonce, | ||
| }); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| async function main() { | ||
| const params = await initMonitoringParams(process.env); | ||
|
|
@@ -16,12 +140,20 @@ async function main() { | |
| }); | ||
|
|
||
| const gasEstimator = new GasEstimator(logger, undefined, params.chainId, params.provider); | ||
| const nonceBacklogConfig = getNonceBacklogConfig(process.env); | ||
|
|
||
| const cmds = { | ||
| settleRequestsEnabled: settleRequests, | ||
| }; | ||
|
|
||
| for (;;) { | ||
| const backlogDetected = await handleNonceBacklog(params, gasEstimator, nonceBacklogConfig); | ||
| if (backlogDetected) { | ||
| await delay(5); // Let any in-flight logs flush before exiting. | ||
| await waitForLogger(logger); | ||
| break; | ||
| } | ||
|
|
||
| await gasEstimator.update(); | ||
|
|
||
| const runCmds = Object.entries(cmds) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be more predictable if we only allow positive integers in the value (unless undefined) and throw if the value does not parse correctly. I would rather expect to use default value only when value was not passed in env