feat(price-pusher): migrate EVM pusher to EIP-1559 gas with legacy fallback - #3785
feat(price-pusher): migrate EVM pusher to EIP-1559 gas with legacy fallback#3785aditya520 wants to merge 1 commit into
Conversation
…llback Default to EIP-1559 (type-2) transactions using maxFeePerGas and maxPriorityFeePerGas from viem's estimateFeesPerGas(). Operators can pass --legacy to use the old gasPrice model for chains without EIP-1559 support. Changes: - common.ts: Add GasParams discriminated union type and update PushAttempt - evm.ts: Refactor gas handling into helper methods (getGasParams, applyStaticOverride, escalateGas, capGas, gasToTxParams) that work with both EIP-1559 and legacy params - command.ts: Add --legacy boolean CLI flag, pass through to pusher - custom-gas-station.ts: Return structured CustomGasResult with both maxFeePerGas/maxPriorityFeePerGas in EIP-1559 mode Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a6c24f992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (a.type === "eip1559" && b.type === "eip1559") { | ||
| return a.maxFeePerGas > b.maxFeePerGas; |
There was a problem hiding this comment.
When a previous EIP-1559 push is still pending, replacement pricing has to keep the priority-fee cap bumped as well as the max-fee cap. In the case where the latest estimate has a higher maxFeePerGas but a lower maxPriorityFeePerGas than the prior attempt (for example base fee rises while the suggested tip drops), this check returns false and the code sends the replacement with the lower tip, so clients can reject it as underpriced and the nonce can remain stuck until estimates happen to rise again. Please treat either EIP-1559 component needing a bump as higher, or otherwise preserve/bump the previous priority fee when reusing the nonce.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 No test coverage for new EIP-1559 gas escalation logic
This PR introduces significant new functionality (EIP-1559 gas parameter handling with escalation, capping, static overrides, and type discrimination) but includes no tests. The apps/price_pusher directory has no test files at all. The escalation logic in particular (escalateGas, capGas, gasIsHigherThan, applyStaticOverride) has multiple code paths and edge cases that would benefit from unit tests — especially the interaction between these functions when a pending transaction exists. Per REVIEW.md: "New functionality should have tests."
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (this.lastPushAttempt !== undefined) { | ||
| if (this.lastPushAttempt.nonce <= lastExecutedNonce) { | ||
| this.lastPushAttempt = undefined; | ||
| } else { | ||
| gasPriceToOverride = | ||
| this.lastPushAttempt.gasPrice * this.overrideGasPriceMultiplier; | ||
| const escalated = this.escalateGas( | ||
| this.lastPushAttempt.gas, | ||
| this.overrideGasPriceMultiplier, | ||
| ); | ||
| if (this.gasIsHigherThan(escalated, gas)) { | ||
| gas = this.capGas(escalated, gas); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 EIP-1559 escalation logic ignores maxPriorityFeePerGas, causing stuck replacement transactions
gasIsHigherThan at line 227 only compares maxFeePerGas when deciding whether to use the escalated gas or the fresh gas. When network conditions change such that fresh maxFeePerGas is higher (base fee increased) but fresh maxPriorityFeePerGas is lower (priority fee dropped), the code uses the fresh gas with inadequate priority fee. This fails to replace the pending transaction (nodes require BOTH fields bumped by ~10%). Critically, lastPushAttempt at line 326-329 is updated with this low-priority-fee gas BEFORE the try block, so subsequent escalations start from the low base. The cap at apps/price_pusher/src/evm/evm.ts:212-215 limits priority fee to fresh * cap (e.g., 2 * 5 = 10), which may never reach the replacement threshold (e.g., 10 * 1.1 = 11 needed). The pusher gets permanently stuck until the mempool expires the pending transaction.
Scenario trace showing stuck state
Push 1: sends tx with maxPriorityFeePerGas=10, lands in mempool.
Push 2: fresh gas has maxFeePerGas=60 > escalated 55, so fresh is used with maxPriorityFeePerGas=2. Fails: "replacement underpriced" (need >=11). lastPushAttempt stores priorityFee=2.
Push 3+: escalation starts from 2, cap is 2*5=10, but 10 < 11 needed. Permanently stuck.
Prompt for agents
The problem is in the escalation logic at lines 299-310 of apps/price_pusher/src/evm/evm.ts. When gasIsHigherThan(escalated, gas) returns false because fresh maxFeePerGas is higher, the code uses fresh gas without ensuring maxPriorityFeePerGas is sufficient for replacement.
The fix should ensure that when a pending transaction exists (lastPushAttempt not cleared), the maxPriorityFeePerGas is always at least as high as the escalated value, even when fresh maxFeePerGas is higher. One approach:
1. After the existing if/else block (lines 299-310), add logic for EIP-1559 mode that takes the max of each field independently:
- maxFeePerGas = max(fresh.maxFeePerGas, escalated.maxFeePerGas)
- maxPriorityFeePerGas = max(fresh.maxPriorityFeePerGas, escalated.maxPriorityFeePerGas)
Then apply the cap to the result.
2. Alternatively, modify gasIsHigherThan to return true if EITHER field of the escalated params exceeds fresh, then in capGas take the max of each field independently before applying the cap.
The key insight is that for EIP-1559 replacement transactions, both maxFeePerGas AND maxPriorityFeePerGas need to be bumped. The two values should be escalated independently rather than choosing one set of params wholesale based on a single comparison.
Was this helpful? React with 👍 or 👎 to provide feedback.
| legacy: { | ||
| default: false, | ||
| description: | ||
| "Use legacy (type-0) transactions with gasPrice instead of EIP-1559 (type-2). " + | ||
| "Enable this for chains that don't support EIP-1559.", | ||
| required: false, | ||
| type: "boolean", | ||
| } as Options, |
There was a problem hiding this comment.
🚩 Default behavior changes from legacy gasPrice to EIP-1559 without explicit opt-in
The legacy CLI flag defaults to false (apps/price_pusher/src/evm/command.ts:53), meaning existing deployments that upgrade will silently switch from using gasPrice (old behavior at the base branch) to using maxFeePerGas/maxPriorityFeePerGas. While most modern EVM chains support EIP-1559, chains that don't (and previously worked with the old code's gasPrice field) will break without adding --legacy. The old code always used the gasPrice field for all chains. This is a breaking change in default behavior for the package.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Migrates the EVM price pusher from deprecated legacy gasPrice transactions to EIP-1559 (type-2) transactions, with a --legacy CLI flag for chains that don't support EIP-1559.
Changes
GasParamsdiscriminated union type (eip1559|legacy) and updatedPushAttemptto use it.getGasParams,applyStaticOverride,escalateGas,capGas,gasToTxParams) that work with both EIP-1559 and legacy params. Default path usesestimateFeesPerGas().--legacyboolean CLI flag (defaultfalse), passed through toEvmPricePusher.CustomGasResultwithmaxFeePerGas/maxPriorityFeePerGasin EIP-1559 mode, orgasPricein legacy mode. Polygon gas station now returns both fee fields.Testing
pnpm turbo buildpasses)