feat(swapper): state-override gas estimation, replace tenderly - #12515
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change replaces Tenderly-based EVM fee simulation with local state-override estimation. Swapper flows now pass sell asset, amount, and spender data. Tenderly configuration and utilities are removed. Portals adds unvalidated-order fallback handling. ChangesEVM fee estimation migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SwapperStepData
participant FeeEstimator
participant StateOverride
participant EvmRpc
SwapperStepData->>FeeEstimator: Provide transaction, sell asset, amount, and spender
FeeEstimator->>StateOverride: Build minimal balance and allowance override
StateOverride->>EvmRpc: Read balances and probe token storage slots
StateOverride-->>FeeEstimator: Return state override or undefined
FeeEstimator->>EvmRpc: Estimate gas with override
EvmRpc-->>FeeEstimator: Return gas estimate
FeeEstimator-->>SwapperStepData: Return buffered network fee data
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/swapper/src/utils/evm/index.ts (1)
5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
withTimeoutis now public API under a generic name.The barrel re-exports
withTimeoutfromstateOverride. The name does not describe the EVM state-override context, and it is broad enough to collide with an existing timeout helper elsewhere in the monorepo. Either keep it module-internal, or rename it to reflect scope.#!/bin/bash # Check for existing withTimeout helpers and for out-of-module consumers. rg -nP --type=ts -C2 '\bwithTimeout\b'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/utils/evm/index.ts` around lines 5 - 6, Update the state-override module and its barrel export so the generic withTimeout helper is not exposed as public API: either keep it internal by removing its export, or rename it to an EVM state-override-specific symbol and update all references and exports accordingly. Use repository-wide search to ensure no consumers remain on the ambiguous name.packages/swapper/src/utils/evm/stateOverride.ts (3)
283-289: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueValidate
databefore asserting it toHex.
data as Hexon line 286 accepts any string. Callers pass provider-supplied calldata. If the value is not0x-prefixed hex,estimateGasfails with an opaque Viem error instead of a clear message. UseisHexfromviemas a guard, or typedataasHexinEstimateGasWithStateOverrideArgsand convert at the call sites.As per coding guidelines: "NEVER use type assertions without proper validation in TypeScript".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/utils/evm/stateOverride.ts` around lines 283 - 289, Validate the provider-supplied data before the `estimateGas` call in the surrounding state-override flow, rather than unconditionally asserting `data as Hex`. Use Viem’s `isHex` guard (or make `EstimateGasWithStateOverrideArgs.data` a validated `Hex` at all call sites) and reject invalid non-0x-prefixed calldata with a clear error before invoking `client.estimateGas`.Source: Coding guidelines
174-187: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the explicit return type and guard the
sellAmountCryptoBaseUnitconversion.Two points in this block:
needsNativeBalanceon line 183 has no explicit return type. Add: Promise<boolean>.BigInt(sellAmountCryptoBaseUnit)on line 176 throwsSyntaxErrorfor any non-integer string, for example a decimal amount from a provider response. Validate the input and return an explicit error instead.As per coding guidelines: "ALWAYS use explicit types for function parameters and return values in TypeScript" and "ALWAYS validate inputs before processing with clear validation error messages and use early returns for validation failures".
♻️ Proposed change
const valueBigInt = BigInt(value || '0') - const sellAmount = BigInt(sellAmountCryptoBaseUnit) + if (!/^\d+$/.test(sellAmountCryptoBaseUnit)) { + throw new Error( + `getMinimalStateOverride: sellAmountCryptoBaseUnit must be an integer base unit string, got '${sellAmountCryptoBaseUnit}'`, + ) + } + const sellAmount = BigInt(sellAmountCryptoBaseUnit)- const needsNativeBalance = async () => { + const needsNativeBalance = async (): Promise<boolean> => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/utils/evm/stateOverride.ts` around lines 174 - 187, Update the state-override setup around sellAmountCryptoBaseUnit to validate that the input is a valid integer string before converting it with BigInt, returning a clear validation error through the surrounding function’s existing error path when invalid. Add the explicit : Promise<boolean> return type to needsNativeBalance while preserving its current balance-check behavior.Source: Coding guidelines
118-153: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache discovery failures as well as successes.
discoveredSlotsstores only found slots. When discovery fails, the next estimate for the same chain, token, and kind repeats the guess probe plus all remaining candidate probes. On an RPC that does not supportstateOverride, every probe returnsfalse, so each quote pays 8 RPC calls and then throws. The PR notes Scroll has no override support, so that chain hits this path for every token quote.Consider a negative cache entry with a short TTL, and separately detect the "state override unsupported" case so the thrown error text is actionable.
♻️ Sketch
-const discoveredSlots = new Map<string, number>() +// `null` records a failed discovery so repeat estimates skip the probe fan out +const discoveredSlots = new Map<string, number | null>()const cached = discoveredSlots.get(cacheKey) - if (cached !== undefined) return cached + if (cached === null) throw new Error(`Unable to locate ${kind} storage slot for token ${tokenAddress}`) + if (cached !== undefined) return cachedif (found === undefined) { + discoveredSlots.set(cacheKey, null) throw new Error(`Unable to locate ${kind} storage slot for token ${tokenAddress}`) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/utils/evm/stateOverride.ts` around lines 118 - 153, Update discoverSlot and discoveredSlots to cache failed discoveries with a short expiration, preventing repeated guess and candidate probes for the same chain, token, and kind. Separately identify when probes indicate stateOverride is unsupported, and throw an actionable error describing that limitation; preserve normal slot-not-found errors for other failures and avoid caching expired negative entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/swapper/src/swappers/BebopSwapper/endpoints.ts`:
- Around line 28-37: Add and use isBebopSolanaTradeQuoteInput and
isBebopSolanaTradeRateInput in getTradeQuote and getTradeRate to validate both
input.chainId and sellAsset.chainId before route-specific casts. When either
guard detects a mismatch, return Err(InvalidInput); otherwise preserve the
existing Solana and EVM handler routing.
In
`@packages/swapper/src/swappers/NearIntentsSwapper/utils/getNearIntentsStepData.ts`:
- Around line 83-95: Update the rate estimation fallback in the EVM branch of
getNearIntentsStepData so the catch path returns undefined instead of '0'.
Preserve the existing estimate call and NearIntentsRateStepData construction
while matching the file’s established undefined sentinel for unavailable fee
estimates.
In
`@packages/swapper/src/swappers/PortalsSwapper/getPortalsTradeQuote/getPortalsTradeQuote.ts`:
- Around line 108-131: Restrict the validate:false retry in getPortalsTradeQuote
to errors explicitly identified as sender allowance or balance-state failures;
do not retry for other PortalsError values or network/axios failures. For all
non-matching failures, return the original error unchanged, preserving the
existing fallback error mapping only when the sender-state predicate matches.
In `@packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts`:
- Around line 60-66: Update the override argument construction in
getEvmNetworkFeeCryptoBaseUnit to pass spenderAddress through unchanged instead
of defaulting it to to. Preserve the existing caller opt-in behavior so
balance-only state overrides do not trigger allowance lookup or slot discovery.
- Around line 55-80: Update the state-override estimation block in
getEvmNetworkFeeCryptoBaseUnit so timeout, getMinimalStateOverride, and
estimateGasWithStateOverride rejections are caught and treated as an unavailable
override estimate, allowing execution to continue to the existing plain on-chain
gas estimation fallback. Preserve the current overriddenGasLimit success path
and only change the rejection handling; if rejection propagation is intentional
instead, revise the nearby fallback comment to match that behavior.
---
Nitpick comments:
In `@packages/swapper/src/utils/evm/index.ts`:
- Around line 5-6: Update the state-override module and its barrel export so the
generic withTimeout helper is not exposed as public API: either keep it internal
by removing its export, or rename it to an EVM state-override-specific symbol
and update all references and exports accordingly. Use repository-wide search to
ensure no consumers remain on the ambiguous name.
In `@packages/swapper/src/utils/evm/stateOverride.ts`:
- Around line 283-289: Validate the provider-supplied data before the
`estimateGas` call in the surrounding state-override flow, rather than
unconditionally asserting `data as Hex`. Use Viem’s `isHex` guard (or make
`EstimateGasWithStateOverrideArgs.data` a validated `Hex` at all call sites) and
reject invalid non-0x-prefixed calldata with a clear error before invoking
`client.estimateGas`.
- Around line 174-187: Update the state-override setup around
sellAmountCryptoBaseUnit to validate that the input is a valid integer string
before converting it with BigInt, returning a clear validation error through the
surrounding function’s existing error path when invalid. Add the explicit :
Promise<boolean> return type to needsNativeBalance while preserving its current
balance-check behavior.
- Around line 118-153: Update discoverSlot and discoveredSlots to cache failed
discoveries with a short expiration, preventing repeated guess and candidate
probes for the same chain, token, and kind. Separately identify when probes
indicate stateOverride is unsupported, and throw an actionable error describing
that limitation; preserve normal slot-not-found errors for other failures and
avoid caching expired negative entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c48ca597-58c2-4b0d-8505-cdf188e13a85
📒 Files selected for processing (28)
packages/public-api/.env.examplepackages/public-api/src/config.tspackages/public-api/src/env.tspackages/swapper/src/swappers/AcrossSwapper/utils/getAcrossStepData.tspackages/swapper/src/swappers/AcrossSwapper/utils/getAcrossTradeContext.tspackages/swapper/src/swappers/ArbitrumBridgeSwapper/utils/getArbitrumBridgeStepData.tspackages/swapper/src/swappers/BebopSwapper/endpoints.tspackages/swapper/src/swappers/BebopSwapper/utils/getBebopStepData.tspackages/swapper/src/swappers/BebopSwapper/utils/getBebopTradeContext.tspackages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayStepData.tspackages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.tspackages/swapper/src/swappers/ChainflipSwapper/utils/getChainflipStepData.tspackages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeStepData.tspackages/swapper/src/swappers/NearIntentsSwapper/utils/getNearIntentsStepData.tspackages/swapper/src/swappers/PortalsSwapper/getPortalsTradeQuote/getPortalsTradeQuote.tspackages/swapper/src/swappers/PortalsSwapper/getPortalsTradeRate/getPortalsTradeRate.tspackages/swapper/src/swappers/PortalsSwapper/utils/fetchPortalsTradeOrder.tspackages/swapper/src/swappers/PortalsSwapper/utils/getPortalsStepData.tspackages/swapper/src/swappers/RelaySwapper/utils/getRelayStepData.tspackages/swapper/src/types.tspackages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.tspackages/swapper/src/utils/evm/index.tspackages/swapper/src/utils/evm/stateOverride.tspackages/swapper/src/utils/evm/storageSlots.tspackages/swapper/src/utils/tenderly/index.tspackages/swapper/src/utils/tenderly/simulate.tspackages/swapper/src/utils/tenderly/types.tspackages/swapper/src/utils/thorchain/getThorStepData.ts
💤 Files with no reviewable changes (7)
- packages/public-api/.env.example
- packages/swapper/src/utils/tenderly/index.ts
- packages/public-api/src/config.ts
- packages/swapper/src/types.ts
- packages/swapper/src/utils/tenderly/simulate.ts
- packages/swapper/src/utils/tenderly/types.ts
- packages/public-api/src/env.ts
kaladinlight
force-pushed
the
feat/evm-state-override-gas-estimation
branch
5 times, most recently
from
August 3, 2026 18:24
d4d5f8f to
0afe910
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.test.ts (1)
73-73: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse valid spender fixtures in EVM tests.
spenderAddress: ''is not a valid EVM address. BecausegetMinimalStateOverrideis mocked to returnundefined, these tests do not validate spender propagation into the approval-aware estimation path. Use a valid spender fixture in the EVM cases at Lines 73, 89, 108, 136, and 157.The production fee helper forwards spenderAddress to getMinimalStateOverride in packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts, Lines 60-73.
Also applies to: 89-89, 108-108, 136-136, 157-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.test.ts` at line 73, Replace the empty spenderAddress fixtures in the EVM test cases of getButterSwapStepData with a valid EVM address, using the same fixture consistently at the identified cases. Ensure the tests exercise spender propagation through getMinimalStateOverride and the approval-aware estimation path.packages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.test.ts (1)
35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a type-safe mock pattern in both test files.
Both mocks widen the original module to
objectand rely on inferred callback types.
packages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.test.ts#L35-L37: use the stateOverride module type and explicit factory types.packages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.test.ts#L14-L16: apply the same typed mock pattern.As per coding guidelines, TypeScript parameters and return values require explicit types, and object shapes require explicit types.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.test.ts` around lines 35 - 37, Update the vi.mock factories in packages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.test.ts:35-37 and packages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.test.ts:14-16 to use the stateOverride module type instead of object, with explicit factory parameter and return types; preserve the existing getMinimalStateOverride mock behavior.Source: Coding guidelines
packages/swapper/src/swappers/ButterSwap/utils/getButterSwapTradeContext.ts (1)
142-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the
route.contract ?? ''computation.
allowanceContract(line 150) andspenderAddress(line 169) derive from the same expressionroute.contract ?? '', computed independently. If a future edit changes one without the other, the approval target and the gas-estimation spender diverge.getDebridgeTradeContext.tsavoids this by computing the value once and reusing it for both fields.♻️ Proposed fix to compute the address once
+ const allowanceContract = route.contract ?? '' + return Ok({ tradeCommon: { id: route.hash, rate, swapperName: SwapperName.ButterSwap, affiliateBps, isStreaming: false, slippageTolerancePercentageDecimal, }, stepCommon: { rate, buyAmountBeforeFeesCryptoBaseUnit: buyAmountAfterFeesCryptoBaseUnit, buyAmountAfterFeesCryptoBaseUnit, sellAmountIncludingProtocolFeesCryptoBaseUnit, source: SwapperName.ButterSwap, buyAsset, sellAsset, - allowanceContract: route.contract ?? '', + allowanceContract, estimatedExecutionTimeMs: route.timeEstimated * 1000, ... }, protocolFees: undefined, route, stepDataArgs: { route, feeAsset, sellAmountCryptoBaseUnit: sellAmountIncludingProtocolFeesCryptoBaseUnit, sellAsset, - spenderAddress: route.contract ?? '', + spenderAddress: allowanceContract, deps, }, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/swappers/ButterSwap/utils/getButterSwapTradeContext.ts` around lines 142 - 172, In getButterSwapTradeContext, compute route.contract ?? '' once in a local variable before constructing the trade context, then reuse that variable for both stepCommon.allowanceContract and stepDataArgs.spenderAddress so the approval target and spender remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.test.ts`:
- Around line 35-37: Update the vi.mock factories in
packages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.test.ts:35-37
and
packages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.test.ts:14-16
to use the stateOverride module type instead of object, with explicit factory
parameter and return types; preserve the existing getMinimalStateOverride mock
behavior.
In
`@packages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.test.ts`:
- Line 73: Replace the empty spenderAddress fixtures in the EVM test cases of
getButterSwapStepData with a valid EVM address, using the same fixture
consistently at the identified cases. Ensure the tests exercise spender
propagation through getMinimalStateOverride and the approval-aware estimation
path.
In `@packages/swapper/src/swappers/ButterSwap/utils/getButterSwapTradeContext.ts`:
- Around line 142-172: In getButterSwapTradeContext, compute route.contract ??
'' once in a local variable before constructing the trade context, then reuse
that variable for both stepCommon.allowanceContract and
stepDataArgs.spenderAddress so the approval target and spender remain
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a95f69a-7cc1-40a2-96e3-adb91140ca74
📒 Files selected for processing (38)
packages/public-api/.env.examplepackages/public-api/src/config.tspackages/public-api/src/env.tspackages/public-api/src/routes/rates/getRates.tspackages/swapper/src/swappers/AcrossSwapper/utils/getAcrossStepData.tspackages/swapper/src/swappers/AcrossSwapper/utils/getAcrossTradeContext.tspackages/swapper/src/swappers/ArbitrumBridgeSwapper/utils/getArbitrumBridgeStepData.tspackages/swapper/src/swappers/ArbitrumBridgeSwapper/utils/getArbitrumBridgeTradeContext.tspackages/swapper/src/swappers/BebopSwapper/utils/getBebopStepData.tspackages/swapper/src/swappers/BebopSwapper/utils/getBebopTradeContext.tspackages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayStepData.tspackages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayTradeContext.tspackages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.test.tspackages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.test.tspackages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.tspackages/swapper/src/swappers/ButterSwap/utils/getButterSwapTradeContext.tspackages/swapper/src/swappers/ChainflipSwapper/utils/getChainflipStepData.tspackages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeStepData.tspackages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeTradeContext.tspackages/swapper/src/swappers/NearIntentsSwapper/utils/getNearIntentsStepData.tspackages/swapper/src/swappers/PortalsSwapper/getPortalsTradeQuote/getPortalsTradeQuote.tspackages/swapper/src/swappers/PortalsSwapper/getPortalsTradeRate/getPortalsTradeRate.tspackages/swapper/src/swappers/PortalsSwapper/utils/fetchPortalsTradeOrder.tspackages/swapper/src/swappers/PortalsSwapper/utils/getPortalsStepData.tspackages/swapper/src/swappers/PortalsSwapper/utils/getPortalsTradeContext.tspackages/swapper/src/swappers/RelaySwapper/getTradeQuote/getTradeQuote.tspackages/swapper/src/swappers/RelaySwapper/getTradeRate/getTradeRate.tspackages/swapper/src/swappers/RelaySwapper/utils/getRelayStepData.tspackages/swapper/src/swappers/RelaySwapper/utils/getRelayTradeContext.tspackages/swapper/src/types.tspackages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.tspackages/swapper/src/utils/evm/index.tspackages/swapper/src/utils/evm/stateOverride.tspackages/swapper/src/utils/evm/storageSlots.tspackages/swapper/src/utils/tenderly/index.tspackages/swapper/src/utils/tenderly/simulate.tspackages/swapper/src/utils/tenderly/types.tspackages/swapper/src/utils/thorchain/getThorStepData.ts
💤 Files with no reviewable changes (7)
- packages/swapper/src/utils/tenderly/index.ts
- packages/swapper/src/utils/tenderly/simulate.ts
- packages/swapper/src/types.ts
- packages/public-api/src/config.ts
- packages/swapper/src/utils/tenderly/types.ts
- packages/public-api/.env.example
- packages/public-api/src/env.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/swapper/src/utils/evm/storageSlots.ts
- packages/swapper/src/swappers/AcrossSwapper/utils/getAcrossTradeContext.ts
- packages/swapper/src/swappers/ChainflipSwapper/utils/getChainflipStepData.ts
- packages/swapper/src/swappers/BebopSwapper/utils/getBebopStepData.ts
- packages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayStepData.ts
- packages/swapper/src/utils/thorchain/getThorStepData.ts
- packages/swapper/src/swappers/BebopSwapper/utils/getBebopTradeContext.ts
- packages/swapper/src/swappers/AcrossSwapper/utils/getAcrossStepData.ts
- packages/swapper/src/swappers/NearIntentsSwapper/utils/getNearIntentsStepData.ts
- packages/swapper/src/swappers/PortalsSwapper/getPortalsTradeQuote/getPortalsTradeQuote.ts
- packages/swapper/src/swappers/PortalsSwapper/getPortalsTradeRate/getPortalsTradeRate.ts
- packages/swapper/src/utils/evm/stateOverride.ts
- packages/swapper/src/utils/evm/index.ts
- packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts
- packages/swapper/src/swappers/PortalsSwapper/utils/fetchPortalsTradeOrder.ts
- packages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeStepData.ts
kaladinlight
force-pushed
the
feat/evm-state-override-gas-estimation
branch
from
August 3, 2026 19:40
1698645 to
ce3d879
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/swapper/src/swappers/ThorchainSwapper/getTradeQuote/getTradeQuote.test.ts (2)
40-45: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert that quote estimation calls
getMinimalStateOverride.Returning
undefinedprevents live chain access, but the test does not prove that Thorchain estimation invokes the helper. Add an assertion for the call and its sell-state and spender inputs. This also detects an incorrect mock path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/swappers/ThorchainSwapper/getTradeQuote/getTradeQuote.test.ts` around lines 40 - 45, Update the test around the mocked getMinimalStateOverride in getTradeQuote.test.ts to assert that quote estimation invokes it with the expected sell-state and spender arguments. Keep the undefined mock result so estimation remains isolated from live chain state, and ensure the assertion verifies the intended mock path.
42-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit types to the mock factory.
importOriginaland the async factory return type are inferred.importOriginal<object>()also hides the module shape. Define a module-shaped type and annotate the callback parameter and return value. Verify the Vitest mock-factory signature when adding these annotations.As per coding guidelines, TypeScript functions must use explicit parameter and return types, and object shapes must use interfaces or type aliases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/swappers/ThorchainSwapper/getTradeQuote/getTradeQuote.test.ts` around lines 42 - 45, Update the vi.mock factory for stateOverride to use an explicit module-shaped type alias or interface, annotate the importOriginal callback parameter with the appropriate Vitest type, and annotate the async factory return type. Replace importOriginal<object>() with the defined module shape while preserving the existing mocked getMinimalStateOverride behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/swapper/src/swappers/ThorchainSwapper/getTradeQuote/getTradeQuote.test.ts`:
- Around line 40-45: Update the test around the mocked getMinimalStateOverride
in getTradeQuote.test.ts to assert that quote estimation invokes it with the
expected sell-state and spender arguments. Keep the undefined mock result so
estimation remains isolated from live chain state, and ensure the assertion
verifies the intended mock path.
- Around line 42-45: Update the vi.mock factory for stateOverride to use an
explicit module-shaped type alias or interface, annotate the importOriginal
callback parameter with the appropriate Vitest type, and annotate the async
factory return type. Replace importOriginal<object>() with the defined module
shape while preserving the existing mocked getMinimalStateOverride behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9cd6875c-ac15-43cd-9342-288fafd33f6d
📒 Files selected for processing (6)
packages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayStepData.tspackages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.tspackages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeStepData.tspackages/swapper/src/swappers/PortalsSwapper/utils/getPortalsStepData.tspackages/swapper/src/swappers/ThorchainSwapper/getTradeQuote/getTradeQuote.test.tspackages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts
💤 Files with no reviewable changes (3)
- packages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayStepData.ts
- packages/swapper/src/swappers/PortalsSwapper/utils/getPortalsStepData.ts
- packages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeStepData.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts
- packages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.ts
Pre-approval token sells revert eth_estimateGas, so executable quotes could never carry a real gas limit before the user approved. The estimation seat now reads the seller's actual allowance/balance first and, only when insufficient, re-estimates under a minimal node-level stateOverride (allowance/balance slot patched) on our own RPCs. Storage slots are validated against the known-slot tables via an eth_call under the override, with candidate probing and per-token caching for unknown layouts. Rate paths that simulated through Tenderly (Relay, Portals, NearIntents) now use the same native estimation, which also removes the Monad special-casing Tenderly required. The swapper Tenderly module and its config are gone; the walletConnectToDapps integration is separate and untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Portals validates orders by simulating with the sender's live state, so an unapproved sender failed the quote outright. On validation failure the order is refetched without validation - the response carries identical calldata and the same minOutputAmount but no gas limit, which the state-override estimation supplies (buffered). Validated orders are unchanged: sim-refined output amount, expected-slippage errors, and Portals' padded gas limit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The estimation spender was sourced three different ways (context-threaded, re-derived, inline tx.to) plus an implicit tx.to default in the seat that caused needless allowance reads for no-approval swappers. Every context now threads spenderAddress (one name, replacing bebop's approvalTarget) from its allowanceContract derivation, and the seat does nothing implicit. Also fixes the thor longtail spender: swapIn pulls tokens through the aggregator token transfer proxy, not the aggregator itself, so the override patched the wrong allowance slot and longtail pre-approval estimation still reverted. Portals' rate arm had the same latent mismatch (target vs the context's cross-chain-aware derivation). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GetEvmTradeRateInput pins supportsEIP1559: false and swappers discriminate evm from solana inputs on its presence - the rates endpoint omitted it, so bebop routed evm pairs to its solana implementation and errored with "Bebop Solana quote not executable: undefined". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kaladinlight
force-pushed
the
feat/evm-state-override-gas-estimation
branch
from
August 3, 2026 19:57
ce3d879 to
231aac1
Compare
Estimated limits now carry a uniform 1.2 safety margin instead of the previous per-swapper split (butter/debridge/bob/portals buffered, the rest raw). eth_estimateGas returns the bare minimum that succeeds and quotes execute post-approval against moved state - an OOG revert burns the full limit while an oversized one refunds, and the displayed fee still prices the raw estimate, so the margin costs nothing. Provider-supplied limits remain priced and carried as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kaladinlight
force-pushed
the
feat/evm-state-override-gas-estimation
branch
from
August 3, 2026 20:01
231aac1 to
b06cd38
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/swapper/src/swappers/NearIntentsSwapper/utils/getNearIntentsStepData.ts (1)
83-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRate failure still returns
'0'instead of the file'sundefinedsentinel.Every other rate path in this file uses
networkFeeCryptoBaseUnit: undefinedto signal that no fee estimate is available. See lines 209, 230, 251, 268, and the outer catch on line 297. This EVM branch returns'0', which a consumer reads as a free transaction, not as a failed estimate.🐛 Proposed fix
try { return await estimate() } catch { - return '0' + return undefined }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/swappers/NearIntentsSwapper/utils/getNearIntentsStepData.ts` around lines 83 - 90, Update the rate branch in getNearIntentsStepData so a failed estimate() returns the file’s undefined sentinel instead of the string '0'. Preserve the successful estimate result and align this catch with the existing networkFeeCryptoBaseUnit: undefined paths.packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts (1)
55-79: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winOverride-estimation failures still bypass the on-chain fallback.
withTimeoutrejects on failure. Three reachable paths reject here:
- The internal 3s timeout fires.
getMinimalStateOverridethrows, for exampleUnable to locate ${kind} storage slot for token ...fromdiscoverSlot.estimateGasWithStateOverridethrows because the RPC rejectsstateOverride(the PR notes Scroll).Each rejection propagates out of
getEvmNetworkFeeCryptoBaseUnit, so lines 83-84 never run. The comment on lines 81-82 states that the plain on-chain estimation always sets a gas limit. Callers such asgetPortalsStepData,getBebopStepData,getArbitrumBridgeStepData, andgetThorStepDatamap the rejection tomakeNetworkFeeEstimationFailedErr, so the quote fails instead of falling back.🛠️ Proposed fix to preserve the fallback
return estimateGasWithStateOverride({ ...overrideArgs, to, data, stateOverride }) })(), - ) + // Override estimation is best effort - fall through to plain on-chain estimation + ).catch(() => undefined)If rejection is intentional for executable quotes, update the comment on lines 81-82 to describe that.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts` around lines 55 - 79, Handle rejections from the state-override estimation block in getEvmNetworkFeeCryptoBaseUnit so timeout, getMinimalStateOverride, or estimateGasWithStateOverride failures do not propagate. Catch the failure, continue to the existing plain on-chain gas estimation fallback, and preserve the overridden result path when it succeeds.
🧹 Nitpick comments (2)
packages/swapper/src/utils/test-data/cryptoMarketDataById.ts (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the inline market-data shape into a named type.
Define a named interface or type alias for
{ price: string }, then use it inRecord<AssetId, ...>.As per coding guidelines: “ALWAYS use explicit types for object shapes using interfaces or type aliases in TypeScript.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/utils/test-data/cryptoMarketDataById.ts` at line 18, Define a named interface or type alias for the market-data object shape containing price: string, then update marketDataByAssetIdUsd to use that named type as the Record value type instead of the inline object shape.Source: Coding guidelines
packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts (1)
58-73: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant timeout nesting doubles the worst-case budget.
estimateGasWithStateOverridealready wraps its work inwithTimeout(seepackages/swapper/src/utils/evm/stateOverride.tslines 267-288). The two timers are independent, so slot discovery can consume the outer 3s and the inner estimate can then start a fresh 3s. Rates run inside the UI's 10s bulk budget per the comment instateOverride.tsline 16. Consider callinggetMinimalStateOverrideandclient.estimateGasunder a single timeout, or drop the outerwithTimeouthere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts` around lines 58 - 73, Remove the outer withTimeout wrapper around the async block in getEvmNetworkFeeCryptoBaseUnit, since estimateGasWithStateOverride already enforces its own timeout. Preserve the existing overrideArgs construction, getMinimalStateOverride early return, and estimateGasWithStateOverride invocation without introducing a second timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/swapper/src/utils/evm/stateOverride.ts`:
- Around line 133-150: Update discoverSlot so failed slot discovery is cached
before throwing, including cases where every probeSlot result is false. Add or
reuse a negative-cache map keyed by cacheKey with a short expiry, check it
before probing, and record the failure in the found === undefined branch to
prevent repeated discovery attempts while allowing retries after expiration.
---
Duplicate comments:
In
`@packages/swapper/src/swappers/NearIntentsSwapper/utils/getNearIntentsStepData.ts`:
- Around line 83-90: Update the rate branch in getNearIntentsStepData so a
failed estimate() returns the file’s undefined sentinel instead of the string
'0'. Preserve the successful estimate result and align this catch with the
existing networkFeeCryptoBaseUnit: undefined paths.
In `@packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts`:
- Around line 55-79: Handle rejections from the state-override estimation block
in getEvmNetworkFeeCryptoBaseUnit so timeout, getMinimalStateOverride, or
estimateGasWithStateOverride failures do not propagate. Catch the failure,
continue to the existing plain on-chain gas estimation fallback, and preserve
the overridden result path when it succeeds.
---
Nitpick comments:
In `@packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts`:
- Around line 58-73: Remove the outer withTimeout wrapper around the async block
in getEvmNetworkFeeCryptoBaseUnit, since estimateGasWithStateOverride already
enforces its own timeout. Preserve the existing overrideArgs construction,
getMinimalStateOverride early return, and estimateGasWithStateOverride
invocation without introducing a second timeout.
In `@packages/swapper/src/utils/test-data/cryptoMarketDataById.ts`:
- Line 18: Define a named interface or type alias for the market-data object
shape containing price: string, then update marketDataByAssetIdUsd to use that
named type as the Record value type instead of the inline object shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5cdb06e4-cb7a-436b-a885-d62f4d34116f
📒 Files selected for processing (40)
packages/public-api/.env.examplepackages/public-api/src/config.tspackages/public-api/src/env.tspackages/public-api/src/routes/rates/getRates.tspackages/swapper/src/swappers/AcrossSwapper/utils/getAcrossStepData.tspackages/swapper/src/swappers/AcrossSwapper/utils/getAcrossTradeContext.tspackages/swapper/src/swappers/ArbitrumBridgeSwapper/utils/getArbitrumBridgeStepData.tspackages/swapper/src/swappers/ArbitrumBridgeSwapper/utils/getArbitrumBridgeTradeContext.tspackages/swapper/src/swappers/BebopSwapper/utils/getBebopStepData.tspackages/swapper/src/swappers/BebopSwapper/utils/getBebopTradeContext.tspackages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayStepData.tspackages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayTradeContext.tspackages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.test.tspackages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.test.tspackages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.tspackages/swapper/src/swappers/ButterSwap/utils/getButterSwapTradeContext.tspackages/swapper/src/swappers/ChainflipSwapper/utils/getChainflipStepData.tspackages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeStepData.tspackages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeTradeContext.tspackages/swapper/src/swappers/NearIntentsSwapper/utils/getNearIntentsStepData.tspackages/swapper/src/swappers/PortalsSwapper/getPortalsTradeQuote/getPortalsTradeQuote.tspackages/swapper/src/swappers/PortalsSwapper/getPortalsTradeRate/getPortalsTradeRate.tspackages/swapper/src/swappers/PortalsSwapper/utils/fetchPortalsTradeOrder.tspackages/swapper/src/swappers/PortalsSwapper/utils/getPortalsStepData.tspackages/swapper/src/swappers/PortalsSwapper/utils/getPortalsTradeContext.tspackages/swapper/src/swappers/RelaySwapper/getTradeQuote/getTradeQuote.tspackages/swapper/src/swappers/RelaySwapper/getTradeRate/getTradeRate.tspackages/swapper/src/swappers/RelaySwapper/utils/getRelayStepData.tspackages/swapper/src/swappers/RelaySwapper/utils/getRelayTradeContext.tspackages/swapper/src/swappers/ThorchainSwapper/getTradeQuote/getTradeQuote.test.tspackages/swapper/src/types.tspackages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.tspackages/swapper/src/utils/evm/index.tspackages/swapper/src/utils/evm/stateOverride.tspackages/swapper/src/utils/evm/storageSlots.tspackages/swapper/src/utils/tenderly/index.tspackages/swapper/src/utils/tenderly/simulate.tspackages/swapper/src/utils/tenderly/types.tspackages/swapper/src/utils/test-data/cryptoMarketDataById.tspackages/swapper/src/utils/thorchain/getThorStepData.ts
💤 Files with no reviewable changes (7)
- packages/swapper/src/types.ts
- packages/public-api/.env.example
- packages/swapper/src/utils/tenderly/types.ts
- packages/swapper/src/utils/tenderly/index.ts
- packages/public-api/src/config.ts
- packages/public-api/src/env.ts
- packages/swapper/src/utils/tenderly/simulate.ts
🚧 Files skipped from review as they are similar to previous changes (29)
- packages/swapper/src/swappers/RelaySwapper/getTradeRate/getTradeRate.ts
- packages/swapper/src/utils/evm/storageSlots.ts
- packages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeTradeContext.ts
- packages/swapper/src/swappers/RelaySwapper/utils/getRelayTradeContext.ts
- packages/swapper/src/swappers/AcrossSwapper/utils/getAcrossTradeContext.ts
- packages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayTradeContext.ts
- packages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.ts
- packages/swapper/src/swappers/RelaySwapper/getTradeQuote/getTradeQuote.ts
- packages/swapper/src/swappers/PortalsSwapper/getPortalsTradeQuote/getPortalsTradeQuote.ts
- packages/public-api/src/routes/rates/getRates.ts
- packages/swapper/src/utils/evm/index.ts
- packages/swapper/src/swappers/BebopSwapper/utils/getBebopStepData.ts
- packages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayStepData.ts
- packages/swapper/src/utils/thorchain/getThorStepData.ts
- packages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeStepData.ts
- packages/swapper/src/swappers/ThorchainSwapper/getTradeQuote/getTradeQuote.test.ts
- packages/swapper/src/swappers/ButterSwap/utils/getButterSwapTradeContext.ts
- packages/swapper/src/swappers/PortalsSwapper/getPortalsTradeRate/getPortalsTradeRate.ts
- packages/swapper/src/swappers/AcrossSwapper/utils/getAcrossStepData.ts
- packages/swapper/src/swappers/PortalsSwapper/utils/getPortalsTradeContext.ts
- packages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.test.ts
- packages/swapper/src/swappers/ArbitrumBridgeSwapper/utils/getArbitrumBridgeTradeContext.ts
- packages/swapper/src/swappers/BebopSwapper/utils/getBebopTradeContext.ts
- packages/swapper/src/swappers/PortalsSwapper/utils/getPortalsStepData.ts
- packages/swapper/src/swappers/ArbitrumBridgeSwapper/utils/getArbitrumBridgeStepData.ts
- packages/swapper/src/swappers/PortalsSwapper/utils/fetchPortalsTradeOrder.ts
- packages/swapper/src/swappers/RelaySwapper/utils/getRelayStepData.ts
- packages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.test.ts
- packages/swapper/src/swappers/ChainflipSwapper/utils/getChainflipStepData.ts
…gative cache - Override-path failures (timeout, undiscoverable slot, rpc without stateOverride support) now fall through to plain estimation instead of rejecting the quote outright - sufficient-state sellers survive transient read failures, insufficient ones fail identically just later - NearIntents evm rate failure returns undefined like the file's other namespaces, not '0' masquerading as a free transaction - Failed slot discoveries are cached with a 60s ttl so unknown layouts and unsupported rpcs don't pay the full candidate sweep on every rate refresh Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… uint96) - probe with a sub-uint96 sentinel so packed-storage tokens (UNI/COMP) read it back intact, and write the trade-sized value instead of maxUint256 to keep flag bits and packed neighbors clear - support the vyper mapping hash order (Curve ecosystem) - discover arbitrary layouts via eth_createAccessList preimage matching (supported on 23/32 EVM chains), with a widened dual-layout candidate sweep as fallback - discover balance and allowance slots in parallel Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- omit the access-list account so gas-charging nodes (plume-style) accept unfunded owners - node defaults to the zero address, which never affects the slots a read touches (fixes a 61s sweep observed on plume) - raw-slot fallback for namespaced storage (ERC-7201) - when no slot number is recoverable, sentinel-probe the touched slots directly and cache per owner - batch the candidate sweep (6 concurrent) - public rpcs on long-tail chains drop requests above ~14 concurrent, and an early hit now skips the remaining batches Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/swapper/src/utils/evm/stateOverride.ts (2)
40-45: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the discovery caches.
The three maps never evict entries.
discoveredSlotsandfailedSlotDiscoveriesare bounded by chain and token count, butrawDiscoveredSlotsembeds the owner and the spender in its key, so its key space grows with every distinct trader.failedSlotDiscoveriesalso keeps expired keys forever, because the TTL check on Line 220 never deletes them. In the long-livedpackages/public-apiprocess this memory is retained for the process lifetime.Add a size cap with oldest-entry eviction, and delete expired failure entries when you read them.
♻️ Sketch of a bounded cache
+const MAX_CACHED_SLOTS = 2_000 + +const setBoundedCacheEntry = <TValue>(cache: Map<string, TValue>, key: string, value: TValue): void => { + if (cache.size >= MAX_CACHED_SLOTS) { + const oldestKey = cache.keys().next().value + if (oldestKey !== undefined) cache.delete(oldestKey) + } + cache.set(key, value) +}Then use
setBoundedCacheEntryfor the writes on lines 267, 272, and 276, and callfailedSlotDiscoveries.delete(cacheKey)when the TTL has elapsed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/utils/evm/stateOverride.ts` around lines 40 - 45, Bound all three discovery maps with oldest-entry eviction: add or reuse a setBoundedCacheEntry helper and use it for writes to discoveredSlots, rawDiscoveredSlots, and failedSlotDiscoveries in the relevant discovery flow. Preserve the existing cache keys and capacity policy, and in the failedSlotDiscoveries TTL-read path delete the cacheKey when its entry has expired before continuing discovery.
224-264: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDeduplicate concurrent discovery for the same key.
The caches are written only after discovery completes. Concurrent requests for the same chain, token, and kind therefore run the full sequence in parallel: the known-slot guess, the access list, up to 8 raw probes, and the batched sweep. Rate refreshes and the following quote overlap in practice, so the probe cost multiplies by the number of in-flight callers.
Store the in-flight promise in a map keyed by
cacheKeyand return it to later callers, then delete the entry when it settles.Also add an explicit return type to
probeLocatoron Line 224.♻️ Proposed changes
- const probeLocator = (locator: SlotLocator) => + const probeLocator = (locator: SlotLocator): Promise<boolean> => probeStorageSlot({ ...args, storageSlot: getStorageSlotFromLocator({ ...args, ...locator }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/utils/evm/stateOverride.ts` around lines 224 - 264, Deduplicate concurrent slot discovery by adding an in-flight promise map keyed by cacheKey around the discovery flow that defines found, returning the existing promise for callers with the same key and deleting the map entry when it settles. Preserve the existing cache behavior and probe sequence. Add an explicit promise/boolean return type to probeLocator.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/swapper/src/swappers/NearIntentsSwapper/utils/getNearIntentsStepData.ts`:
- Around line 79-92: Update the rate-path call to getEvmNetworkFeeCryptoBaseUnit
so walletless estimation uses a placeholder sender distinct from depositAddress
instead of falling back to depositAddress. Preserve the existing from value when
a wallet is connected, and ensure the placeholder is used only for the
walletless case while retaining the balance override flow.
---
Nitpick comments:
In `@packages/swapper/src/utils/evm/stateOverride.ts`:
- Around line 40-45: Bound all three discovery maps with oldest-entry eviction:
add or reuse a setBoundedCacheEntry helper and use it for writes to
discoveredSlots, rawDiscoveredSlots, and failedSlotDiscoveries in the relevant
discovery flow. Preserve the existing cache keys and capacity policy, and in the
failedSlotDiscoveries TTL-read path delete the cacheKey when its entry has
expired before continuing discovery.
- Around line 224-264: Deduplicate concurrent slot discovery by adding an
in-flight promise map keyed by cacheKey around the discovery flow that defines
found, returning the existing promise for callers with the same key and deleting
the map entry when it settles. Preserve the existing cache behavior and probe
sequence. Add an explicit promise/boolean return type to probeLocator.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7134d6fa-9a60-4e72-959d-9f9257a6edc4
📒 Files selected for processing (9)
packages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayStepData.tspackages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.tspackages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeStepData.tspackages/swapper/src/swappers/NearIntentsSwapper/utils/getNearIntentsStepData.tspackages/swapper/src/swappers/PortalsSwapper/utils/getPortalsStepData.tspackages/swapper/src/swappers/ThorchainSwapper/getTradeQuote/getTradeQuote.test.tspackages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.tspackages/swapper/src/utils/evm/stateOverride.tspackages/swapper/src/utils/evm/storageSlots.ts
💤 Files with no reviewable changes (3)
- packages/swapper/src/swappers/PortalsSwapper/utils/getPortalsStepData.ts
- packages/swapper/src/swappers/BobGatewaySwapper/utils/getBobGatewayStepData.ts
- packages/swapper/src/swappers/DebridgeSwapper/utils/getDebridgeStepData.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/swapper/src/swappers/ButterSwap/utils/getButterSwapStepData.ts
- packages/swapper/src/swappers/ThorchainSwapper/getTradeQuote/getTradeQuote.test.ts
- packages/swapper/src/utils/evm/getEvmNetworkFeeCryptoBaseUnit.ts
…er sender A depositAddress fallback priced a self-transfer, which touches one balance slot instead of two and estimates low Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 3, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Pre-approval ERC-20 sells revert
eth_estimateGas, so executable quotes could never carry a real gas limit until the user approved — ArbitrumBridge, ButterSwap, THORChain/MAYAChain EVM, and BOB Gateway hard-failed pre-approval quotes outright, and Relay/Across/deBridge/Bebop failed whenever their provider omitted gas. This PR makes the estimation seat approval-aware and removes Tenderly from the swapper package.State-override estimation (
utils/evm/stateOverride.ts): the quote arm ofgetEvmNetworkFeeCryptoBaseUnitnow reads the seller's actual allowance/balance first and, only when insufficient, re-estimates under a minimal node-levelstateOverride(allowance/balance slot patched) on our own RPCs. Sufficient state estimates plainly as before — overrides only apply when they have to. All simulation work is bounded by a single global 3s timeout. All 35 EVM chains probed with negative controls: 34 supporteth_estimateGasoverrides (Scroll's l2geth cannot; only Relay/Across route Scroll and both normally supply provider gas).Slot discovery handles arbitrary tokens: known-table guess first, then
eth_createAccessList-assisted discovery — the node reports which slotsbalanceOf/allowancetouch, and the mapping slot number is recovered by matching them against locally computed hashes (any slot number 0–255, both Soliditykeccak(key‖slot)and Vyperkeccak(slot‖key)orders, one RPC call; supported on 23/32 chains, with a raw-slot fallback for ERC-7201 namespaced storage) — then a batched candidate sweep across both layouts as last resort. Every result is validated by a sentinel probe (aneth_callunder the override must echo the written value) before caching per chain+token. The sentinel fits uint96-packed storage, so UNI/COMP-style governance tokens discover correctly, and the override writesmax(sellAmount, sentinel)rather thanmaxUint256to keep flag bits and packed neighbors clear (USDC blacklist bit, packed balances). Genuinely un-overridable tokens (computed balances: stETH, aTokens, AMPL; shifted-packed: Katana AUSD) fail discovery cleanly, are negative-cached for 60s, and degrade to plain estimation — for funded/approved sellers nothing changes.spenderAddresssingle-sourced from trade contexts: every EVM swapper threads the estimation spender throughstepDataArgsfrom the same expression its context uses forallowanceContract(''= no approval, same sentinel convention). This surfaced and fixed two latent spender mismatches: THORChain longtail pulls tokens through the aggregator token transfer proxy (not the aggregator the tx targets), and the Portals rate arm used the raw ordertargetinstead of the context's cross-chain-aware derivation. Simulating the spender the user will actually approve means a stale spender fails loudly at quote time instead of green-lighting a swap that breaks after approval.Estimated gas limits buffered by default (1.2): previously split per swapper (ButterSwap/deBridge/BOB/Portals buffered, the rest raw).
eth_estimateGasreturns the bare minimum that succeeds and these quotes execute post-approval against moved state — an OOG revert burns the full limit while an oversized one refunds, and the displayed fee still prices the raw estimate, so the margin is effectively free. Provider-supplied limits are never buffered.Tenderly removed from the swapper: the three rate-arm callers (Relay, Portals, NearIntents) now use the same native estimation, which also removes both Monad special-cases Tenderly required. Swapper Tenderly module, config, and public-api env plumbing deleted. The walletConnectToDapps Tenderly integration is separate (asset-change decoding has no estimateGas equivalent) and untouched.
Portals pre-approval quotes (
validate: falsefallback): Portals validates orders by simulating with the sender's live state, so unapproved senders failed at their API before our estimation ran. On validation failure the order is refetched without validation and our estimation supplies the gas limit. Probed extensively: the unvalidated response carries byte-identical calldata and the sameminOutputAmount(fill protection unchanged) — and itsoutputAmountis the minOut, so the fallback quote displays the guaranteed worst-case fill and can never overstate. Validated orders are unchanged (sim-refined output, expected-slippage errors, padded gas).public-api rate inputs are now well-formed (
supportsEIP1559: falsefor EVM sell chains):GetEvmTradeRateInputpins the field and swappers discriminate EVM from Solana inputs on its presence — the rates endpoint omitted it, so Bebop routed EVM pairs to its Solana implementation and errored ("Bebop Solana quote not executable: undefined").Deliberately out of scope (tracked separately): populating
approvalTx/approvalTxson the public-api wire, and USDT approve-to-zero reset detection.Issue (if applicable)
closes #
Risk
Medium. Quote-time gas limits for previously-failing pre-approval token sells are now produced by state-override estimation — the same estimation engine over patched pre-state; the only systematic delta is the unrealized allowance-decrement refund (a few k gas conservative, absorbed by the buffer). Provider-supplied gas limits are still priced as-is everywhere they exist. Estimated limits across all swappers now carry the 1.2 margin (previously only four swappers did). No transaction construction, signing, or broadcast logic is touched — gas limits and displayed fees only.
EVM swap quotes/rates across all swappers (gas limit + network fee display). No on-chain transaction shape changes.
Testing
Engineering
eth_call/eth_estimateGasstate overrides with negative controls (silent param-ignoring caught): 34 PASS, Scroll FAIL (node software; all public Scroll RPCs checked).approval.isRequired, ButterSwap ~1.5M, Relay 76k, Chainflip 63078, NearIntents 63078, MAYAChain 45488, Portals ~430k (via fallback), Bebop/0x provider gas preserved unchanged, CoW gasless. Zero estimation failures remain. Bebop EVM rates verified returning real amounts post input fix, with Solana sells still routing to the Solana implementation. Exotic-token pre-approval quotes through the full API flow: Portals UNI/CRV/COMP/PAXG and NearIntents UNI all pass with estimated gas limits (a dead sender can't estimate plainly, so passing proves the override path end to end).outputAmount === minOutputAmount, drift tracks slippage 1:1).pnpm type-checkclean; full swapper test suite passing (unit tests mockgetMinimalStateOverridesince the override path reads live chain state).Operations
Screenshots (if applicable)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores