feat: Phase 2 V4-native automation — TP/SL, keeper, auto-recenter#310
feat: Phase 2 V4-native automation — TP/SL, keeper, auto-recenter#310irfndi wants to merge 1 commit into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
WalkthroughAdds a Uniswap v4 TP/SL contract, API and database support, verification and auto-recenter services, keeper queue processing, scheduled orchestration, and unit tests. ChangesTP/SL and Keeper Execution
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant User
participant TpSlRoute
participant TpSlService
participant D1
participant CronHandler
participant KEEPER_QUEUE
participant QueueHandler
User->>TpSlRoute: Create TP/SL order
TpSlRoute->>TpSlService: createOrder
TpSlService->>D1: Insert pending order
CronHandler->>KEEPER_QUEUE: Enqueue evaluation
KEEPER_QUEUE->>QueueHandler: Deliver evaluation
QueueHandler->>D1: Update order status
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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}
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (20)
apps/api/src/services/tp-sl.service.ts-178-187 (1)
178-187: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not convert D1 failures into empty order sets. A database outage currently returns HTTP 200 with no user orders and makes keeper evaluation skip executable orders.
apps/api/src/services/tp-sl.service.ts#L178-L187: expose a database error instead ofEffect.succeed([]).apps/api/src/services/tp-sl.service.ts#L189-L205: expose a database error instead ofEffect.succeed([]).apps/api/src/services/tp-sl.service.ts#L207-L216: expose a database error instead ofEffect.succeed([]).apps/api/src/services/tp-sl.service.ts#L274-L283: expose a database error instead ofEffect.succeed([]).🤖 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 `@apps/api/src/services/tp-sl.service.ts` around lines 178 - 187, Stop swallowing database failures in listByUser and the corresponding order-query methods at apps/api/src/services/tp-sl.service.ts lines 178-187, 189-205, 207-216, and 274-283; remove each Effect.succeed([]) recovery so D1 errors propagate to the caller instead of producing empty successful results.apps/api/src/services/tp-sl.service.ts-82-106 (1)
82-106: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftScope the TP/SL data model and every operation by
chain_id. Orders are written with a chain ID but reads and lifecycle updates are chain-agnostic; this exposes and processes orders from other configured networks. The schema also leaves keeper executions unpartitioned and uses non-chain-leading indexes.
apps/api/src/services/tp-sl.service.ts#L82-L106: addchainIdto every relevant service operation.apps/api/src/services/tp-sl.service.ts#L168-L283: includechain_id = ${chainId}in everySELECTand lifecycleUPDATE.apps/api/src/routes/tp-sl.ts#L169-L215: pass the validated configured chain ID to each service call.apps/api/migrations/0004_tp_sl_orders.sql#L22-L25: replace single-chain indexes with chain-leading composite indexes matching user/pool/status queries.apps/api/migrations/0004_tp_sl_orders.sql#L28-L40: addchain_idtokeeper_executionsand index/filter it consistently.apps/api/migrations/0004_tp_sl_orders.sql#L58-L59: make position-policy indexes chain-leading composites.🤖 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 `@apps/api/src/services/tp-sl.service.ts` around lines 82 - 106, Scope all TP/SL operations and persistence by chain_id: in apps/api/src/services/tp-sl.service.ts lines 82-106 add chainId to every relevant TpSlService method, and in lines 168-283 apply it to every SELECT and lifecycle UPDATE; in apps/api/src/routes/tp-sl.ts lines 169-215 pass the validated configured chain ID to each service call. Update apps/api/migrations/0004_tp_sl_orders.sql lines 22-25 with chain-leading composite indexes for user/pool/status queries, lines 28-40 to add and consistently index/filter keeper_executions by chain_id, and lines 58-59 to make position-policy indexes chain-leading composites.Source: Coding guidelines
apps/api/src/routes/tp-sl.ts-49-99 (1)
49-99: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNormalize and strictly validate order inputs before persistence.
poolIdis stored unchanged here, while Lines 172 and 212 lowercase lookup IDs; a mixed-case valid pool ID therefore becomes invisible to trigger evaluation. Require the pool-ID format, persistpoolId.toLowerCase(), and reject invalid boolean/numeric values, negative slippage, non-integer windows, non-future deadlines, and malformed token amounts.🤖 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 `@apps/api/src/routes/tp-sl.ts` around lines 49 - 99, Strengthen the request validation block before destructuring and persistence: require a valid pool-ID format, an actual boolean zeroForOne, finite numeric values, non-negative slippage, an integer twapWindow, a future deadline, and correctly formatted token amounts. Normalize poolId to lowercase in the createOrder payload so persisted IDs match the lowercase lookup IDs used by trigger evaluation. Preserve the existing allowed orderType and range checks.apps/api/src/routes/tp-sl.ts-12-17 (1)
12-17: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRoute the TP/SL endpoints through
@effect/rpc/HonoRPC instead of manual JSON assertions.
c.req.json<T>()accepts untrusted request bodies and then runs Effects directly viarunEffect. Move the request contract into a shared schema/operator boundary so validation/decoding happens once and is reusable for the server and typed client.🤖 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 `@apps/api/src/routes/tp-sl.ts` around lines 12 - 17, Replace the manual TP/SL request handling in the route module with an `@effect/rpc/Hono` RPC contract and shared request schemas/operators. Define the endpoint inputs and outputs at the shared service boundary around TpSlService, have the Hono RPC adapter perform decoding and validation, and remove direct c.req.json<T>() assertions and runEffect-based endpoint execution while preserving requireAuth and existing service behavior.Source: Coding guidelines
apps/api/src/services/keeper.service.ts-366-370 (1)
366-370: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
UPDATElacks achain_idpredicate.
position_idis not unique across chains, so this bumpslast_rebalance_at/rebalance_counton same-id policies belonging to other chains. ThreadchainIdintorecordRebalanceand addAND chain_id = ${chainId}.As per coding guidelines: "Data schemas and queries must include
chain_idin composite keys and filters before indexing a second chain."🤖 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 `@apps/api/src/services/keeper.service.ts` around lines 366 - 370, Update recordRebalance to accept chainId and add a chain_id = ${chainId} predicate to the position_policies UPDATE alongside position_id. Ensure every caller passes the relevant chain ID so only the policy for the intended chain is modified.Source: Coding guidelines
apps/api/src/services/keeper.service.ts-271-278 (1)
271-278: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winCounters report work that never happened.
ordersTriggered++runs for every pending order (the trigger evaluation is still a TODO), andrebalancesQueued++increments without queuing anything. These metrics will read as successful automation activity in production logs. Keep them at zero (or add a separateordersEvaluated) until the real evaluation and queueing are wired in.Also applies to: 300-300
🤖 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 `@apps/api/src/services/keeper.service.ts` around lines 271 - 278, The pending-order loop in the keeper service currently increments ordersTriggered without performing evaluation, and the related rebalance path increments rebalancesQueued without queueing work. Remove or defer both increments until their respective operations are implemented; keep these success counters at zero rather than reporting unperformed automation.apps/api/src/services/auto-recenter.service.ts-89-104 (1)
89-104: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEvery active policy is unconditionally reported as out-of-range.
The current tick is never read, yet each row is pushed as a
RebalanceRequestwithtargetTick* === currentTick*. Downstream this queues a rebalance for every position on every tick — churn and gas burn for no-op ranges. Gate on a real tick read (ChainStateReader.getPoolState) or return an empty list until that wiring lands, rather than emitting false positives.Want me to open an issue tracking the
ChainStateReaderwiring for this TODO?🤖 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 `@apps/api/src/services/auto-recenter.service.ts` around lines 89 - 104, Update the request-building flow in the auto-recenter service so it does not emit a RebalanceRequest without a real current tick from ChainStateReader.getPoolState. Until that wiring is available, return an empty list or otherwise skip these rows; only push requests when the pool state confirms the position is out of range, avoiding requests whose target ticks equal the current range.apps/api/src/services/verification.service.ts-190-192 (1)
190-192: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDual spot+TWAP gate is a no-op — TWAP is aliased to spot.
twapPriceX18 = spotPriceX18, sodualTriggerMetdegenerates to the spot check whileisTriggeredis documented as "Require both spot and TWAP". Any fund-moving execution gated on this is decided purely by manipulable instantaneous pool price. Until the Phase-0 G2.5 TWAP source exists, returntwapPriceX18: nulland surface an explicit error/flag (or refuse to reportisTriggered: true) rather than silently claiming a dual-trigger.As per coding guidelines: "Do not rely on the AetherHook observation buffer for TP/SL or auto-recenter until the Phase-0 G2.5 fix records and time-weights pool-state ticks".
Also applies to: 226-227
🤖 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 `@apps/api/src/services/verification.service.ts` around lines 190 - 192, Remove the spot-price alias from the TWAP path in the verification service: set twapPriceX18 to null until the Phase-0 G2.5 TWAP source is available. Update the dual-trigger/isTriggered handling so it cannot report a successful dual spot+TWAP trigger when TWAP is unavailable, and surface the existing explicit error or unavailable flag through the response fields around the related return logic.Source: Coding guidelines
apps/api/src/services/keeper.service.ts-363-365 (1)
363-365: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
last_rebalance_atis written as fractional seconds while the schema uses epoch milliseconds.apps/api/migrations/0004_tp_sl_orders.sqldefaults every other timestamp tounixepoch() * 1000, andDate.now() / 1000also stores a non-integer into an INTEGER column. Pick milliseconds consistently across the writer and both readers.
apps/api/src/services/keeper.service.ts#L363-L365: writeDate.now()(integer ms) instead ofDate.now() / 1000.apps/api/src/services/auto-recenter.service.ts#L175-L175: comparepp.last_rebalance_atagainst a millisecond cutoff.apps/api/src/services/keeper.service.ts#L152-L152: computeelapsedin ms inevaluateAntiWhipsawand scalecooldownSecondsby 1000.🤖 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 `@apps/api/src/services/keeper.service.ts` around lines 363 - 365, Use epoch milliseconds consistently for last_rebalance_at: in apps/api/src/services/keeper.service.ts lines 363-365, update recordRebalance to write Date.now(); in apps/api/src/services/auto-recenter.service.ts line 175, compare pp.last_rebalance_at against a millisecond cutoff; and in apps/api/src/services/keeper.service.ts line 152, update evaluateAntiWhipsaw to calculate elapsed milliseconds and multiply cooldownSeconds by 1000.apps/api/src/services/auto-recenter.service.ts-146-150 (1)
146-150: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftInsert with
order_id = 0violates the FK and the failure is swallowed.
keeper_executions.order_idreferencestp_sl_orders(id)(apps/api/migrations/0004_tp_sl_orders.sqlLines 28-40), and no row has id 0, so this insert fails whenever FK enforcement is on.Effect.catch(() => Effect.succeed(undefined))then hides it andqueueRebalancestill returns arequestId, so callers believe work was queued that never persisted. Also notetx_hashis being repurposed to holdrequestId.Rebalances need their own table (or a nullable
order_id), and the insert error must propagate asAutoRecenterError.🤖 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 `@apps/api/src/services/auto-recenter.service.ts` around lines 146 - 150, Update the auto-recenter persistence flow around queueRebalance so rebalances are stored without using a fabricated order_id of 0 or repurposing tx_hash for requestId. Add/use a dedicated rebalance record or make order_id nullable with an appropriate schema migration, store requestId in its intended field, and remove the swallowed Effect.catch. Propagate insertion failures as AutoRecenterError so queueRebalance does not return a requestId unless the work is persisted.apps/api/src/services/auto-recenter.service.ts-121-129 (1)
121-129: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winComputed range bounds are not tick-spacing aligned.
Only
alignedTickis aligned; subtracting/addinghalfRangebreaks alignment (e.g.tickSpacing = 60→halfRange = 90, so both bounds are off-grid). V4 rejects positions whose ticks are not multiples oftickSpacing.🐛 Proposed fix
- const halfRange = Math.floor(rangeWidthTicks / 2) + const halfRange = Math.max(1, Math.round(rangeWidthTicks / 2 / tickSpacing)) * tickSpacing🤖 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 `@apps/api/src/services/auto-recenter.service.ts` around lines 121 - 129, Update the range-bound calculation in the auto-recenter logic so both tickLower and tickUpper are multiples of tickSpacing, not just alignedTick. Quantize the range width or half-range to tick-spacing units before applying it around alignedTick, while preserving the centered-range behavior and existing minimum width.apps/api/src/services/keeper.service.ts-288-304 (1)
288-304: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle the per-position
yieldresult instead of usingtry/catch.Inside
Effect.gen, a failingyield*escapes to the outerEffect.catch, so one bad policy fails the whole keeper tick instead of recording the position-specific error and skipping it. UseEffect.result(evaluateAntiWhipsaw(policy))and branch on the returnedResult, then apply the same pattern if the pending-order handling becomes effectful.🤖 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 `@apps/api/src/services/keeper.service.ts` around lines 288 - 304, Update the per-policy evaluation loop around evaluateAntiWhipsaw to wrap it with Effect.result and branch on the returned Result instead of relying on try/catch. Record failures in errors with the policy.positionId, continue to the next policy, and preserve the existing shouldExecute handling for successful results; apply the same per-position Result handling to any pending-order effect introduced in this flow.apps/api/src/workers/queue-handler.ts-113-117 (1)
113-117: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeeper queue handlers ignore
msg.chainIdin every SQL predicate. Both handlers receivechainIdin the message but scope reads and writes by primary key alone, so rows from another chain can be read or mutated once a second chain is indexed.
apps/api/src/workers/queue-handler.ts#L113-L117: addAND chain_id = ?to the pending-orderSELECTand to theexpired/triggeredUPDATEs (Lines 126 and 202-208), bindingmsg.chainId.apps/api/src/workers/queue-handler.ts#L233-L237: addAND chain_id = ?to theposition_policiesSELECTand to thelast_rebalance_atUPDATE(Lines 307-313), bindingmsg.chainId.As per coding guidelines, "Data schemas and queries must include
chain_idin composite keys and filters before indexing a second chain."🤖 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 `@apps/api/src/workers/queue-handler.ts` around lines 113 - 117, Scope every Keeper queue-handler query by msg.chainId: in apps/api/src/workers/queue-handler.ts lines 113-117, add the chain_id predicate and bind msg.chainId for the pending-order SELECT, and apply the same predicate and binding to the expired/triggered UPDATEs at lines 126 and 202-208; in lines 233-237, add and bind chain_id for the position_policies SELECT and the last_rebalance_at UPDATE at lines 307-313.Source: Coding guidelines
apps/api/src/workers/queue-handler.ts-431-438 (1)
431-438: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftTick derived from a USD price ratio ignores token decimals, so range checks are wrong.
Uniswap ticks encode the raw-amount ratio
token1/token0including decimal scaling;poolSpot.priceis a USD-ratio of human-readable prices. For any pair with differing decimals (e.g. 18 vs 6) the derived tick is off bylog_1.0001(10^(d1-d0))≈ ±276k ticks, which makesisInRange/driftBpsmeaningless and can trigger rebalances on in-range positions. Prefer persisting the pool's actualtick/sqrtPriceX96(the writer already supports an optionaltickfield) and treat the price fallback as unavailable rather than approximating.Also, the
_prefix conventionally signals unused; it is called at Line 265 — drop the prefix.🤖 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 `@apps/api/src/workers/queue-handler.ts` around lines 431 - 438, The tick derivation fallback is invalid because poolSpot.price lacks token-decimal scaling; use the pool’s persisted actual tick (or sqrtPriceX96-derived tick) via the existing optional tick field, and treat missing market tick data as unavailable instead of calling _tickFromPriceX18. Update the caller around the tick calculation at line 265 to handle the unavailable fallback without range or drift checks, and rename _tickFromPriceX18 to tickFromPriceX18 if it remains necessary.apps/api/wrangler.jsonc-65-83 (1)
65-83: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd queue bindings to each named environment.
Wrangler named environments do not inherit
queuesfrom the top-level config. Staging and production only definevars, soKEEPER_QUEUEand thekeeper-jobsconsumer will be missing when deploying those environments. Add the matchingqueuesproducers/consumers to both named environments, or deploy only the top-level environment if these bindings are intended there.🤖 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 `@apps/api/wrangler.jsonc` around lines 65 - 83, Add the complete queues configuration to both named environments, including the PRICE_QUEUE, SETTLE_QUEUE, and KEEPER_QUEUE producers plus the price-refresh and keeper-jobs consumers with their existing batch sizes, retry limits, and dead-letter queues. Keep the top-level queues configuration unchanged.apps/api/src/workers/queue-handler.ts-141-156 (1)
141-156: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUnguarded
JSON.parse+BigInt()on cache payloads causes retry storms.Malformed/exponential cache values (see the
poolSpotwriter inapps/api/src/workers/cron-handler.ts) throw here, the batch handler callsmessage.retry(), and the message churns until DLQ. Wrap parsing/conversion and skip the message on invalid data.🛡️ Proposed guard
- const { price: spotPriceX18 } = JSON.parse(spotPriceData) as { price: string } + let spotPriceX18: string + try { + spotPriceX18 = (JSON.parse(spotPriceData) as { price: string }).price + BigInt(spotPriceX18) + } catch { + console.warn(`[Keeper] Invalid spot price payload for pool ${msg.poolId}, skipping`) + return + }🤖 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 `@apps/api/src/workers/queue-handler.ts` around lines 141 - 156, Guard the spot and TWAP cache payload handling in the queue worker before JSON.parse and BigInt conversion, including the values read near spotPriceX18 and twapPriceX18. Catch malformed, exponential, or otherwise invalid payloads, log or record the invalid-data condition using the existing worker conventions, and skip/acknowledge the message without calling message.retry(); preserve normal trigger evaluation for valid cache values.packages/contracts/src/tpsl/AetherTPSL.sol-201-208 (1)
201-208: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winProceeds transfer happens before order-state update;
cancelOrderlacksnonReentrant.
order.statusis set toEXECUTED(line 275) only aftersafeTransfertoorder.owner(line 273) — the reverse of checks-effects-interactions despite the comment claiming CEI.executeOrder's ownnonReentrantblocks re-entry into itself/othernonReentrantfunctions, butcancelOrderisn't guarded and only checksstatus == PENDING. IftokenOuthas any transfer callback (e.g., ERC-777-style), the recipient (the order owner) can callcancelOrdermid-transfer whilestatusis stillPENDING, emitting a misleadingOrderCancelledevent that off-chain consumers may act on (pertp-sl.service.ts's cancel/execute transitions), even thoughexecuteOrderwill subsequently overwrite it toEXECUTED.Move the
order.status = EXECUTED/executedAtupdate before thesafeTransfer, and/or addnonReentranttocancelOrder.Also applies to: 236-279
🤖 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/contracts/src/tpsl/AetherTPSL.sol` around lines 201 - 208, Update the executeOrder flow to apply checks-effects-interactions: set order.status to EXECUTED and update executedAt before transferring tokenOut to order.owner. Also guard cancelOrder with nonReentrant so it cannot cancel an order during the transfer callback, while preserving its existing authorization and pending-status checks.packages/contracts/test/unit/AetherTPSL.t.sol-44-355 (1)
44-355: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftCore money-moving paths have zero test coverage.
There are no tests for
deposit,withdraw, orexecuteOrder(the swap-execution path that moves user funds and sends proceeds), nor forunlockCallback/_handleSwapExactIn, nor for actual_evaluateTriggerbreach scenarios (isTriggeredtests only cover the "not pending"/"expired" short-circuits). GivenexecuteOrderis where the sign/PoolId/CEI issues flagged inAetherTPSL.solwould actually surface, exercising it here would have caught them.As per coding guidelines, "Maintain contract test coverage above 90% and use Forge tests together with Slither and Echidna analysis."
🤖 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/contracts/test/unit/AetherTPSL.t.sol` around lines 44 - 355, Extend AetherTPSLTest with Forge coverage for the money-moving paths: deposit, withdraw, executeOrder, unlockCallback, and _handleSwapExactIn. Add trigger-breach scenarios that exercise _evaluateTrigger through isTriggered, and assert successful transfers, proceeds, state transitions, and relevant reverts/events while preserving existing setup helpers and mock contracts. Ensure tests cover executeOrder’s PoolId, sign, and CEI-sensitive behavior and raise contract coverage above 90%.Source: Coding guidelines
packages/contracts/src/tpsl/AetherTPSL.sol-213-231 (1)
213-231: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo native-currency support; stray
receive()risks permanently stuck ETH.
deposit/withdraw/executeOrderalways callIERC20(token).safeTransferFrom/safeTransfer, which reverts foraddress(0)(the conventional native-ETHCurrency), so native-ETH pools can't actually be funded or paid out through this contract despite importingCurrencyLibrary. Meanwhilereceive() external payable {}(line 419) silently accepts direct ETH transfers with zero accounting indepositedBalances, so any ETH sent to the contract (accidentally or otherwise) is unrecoverable.Either implement native-currency handling consistently (checking
Currency.isNative()before choosing ERC20 vs native transfer paths) or removereceive()so unsupported ETH transfers simply revert instead of being locked.Also applies to: 272-273, 419-419
🤖 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/contracts/src/tpsl/AetherTPSL.sol` around lines 213 - 231, Add consistent native-currency handling to deposit, withdraw, and executeOrder using Currency.isNative(): require and account msg.value for native deposits, and use native value transfers for payouts while retaining ERC20 behavior otherwise. Remove the empty receive() so unaccounted direct ETH transfers revert, and ensure native paths validate value and preserve existing balance accounting.packages/contracts/src/tpsl/AetherTPSL.sol-89-91 (1)
89-91: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUnwrap
PoolIdat thebytes32boundaries.
PoolKey.toId()returns Solidity 0.8 user-defined typePoolId; it’s not implicitly convertible tobytes32forpoolOrders,OrderCreated.poolId,PoolManager.getSlot0(), orIAetherHook.getCurrentTwap(). Unwrap at each call, e.g.PoolId.unwrap(params.poolKey.toId())andPoolId.unwrap(order.poolKey.toId()).🤖 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/contracts/src/tpsl/AetherTPSL.sol` around lines 89 - 91, Update every bytes32 boundary involving PoolId to explicitly unwrap the user-defined value with PoolId.unwrap, including poolOrders accesses, OrderCreated.poolId assignments, PoolManager.getSlot0 calls, and IAetherHook.getCurrentTwap calls. Preserve the existing PoolKey.toId() behavior while ensuring each relevant argument or key is passed as bytes32.
🟡 Minor comments (2)
apps/api/src/services/verification.service.ts-139-141 (1)
139-141: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHardcoded
887220tick bound is wrong in both services. Uniswap V4's absolute tick limits are ±887272;887220is only the max usable tick fortickSpacing = 60, so valid ticks/ranges near the extremes are rejected as "out of V4 bounds". One shared constant fixes both.
apps/api/src/services/verification.service.ts#L139-L141: comparecurrentTickagainst a sharedMIN_TICK/MAX_TICK(±887272) instead of the inline887220literals.apps/api/src/services/auto-recenter.service.ts#L132-L136: validate the computed[tickLower, tickUpper]against the same shared constants.🤖 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 `@apps/api/src/services/verification.service.ts` around lines 139 - 141, Replace the incorrect 887220 bounds with shared MIN_TICK/MAX_TICK constants set to ±887272. Update currentTick validation in apps/api/src/services/verification.service.ts lines 139-141 and tickLower/tickUpper validation in apps/api/src/services/auto-recenter.service.ts lines 132-136 to reuse those constants.apps/api/src/workers/queue-handler.ts-245-253 (1)
245-253: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPersist
last_rebalance_atas an integer second.
Date.now() / 1000writes a fractional epoch intolast_rebalance_at; useMath.floor(Date.now() / 1000)for consistent cooldown arithmetic and storage.Also applies to: 307-313
🤖 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 `@apps/api/src/workers/queue-handler.ts` around lines 245 - 253, Update the timestamp calculation used by the queue-handler rebalance flow to assign `now` with `Math.floor(Date.now() / 1000)`, including the corresponding calculation near the other reported location. Use this integer-second value for cooldown arithmetic and when persisting `last_rebalance_at`.
🧹 Nitpick comments (7)
apps/api/src/services/keeper.service.ts (3)
198-217: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused
_evaluateIdleReservewith underscore-prefixed params that are used. The header comment advertises 5 policies but this one is neither exported nor on theKeeperServiceinterface. Either expose it (dropping the_prefixes, since_reserveBalanceUsd/_minReserveUsdare read at Lines 202-206) or remove it until wired up.🤖 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 `@apps/api/src/services/keeper.service.ts` around lines 198 - 217, The unused _evaluateIdleReserve function is not exposed despite being implemented. Either export it and add it to the KeeperService interface, renaming _reserveBalanceUsd and _minReserveUsd to reserveBalanceUsd and minReserveUsd, or remove the function and its related policy reference until it is wired up.
353-361: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSwallowing query failures into
[]hides DB outages.getActivePoliciesis typedneveron the error channel, so a D1 failure is indistinguishable from "no active policies" and the keeper silently does nothing. Prefer surfacingKeeperErrorand letting the caller decide, or at minimum log before defaulting.🤖 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 `@apps/api/src/services/keeper.service.ts` around lines 353 - 361, Update getActivePolicies to stop silently converting database query failures into an empty policy list. Surface the SQL failure as the existing KeeperError type through the Effect error channel and adjust its return type and callers to handle that error, rather than retaining Effect.catch with Effect.succeed([]).
243-249: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMutable counters live outside the returned Effect. If the tick Effect is ever retried (queue/cron retry semantics), counters accumulate across attempts. Moving the state initialization inside
Effect.genmakes each run independent.🤖 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 `@apps/api/src/services/keeper.service.ts` around lines 243 - 249, Move the counter and errors-array initialization out of the outer tick setup and into the returned Effect.gen execution in tick. Ensure each evaluation of the Keeper tick creates fresh ordersChecked, ordersTriggered, ordersFailed, positionsChecked, rebalancesQueued, and errors state so retries start from zero.apps/api/src/services/verification.service.ts (2)
103-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated read-error wrapper; drop the unused
_rangeCenter.The same
Effect.catchblock ongetPoolStateis repeated in all three methods, and_rangeCenterat Line 116 is dead code.♻️ Suggested cleanup
+const readPoolState = (reader: ChainStateReader, poolKey: PoolKeyParams) => + reader.getPoolState(poolKey).pipe( + Effect.catch((error) => + Effect.fail( + error._tag === "OnChainReadError" + ? new VerificationError("pool_read_failed", error.message) + : new VerificationError("unknown", String(error)), + ), + ), + )- // Calculate drift from range center - const _rangeCenter = (input.tickLower + input.tickUpper) / 2 const rangeSize = input.tickUpper - input.tickLowerAlso applies to: 116-116
🤖 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 `@apps/api/src/services/verification.service.ts` around lines 103 - 110, Extract the shared getPoolState Effect.catch transformation into a reusable helper, then use it in all three methods while preserving the existing OnChainReadError and unknown VerificationError mappings. Remove the unused _rangeCenter variable at the indicated location.
143-154: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
poolId: ""leaks a placeholder through a typed contract. Either compute the pool id here frominput.poolKeyor remove the field fromPositionVerificationso callers can't consume an empty string as a real id.🤖 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 `@apps/api/src/services/verification.service.ts` around lines 143 - 154, Resolve the placeholder poolId in the PositionVerification result returned by the verification method: compute it from input.poolKey and return the real identifier, or remove poolId from the PositionVerification type and this object if callers do not require it. Ensure no empty-string pool ID remains consumable as a valid value.apps/api/wrangler.jsonc (1)
77-82: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider explicit batch timeout and retry delay for
keeper-jobs.
max_retries: 5with default (immediate-ish) redelivery gives no backoff for transient RPC/DB failures;retry_delayplusmax_batch_timeoutwould make keeper retries predictable, and TP/SL evaluation is latency-sensitive so a small timeout matters.🤖 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 `@apps/api/wrangler.jsonc` around lines 77 - 82, Update the keeper-jobs queue configuration to explicitly define a small max_batch_timeout for latency-sensitive TP/SL evaluation and a retry_delay that provides backoff between transient RPC/DB failures, while preserving the existing max_batch_size, max_retries, and dead-letter queue settings.apps/api/src/workers/cron-handler.ts (1)
214-237: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueSequential KV round-trips per pool inflate cron duration.
Up to 50 pools × (2
get+ 1put) executed serially. Reuse the token prices already fetched, or at least batch pools with bounded concurrency.🤖 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 `@apps/api/src/workers/cron-handler.ts` around lines 214 - 237, Update the poolSpot-writing flow around the loop over pools.results to avoid serial KV round-trips for every pool: reuse already-fetched token prices when available, or process pool updates with bounded concurrency while preserving the existing price validation, cache key, TTL, and per-pool error handling. Do not allow unbounded parallelism.
🤖 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 `@apps/api/src/services/auto-recenter.service.ts`:
- Around line 71-79: Replace the PostgreSQL-specific lp.id::text casts in both
SQL queries within the auto-recenter service with SQLite-compatible CAST(lp.id
AS TEXT) expressions, preserving the existing join behavior.
In `@apps/api/src/workers/cron-handler.ts`:
- Around line 144-152: Update the outOfRangePositions query to use
SQLite-compatible CAST(lp.id AS TEXT) instead of PostgreSQL’s lp.id::text, and
add lp.chain_id = ? to the join/filter conditions. Bind the additional chain ID
parameter in the existing chainId query preparation while preserving the current
auto-recenter selection behavior.
- Around line 225-233: Update the spot-price calculation in the cron handler
around spotPriceX18 to avoid Number-based scaling and exponential serialization.
Convert each positive USD price to a fixed-precision integer, perform the ratio
and 1e18 scaling using BigInt arithmetic, then stringify the resulting BigInt so
consumers such as handleTpSlEvaluate and handleAutoRecenterCheck can parse it
reliably.
In `@packages/contracts/src/tpsl/AetherTPSL.sol`:
- Line 33: Redesign AetherTPSL to remove custodial balances: eliminate
depositedBalances and the deposit()/withdraw() escrow flow, and update
executeOrder() to pull amountIn from the order owner with safeTransferFrom at
execution time using the existing approval model. Ensure the contract holds no
user funds between transactions and update the related documentation and
affected order logic accordingly.
- Around line 253-266: Update the SwapParams construction in the exact-input
swap flow to set amountSpecified to the negative int256 representation of
order.amountIn. In _handleSwapExactIn, derive amountIn by negating
swapParams.amountSpecified before converting it to uint256, while preserving the
existing SWAP_EXACT_IN action and output-delta handling.
In `@packages/contracts/test/unit/AetherTPSL.t.sol`:
- Line 117: Update the tuple destructuring in the tests around
tpsl.orders(orderId) at both affected locations to match all 14 fields returned
by TpSlOrder. Add placeholders for the omitted fields, especially between
triggerPrice and status and through createdAt, while preserving the existing
owner, orderType, zeroForOne, triggerPrice, and status bindings; alternatively,
use the explicit getOrder(uint256) accessor.
- Line 5: Update the test helpers in AetherTPSL.t.sol to qualify every
CreateOrderParams reference with the imported AetherTPSL namespace, including
local declarations and return types, so they use AetherTPSL.CreateOrderParams
consistently.
---
Major comments:
In `@apps/api/src/routes/tp-sl.ts`:
- Around line 49-99: Strengthen the request validation block before
destructuring and persistence: require a valid pool-ID format, an actual boolean
zeroForOne, finite numeric values, non-negative slippage, an integer twapWindow,
a future deadline, and correctly formatted token amounts. Normalize poolId to
lowercase in the createOrder payload so persisted IDs match the lowercase lookup
IDs used by trigger evaluation. Preserve the existing allowed orderType and
range checks.
- Around line 12-17: Replace the manual TP/SL request handling in the route
module with an `@effect/rpc/Hono` RPC contract and shared request
schemas/operators. Define the endpoint inputs and outputs at the shared service
boundary around TpSlService, have the Hono RPC adapter perform decoding and
validation, and remove direct c.req.json<T>() assertions and runEffect-based
endpoint execution while preserving requireAuth and existing service behavior.
In `@apps/api/src/services/auto-recenter.service.ts`:
- Around line 89-104: Update the request-building flow in the auto-recenter
service so it does not emit a RebalanceRequest without a real current tick from
ChainStateReader.getPoolState. Until that wiring is available, return an empty
list or otherwise skip these rows; only push requests when the pool state
confirms the position is out of range, avoiding requests whose target ticks
equal the current range.
- Around line 146-150: Update the auto-recenter persistence flow around
queueRebalance so rebalances are stored without using a fabricated order_id of 0
or repurposing tx_hash for requestId. Add/use a dedicated rebalance record or
make order_id nullable with an appropriate schema migration, store requestId in
its intended field, and remove the swallowed Effect.catch. Propagate insertion
failures as AutoRecenterError so queueRebalance does not return a requestId
unless the work is persisted.
- Around line 121-129: Update the range-bound calculation in the auto-recenter
logic so both tickLower and tickUpper are multiples of tickSpacing, not just
alignedTick. Quantize the range width or half-range to tick-spacing units before
applying it around alignedTick, while preserving the centered-range behavior and
existing minimum width.
In `@apps/api/src/services/keeper.service.ts`:
- Around line 366-370: Update recordRebalance to accept chainId and add a
chain_id = ${chainId} predicate to the position_policies UPDATE alongside
position_id. Ensure every caller passes the relevant chain ID so only the policy
for the intended chain is modified.
- Around line 271-278: The pending-order loop in the keeper service currently
increments ordersTriggered without performing evaluation, and the related
rebalance path increments rebalancesQueued without queueing work. Remove or
defer both increments until their respective operations are implemented; keep
these success counters at zero rather than reporting unperformed automation.
- Around line 363-365: Use epoch milliseconds consistently for
last_rebalance_at: in apps/api/src/services/keeper.service.ts lines 363-365,
update recordRebalance to write Date.now(); in
apps/api/src/services/auto-recenter.service.ts line 175, compare
pp.last_rebalance_at against a millisecond cutoff; and in
apps/api/src/services/keeper.service.ts line 152, update evaluateAntiWhipsaw to
calculate elapsed milliseconds and multiply cooldownSeconds by 1000.
- Around line 288-304: Update the per-policy evaluation loop around
evaluateAntiWhipsaw to wrap it with Effect.result and branch on the returned
Result instead of relying on try/catch. Record failures in errors with the
policy.positionId, continue to the next policy, and preserve the existing
shouldExecute handling for successful results; apply the same per-position
Result handling to any pending-order effect introduced in this flow.
In `@apps/api/src/services/tp-sl.service.ts`:
- Around line 178-187: Stop swallowing database failures in listByUser and the
corresponding order-query methods at apps/api/src/services/tp-sl.service.ts
lines 178-187, 189-205, 207-216, and 274-283; remove each Effect.succeed([])
recovery so D1 errors propagate to the caller instead of producing empty
successful results.
- Around line 82-106: Scope all TP/SL operations and persistence by chain_id: in
apps/api/src/services/tp-sl.service.ts lines 82-106 add chainId to every
relevant TpSlService method, and in lines 168-283 apply it to every SELECT and
lifecycle UPDATE; in apps/api/src/routes/tp-sl.ts lines 169-215 pass the
validated configured chain ID to each service call. Update
apps/api/migrations/0004_tp_sl_orders.sql lines 22-25 with chain-leading
composite indexes for user/pool/status queries, lines 28-40 to add and
consistently index/filter keeper_executions by chain_id, and lines 58-59 to make
position-policy indexes chain-leading composites.
In `@apps/api/src/services/verification.service.ts`:
- Around line 190-192: Remove the spot-price alias from the TWAP path in the
verification service: set twapPriceX18 to null until the Phase-0 G2.5 TWAP
source is available. Update the dual-trigger/isTriggered handling so it cannot
report a successful dual spot+TWAP trigger when TWAP is unavailable, and surface
the existing explicit error or unavailable flag through the response fields
around the related return logic.
In `@apps/api/src/workers/queue-handler.ts`:
- Around line 113-117: Scope every Keeper queue-handler query by msg.chainId: in
apps/api/src/workers/queue-handler.ts lines 113-117, add the chain_id predicate
and bind msg.chainId for the pending-order SELECT, and apply the same predicate
and binding to the expired/triggered UPDATEs at lines 126 and 202-208; in lines
233-237, add and bind chain_id for the position_policies SELECT and the
last_rebalance_at UPDATE at lines 307-313.
- Around line 431-438: The tick derivation fallback is invalid because
poolSpot.price lacks token-decimal scaling; use the pool’s persisted actual tick
(or sqrtPriceX96-derived tick) via the existing optional tick field, and treat
missing market tick data as unavailable instead of calling _tickFromPriceX18.
Update the caller around the tick calculation at line 265 to handle the
unavailable fallback without range or drift checks, and rename _tickFromPriceX18
to tickFromPriceX18 if it remains necessary.
- Around line 141-156: Guard the spot and TWAP cache payload handling in the
queue worker before JSON.parse and BigInt conversion, including the values read
near spotPriceX18 and twapPriceX18. Catch malformed, exponential, or otherwise
invalid payloads, log or record the invalid-data condition using the existing
worker conventions, and skip/acknowledge the message without calling
message.retry(); preserve normal trigger evaluation for valid cache values.
In `@apps/api/wrangler.jsonc`:
- Around line 65-83: Add the complete queues configuration to both named
environments, including the PRICE_QUEUE, SETTLE_QUEUE, and KEEPER_QUEUE
producers plus the price-refresh and keeper-jobs consumers with their existing
batch sizes, retry limits, and dead-letter queues. Keep the top-level queues
configuration unchanged.
In `@packages/contracts/src/tpsl/AetherTPSL.sol`:
- Around line 201-208: Update the executeOrder flow to apply
checks-effects-interactions: set order.status to EXECUTED and update executedAt
before transferring tokenOut to order.owner. Also guard cancelOrder with
nonReentrant so it cannot cancel an order during the transfer callback, while
preserving its existing authorization and pending-status checks.
- Around line 213-231: Add consistent native-currency handling to deposit,
withdraw, and executeOrder using Currency.isNative(): require and account
msg.value for native deposits, and use native value transfers for payouts while
retaining ERC20 behavior otherwise. Remove the empty receive() so unaccounted
direct ETH transfers revert, and ensure native paths validate value and preserve
existing balance accounting.
- Around line 89-91: Update every bytes32 boundary involving PoolId to
explicitly unwrap the user-defined value with PoolId.unwrap, including
poolOrders accesses, OrderCreated.poolId assignments, PoolManager.getSlot0
calls, and IAetherHook.getCurrentTwap calls. Preserve the existing
PoolKey.toId() behavior while ensuring each relevant argument or key is passed
as bytes32.
In `@packages/contracts/test/unit/AetherTPSL.t.sol`:
- Around line 44-355: Extend AetherTPSLTest with Forge coverage for the
money-moving paths: deposit, withdraw, executeOrder, unlockCallback, and
_handleSwapExactIn. Add trigger-breach scenarios that exercise _evaluateTrigger
through isTriggered, and assert successful transfers, proceeds, state
transitions, and relevant reverts/events while preserving existing setup helpers
and mock contracts. Ensure tests cover executeOrder’s PoolId, sign, and
CEI-sensitive behavior and raise contract coverage above 90%.
---
Minor comments:
In `@apps/api/src/services/verification.service.ts`:
- Around line 139-141: Replace the incorrect 887220 bounds with shared
MIN_TICK/MAX_TICK constants set to ±887272. Update currentTick validation in
apps/api/src/services/verification.service.ts lines 139-141 and
tickLower/tickUpper validation in apps/api/src/services/auto-recenter.service.ts
lines 132-136 to reuse those constants.
In `@apps/api/src/workers/queue-handler.ts`:
- Around line 245-253: Update the timestamp calculation used by the
queue-handler rebalance flow to assign `now` with `Math.floor(Date.now() /
1000)`, including the corresponding calculation near the other reported
location. Use this integer-second value for cooldown arithmetic and when
persisting `last_rebalance_at`.
---
Nitpick comments:
In `@apps/api/src/services/keeper.service.ts`:
- Around line 198-217: The unused _evaluateIdleReserve function is not exposed
despite being implemented. Either export it and add it to the KeeperService
interface, renaming _reserveBalanceUsd and _minReserveUsd to reserveBalanceUsd
and minReserveUsd, or remove the function and its related policy reference until
it is wired up.
- Around line 353-361: Update getActivePolicies to stop silently converting
database query failures into an empty policy list. Surface the SQL failure as
the existing KeeperError type through the Effect error channel and adjust its
return type and callers to handle that error, rather than retaining Effect.catch
with Effect.succeed([]).
- Around line 243-249: Move the counter and errors-array initialization out of
the outer tick setup and into the returned Effect.gen execution in tick. Ensure
each evaluation of the Keeper tick creates fresh ordersChecked, ordersTriggered,
ordersFailed, positionsChecked, rebalancesQueued, and errors state so retries
start from zero.
In `@apps/api/src/services/verification.service.ts`:
- Around line 103-110: Extract the shared getPoolState Effect.catch
transformation into a reusable helper, then use it in all three methods while
preserving the existing OnChainReadError and unknown VerificationError mappings.
Remove the unused _rangeCenter variable at the indicated location.
- Around line 143-154: Resolve the placeholder poolId in the
PositionVerification result returned by the verification method: compute it from
input.poolKey and return the real identifier, or remove poolId from the
PositionVerification type and this object if callers do not require it. Ensure
no empty-string pool ID remains consumable as a valid value.
In `@apps/api/src/workers/cron-handler.ts`:
- Around line 214-237: Update the poolSpot-writing flow around the loop over
pools.results to avoid serial KV round-trips for every pool: reuse
already-fetched token prices when available, or process pool updates with
bounded concurrency while preserving the existing price validation, cache key,
TTL, and per-pool error handling. Do not allow unbounded parallelism.
In `@apps/api/wrangler.jsonc`:
- Around line 77-82: Update the keeper-jobs queue configuration to explicitly
define a small max_batch_timeout for latency-sensitive TP/SL evaluation and a
retry_delay that provides backoff between transient RPC/DB failures, while
preserving the existing max_batch_size, max_retries, and dead-letter queue
settings.
🪄 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: 23517f8b-1e52-4ce3-b5b2-a613ef508e0e
📒 Files selected for processing (13)
apps/api/migrations/0004_tp_sl_orders.sqlapps/api/src/routes/tp-sl.tsapps/api/src/services/auto-recenter.service.tsapps/api/src/services/keeper.service.tsapps/api/src/services/tp-sl.service.tsapps/api/src/services/verification.service.tsapps/api/src/workers/cron-handler.tsapps/api/src/workers/queue-handler.tsapps/api/wrangler.jsoncpackages/contracts/src/hook/IAetherHook.solpackages/contracts/src/lib/Errors.solpackages/contracts/src/tpsl/AetherTPSL.solpackages/contracts/test/unit/AetherTPSL.t.sol
| const positions = (yield* sql` | ||
| SELECT pp.*, lp.tick_lower, lp.tick_upper, lp.liquidity | ||
| FROM position_policies pp | ||
| JOIN liquidity_positions lp ON pp.position_id = lp.id::text | ||
| WHERE pp.chain_id = ${chainId} | ||
| AND pp.is_active = 1 | ||
| AND pp.policy_type = 'auto_recenter' | ||
| AND lp.is_active = 1 | ||
| `) as unknown as readonly Record<string, unknown>[] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
lp.id::text is PostgreSQL syntax and will fail on D1/SQLite.
Both queries use the ::text cast operator, which SQLite does not support — these statements will throw at runtime. Use CAST(lp.id AS TEXT).
🐛 Proposed fix
- JOIN liquidity_positions lp ON pp.position_id = lp.id::text
+ JOIN liquidity_positions lp ON pp.position_id = CAST(lp.id AS TEXT)As per coding guidelines: "Do not reintroduce dropped product areas such as … PostgreSQL".
Also applies to: 168-176
🤖 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 `@apps/api/src/services/auto-recenter.service.ts` around lines 71 - 79, Replace
the PostgreSQL-specific lp.id::text casts in both SQL queries within the
auto-recenter service with SQLite-compatible CAST(lp.id AS TEXT) expressions,
preserving the existing join behavior.
Source: Coding guidelines
| const outOfRangePositions = await env.DB.prepare(` | ||
| SELECT pp.id, pp.position_id, pp.pool_id, pp.user_address, pp.min_drift_bps, | ||
| lp.tick_lower, lp.tick_upper, lp.liquidity | ||
| FROM position_policies pp | ||
| JOIN liquidity_positions lp ON pp.position_id = lp.id::text | ||
| WHERE pp.chain_id = ? AND pp.is_active = 1 AND pp.policy_type = 'auto_recenter' | ||
| AND lp.is_active = 1 | ||
| `) | ||
| .bind(chainId) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
lp.id::text is PostgreSQL cast syntax and will fail on D1/SQLite.
D1 is SQLite; ::text is not valid SQLite syntax, so this prepare/all will throw and the auto-recenter enqueue step never runs (silently swallowed into the cron error log). Use CAST(lp.id AS TEXT). Also add lp.chain_id = ? to the join filter so the join can't cross chains once a second chain is indexed.
🐛 Proposed fix
FROM position_policies pp
- JOIN liquidity_positions lp ON pp.position_id = lp.id::text
- WHERE pp.chain_id = ? AND pp.is_active = 1 AND pp.policy_type = 'auto_recenter'
+ JOIN liquidity_positions lp ON pp.position_id = CAST(lp.id AS TEXT) AND lp.chain_id = pp.chain_id
+ WHERE pp.chain_id = ? AND pp.is_active = 1 AND pp.policy_type = 'auto_recenter'
AND lp.is_active = 1
`)As per coding guidelines, "Do not reintroduce dropped product areas such as ... PostgreSQL" and "Data schemas and queries must include chain_id in composite keys and filters".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const outOfRangePositions = await env.DB.prepare(` | |
| SELECT pp.id, pp.position_id, pp.pool_id, pp.user_address, pp.min_drift_bps, | |
| lp.tick_lower, lp.tick_upper, lp.liquidity | |
| FROM position_policies pp | |
| JOIN liquidity_positions lp ON pp.position_id = lp.id::text | |
| WHERE pp.chain_id = ? AND pp.is_active = 1 AND pp.policy_type = 'auto_recenter' | |
| AND lp.is_active = 1 | |
| `) | |
| .bind(chainId) | |
| const outOfRangePositions = await env.DB.prepare(` | |
| SELECT pp.id, pp.position_id, pp.pool_id, pp.user_address, pp.min_drift_bps, | |
| lp.tick_lower, lp.tick_upper, lp.liquidity | |
| FROM position_policies pp | |
| JOIN liquidity_positions lp ON pp.position_id = CAST(lp.id AS TEXT) AND lp.chain_id = pp.chain_id | |
| WHERE pp.chain_id = ? AND pp.is_active = 1 AND pp.policy_type = 'auto_recenter' | |
| AND lp.is_active = 1 | |
| `) | |
| .bind(chainId) |
🤖 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 `@apps/api/src/workers/cron-handler.ts` around lines 144 - 152, Update the
outOfRangePositions query to use SQLite-compatible CAST(lp.id AS TEXT) instead
of PostgreSQL’s lp.id::text, and add lp.chain_id = ? to the join/filter
conditions. Bind the additional chain ID parameter in the existing chainId query
preparation while preserving the current auto-recenter selection behavior.
Source: Coding guidelines
| if (t0?.priceUsd && t1?.priceUsd && t0.priceUsd > 0 && t1.priceUsd > 0) { | ||
| // Pool spot price = token0 price / token1 price (1e18-scaled) | ||
| const spotPriceX18 = String(Math.round((t0.priceUsd / t1.priceUsd) * 1e18)) | ||
| await env.CACHE.put( | ||
| `poolSpot:${pool.pool_id}`, | ||
| JSON.stringify({ price: spotPriceX18, updatedAt: Date.now() }), | ||
| { expirationTtl: 120 }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Float-scaled spot price can serialize in exponential notation and break the consumer's BigInt() parse.
Math.round(ratio * 1e18) produces a Number; once the value reaches 1e21 (ratio ≥ 1000, e.g. any pool priced against a stablecoin-like token1), String(...) yields "1e+21" and BigInt(spotPriceX18) in handleTpSlEvaluate/handleAutoRecenterCheck throws, causing permanent retry/DLQ for every order on that pool. Below that threshold you still lose ~15 digits of mantissa precision. Build the scaled value in BigInt space instead.
🐛 Proposed fix
- if (t0?.priceUsd && t1?.priceUsd && t0.priceUsd > 0 && t1.priceUsd > 0) {
- // Pool spot price = token0 price / token1 price (1e18-scaled)
- const spotPriceX18 = String(Math.round((t0.priceUsd / t1.priceUsd) * 1e18))
+ if (t0?.priceUsd && t1?.priceUsd && t0.priceUsd > 0 && t1.priceUsd > 0) {
+ // Pool spot price = token0 price / token1 price (1e18-scaled, integer-safe)
+ const SCALE = 1_000_000_000n // 1e9 fixed-point for each side
+ const num = BigInt(Math.round(t0.priceUsd * 1e9))
+ const den = BigInt(Math.round(t1.priceUsd * 1e9))
+ if (den === 0n) continue
+ const spotPriceX18 = ((num * 10n ** 18n) / den / SCALE) * SCALE === 0n
+ ? String((num * 10n ** 18n) / den)
+ : String((num * 10n ** 18n) / den)Simplest form: String((BigInt(Math.round(t0.priceUsd * 1e9)) * 10n ** 18n) / BigInt(Math.round(t1.priceUsd * 1e9)) / 10n ** 9n).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (t0?.priceUsd && t1?.priceUsd && t0.priceUsd > 0 && t1.priceUsd > 0) { | |
| // Pool spot price = token0 price / token1 price (1e18-scaled) | |
| const spotPriceX18 = String(Math.round((t0.priceUsd / t1.priceUsd) * 1e18)) | |
| await env.CACHE.put( | |
| `poolSpot:${pool.pool_id}`, | |
| JSON.stringify({ price: spotPriceX18, updatedAt: Date.now() }), | |
| { expirationTtl: 120 }, | |
| ) | |
| } | |
| if (t0?.priceUsd && t1?.priceUsd && t0.priceUsd > 0 && t1.priceUsd > 0) { | |
| // Pool spot price = token0 price / token1 price (1e18-scaled, integer-safe) | |
| const SCALE = 1_000_000_000n // 1e9 fixed-point for each side | |
| const num = BigInt(Math.round(t0.priceUsd * 1e9)) | |
| const den = BigInt(Math.round(t1.priceUsd * 1e9)) | |
| if (den === 0n) continue | |
| const spotPriceX18 = ((num * 10n ** 18n) / den / SCALE) * SCALE === 0n | |
| ? String((num * 10n ** 18n) / den) | |
| : String((num * 10n ** 18n) / den) | |
| await env.CACHE.put( | |
| `poolSpot:${pool.pool_id}`, | |
| JSON.stringify({ price: spotPriceX18, updatedAt: Date.now() }), | |
| { expirationTtl: 120 }, | |
| ) | |
| } |
🤖 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 `@apps/api/src/workers/cron-handler.ts` around lines 225 - 233, Update the
spot-price calculation in the cron handler around spotPriceX18 to avoid
Number-based scaling and exponential serialization. Convert each positive USD
price to a fixed-precision integer, perform the ratio and 1e18 scaling using
BigInt arithmetic, then stringify the resulting BigInt so consumers such as
handleTpSlEvaluate and handleAutoRecenterCheck can parse it reliably.
| /// - Dual trigger: both spot price AND TWAP must breach the trigger (anti-flash-loan) | ||
| /// - Slippage cap: maximum 5% slippage on execution | ||
| /// - Expiry: orders expire after a configurable deadline | ||
| /// - Non-custodial: this contract never holds user funds between transactions |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Deposit/withdraw model contradicts the non-custodial mandate.
The contract's own docstring says this contract never holds user funds between transactions‑equivalent intent, yet depositedBalances plus deposit()/withdraw() is exactly an escrow: tokens sit in the contract from deposit() until executeOrder() or withdraw() is called, for an indefinite period. This is the kind of "code that takes custody of user funds" the coding guidelines explicitly disallow.
Consider redesigning to a pull-based model where executeOrder pulls amountIn from the owner via safeTransferFrom (relying on a standing ERC20 approval) at execution time instead of pre-depositing, so the contract never custodies funds between transactions.
As per coding guidelines, "Keep the product non-custodial: never introduce ERC4626 vaults, position baskets, escrow, or code that takes custody of user funds."
Also applies to: 92-93, 210-231
🤖 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/contracts/src/tpsl/AetherTPSL.sol` at line 33, Redesign AetherTPSL
to remove custodial balances: eliminate depositedBalances and the
deposit()/withdraw() escrow flow, and update executeOrder() to pull amountIn
from the order owner with safeTransferFrom at execution time using the existing
approval model. Ensure the contract holds no user funds between transactions and
update the related documentation and affected order logic accordingly.
Source: Coding guidelines
| SwapParams memory swapParams = SwapParams({ | ||
| zeroForOne: order.zeroForOne, | ||
| amountSpecified: int256(int128(order.amountIn)), | ||
| sqrtPriceLimitX96: _sqrtPriceLimit(order.zeroForOne) | ||
| }); | ||
|
|
||
| bytes memory result = poolManager.unlock( | ||
| abi.encode(Action.SWAP_EXACT_IN, abi.encode(order.poolKey, swapParams, hookData)) | ||
| ); | ||
|
|
||
| BalanceDelta delta = abi.decode(result, (BalanceDelta)); | ||
| uint256 amountOut = order.zeroForOne | ||
| ? uint256(int256(delta.amount1())) | ||
| : uint256(int256(delta.amount0())); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | grep -F 'packages/contracts/src/tpsl/AetherTPSL.sol' || true
echo "== file outline =="
ast-grep outline packages/contracts/src/tpsl/AetherTPSL.sol --view expanded || true
echo "== relevant sections =="
sed -n '220,285p' packages/contracts/src/tpsl/AetherTPSL.sol
echo "---"
sed -n '360,420p' packages/contracts/src/tpsl/AetherTPSL.sol
echo "== swap params definitions/usages =="
rg -n "SwapParams|amountSpecified|settlement|_handleSwapExactIn|_sqrtPriceLimit|unlock\\(|SWAP_EXACT_IN|exact" packages/contracts/src packages/contracts/test -S | head -200Repository: irfndi/AetherDEX
Length of output: 16408
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== AetherRouter exact swap handlers =="
sed -n '88,165p' packages/contracts/src/router/AetherRouter.sol
sed -n '288,325p' packages/contracts/src/router/AetherRouter.sol
echo "== SwapParams source/docs in repo =="
fd -a 'PoolOperation\.sol$' . | while read -r f; do
echo "--- $f"
rg -n "enum|SwapParams|amountSpecified|exact" "$f" || true
done
echo "== TP/SL tests around executions =="
sed -n '1,220p' packages/contracts/test/unit/AetherTPSL.t.solRepository: irfndi/AetherDEX
Length of output: 13632
🌐 Web query:
Uniswap v4 PoolOperation amountSpecified exact input negative exact output positive
💡 Result:
In Uniswap v4, the amountSpecified parameter within the SwapParams struct determines the type of swap being executed through its value and sign [1][2]. For any swap operation: - Exact Input: When amountSpecified is negative, the user specifies the exact amount of input tokens they wish to provide to the pool [3][4]. The pool then calculates the resulting amount of output tokens to be returned to the user [5]. - Exact Output: When amountSpecified is positive, the user specifies the exact amount of output tokens they wish to receive from the pool [3][5]. The pool then calculates the required amount of input tokens the user must provide [5]. This parameter is a core component of the SwapParams struct defined in the Uniswap v4 core contracts: struct SwapParams { bool zeroForOne; int256 amountSpecified; uint160 sqrtPriceLimitX96; } In this structure, zeroForOne indicates the swap direction, and amountSpecified governs the swap logic based on its sign [1][2]. During the swap execution, this value is often tracked as amountSpecifiedRemaining, which is adjusted as the swap progresses through the pool's ticks [6][5].
Citations:
- 1: https://github.com/Uniswap/v4-core/blob/main/src/types/PoolOperation.sol
- 2: https://www.zealynx.io/research/protocol-deep-dives/uniswap-v4-swaps
- 3: https://deepwiki.com/Uniswap/v4-core/2.1-pool-operations
- 4: https://www.v4-by-example.org/swap
- 5: https://blockchaincampus.net/docs/UniswapV4/core-explained/PoolSwap
- 6: https://www.quillaudits.com/research/uniswap-development/uniswap-v4/swap-mechanics-in-uniswap-v4-core
Align amountSpecified with the exact-input swap semantics.
Uniswap v4 treats a negative amountSpecified as exact input and a positive value as exact output. This code encodes SWAP_EXACT_IN but passes a positive value, so _handleSwapExactIn computes amountIn = uint256int256(swapParams.amountSpecified)) and transfers that amount after a swap that may consume a different input amount. Use -int256(uint256(order.amountIn)) for amountSpecified, and derive amountIn from -swapParams.amountSpecified.
🤖 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/contracts/src/tpsl/AetherTPSL.sol` around lines 253 - 266, Update
the SwapParams construction in the exact-input swap flow to set amountSpecified
to the negative int256 representation of order.amountIn. In _handleSwapExactIn,
derive amountIn by negating swapParams.amountSpecified before converting it to
uint256, while preserving the existing SWAP_EXACT_IN action and output-delta
handling.
| pragma solidity ^0.8.31; | ||
|
|
||
| import "forge-std/Test.sol"; | ||
| import {AetherTPSL} from "src/tpsl/AetherTPSL.sol"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
fd -a 'AetherTPSL\.(t\s)?sol$' . || true
echo "== relevant snippets for test file =="
if [ -f packages/contracts/test/unit/AetherTPSL.t.sol ]; then
sed -n '1,380p' packages/contracts/test/unit/AetherTPSL.t.sol | nl -ba | sed -n '1,200p'
echo "--- lines 300-365 ---"
sed -n '300,365p' packages/contracts/test/unit/AetherTPSL.t.sol | nl -ba -v300
fi
echo "== AetherTPSL contract outline/slices =="
if [ -f packages/contracts/src/tpsl/AetherTPSL.sol ]; then
wc -l packages/contracts/src/tpsl/AetherTPSL.sol
ast-grep outline packages/contracts/src/tpsl/AetherTPSL.sol || true
sed -n '1,220p' packages/contracts/src/tpsl/AetherTPSL.sol | nl -ba
fi
echo "== CreateOrderParams occurrences =="
rg -n "CreateOrderParams|struct Order|Order\(" packages/contracts/src/tpsl/AetherTPSL.sol packages/contracts/test/unit/AetherTPSL.t.sol || trueRepository: irfndi/AetherDEX
Length of output: 306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files 'packages/contracts/**/AetherTPSL*.sol'
echo "== test file first 380 lines =="
python3 - <<'PY'
from pathlib import Path
p = Path("packages/contracts/test/unit/AetherTPSL.t.sol")
if p.exists():
for i, line in enumerate(p.read_text().splitlines()[:380], 1):
print(f"{i:4d}\t{line}")
PY
echo "== AetherTPSL contract first 220 lines =="
python3 - <<'PY'
from pathlib import Path
p = Path("packages/contracts/src/tpsl/AetherTPSL.sol")
if p.exists():
for i, line in enumerate(p.read_text().splitlines()[:220], 1):
print(f"{i:4d}\t{line}")
PY
echo "== CreateOrderParams occurrences =="
python3 - <<'PY'
from pathlib import Path
for p in [Path("packages/contracts/src/tpsl/AetherTPSL.sol"), Path("packages/contracts/test/unit/AetherTPSL.t.sol")]:
print(f"--- {p} ---")
if p.exists():
for i, line in enumerate(p.read_text().splitlines(), 1):
if "CreateOrderParams" in line or "struct Order" in line:
print(f"{i:4d}\t{line}")
PYRepository: irfndi/AetherDEX
Length of output: 24660
🌐 Web query:
Solidity nested struct type import contract referenced outside source file without qualification
💡 Result:
In Solidity, when a struct is defined inside a contract, it is considered nested within that contract's namespace. If you need to reference that struct from a different file or contract, you must qualify the struct name with the contract name (e.g., ContractName.StructName) [1][2]. If you attempt to reference such a nested struct without qualification (using only the struct name), the Solidity compiler will not be able to resolve the type, resulting in a compilation error [1][2]. To resolve this: 1. Import the source file containing the contract where the struct is defined [3][4]. 2. Use the syntax ContractName.StructName to refer to the nested struct in your code [1][2]. Note that this issue is distinct from top-level structs. Structs defined at the file level (outside of any contract) are available globally once the file is imported and do not require qualification [3][5]. If you find yourself frequently needing to reference a struct across multiple contracts, defining it at the top level of a dedicated library or interface file is often preferred to avoid the need for qualification [3]. Additionally, some static analysis tools (like older versions of Slither) have historically had difficulty parsing qualified struct references [1], but this is a tool-specific limitation rather than a language requirement. Always ensure your syntax follows the Solidity requirement of explicit qualification for nested types [1][2].
Citations:
- 1: [Bug]: Slither does not recognize Contract Structs when referenced in other contracts crytic/slither#1985
- 2: Aliased imports may not be working argotorg/solidity#5305
- 3: https://www.alchemy.com/overviews/solidity-struct
- 4: https://solstep.gitbook.io/solidity-steps/step-3/24-structs
- 5: https://docs.solidity.org/en/latest/grammar.html
Use AetherTPSL.CreateOrderParams in the test helpers.
CreateOrderParams is a nested AetherTPSL struct, and this test contract only imports the contract namespace, not a top-level type alias. Replace the unqualified CreateOrderParams references at lines 127, 136, 145, 154, 163, 290, 328, and 342 with AetherTPSL.CreateOrderParams so the helpers resolve.
🐛 Proposed fix
- function _defaultParams() internal view returns (CreateOrderParams memory) {
- return CreateOrderParams({
+ function _defaultParams() internal view returns (AetherTPSL.CreateOrderParams memory) {
+ return AetherTPSL.CreateOrderParams({Apply the same qualification to the other CreateOrderParams local declarations/return 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/contracts/test/unit/AetherTPSL.t.sol` at line 5, Update the test
helpers in AetherTPSL.t.sol to qualify every CreateOrderParams reference with
the imported AetherTPSL namespace, including local declarations and return
types, so they use AetherTPSL.CreateOrderParams consistently.
| assertEq(orderId, 0, "First order ID should be 0"); | ||
| assertEq(tpsl.nextOrderId(), 1, "Next order ID should be 1"); | ||
|
|
||
| (, address owner, AetherTPSL.OrderType orderType,, bool zeroForOne,,, uint256 triggerPrice,, AetherTPSL.OrderStatus status,,) = tpsl.orders(orderId); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'AetherTPSL\.t\.sol$|AetherTPSL\.sol$|AetherTPSL' . | sed 's#^\./##' | head -50
echo
echo "== file outline/size =="
if [ -f packages/contracts/test/unit/AetherTPSL.t.sol ]; then
wc -l packages/contracts/test/unit/AetherTPSL.t.sol
sed -n '1,260p' packages/contracts/test/unit/AetherTPSL.t.sol
fi
echo
echo "== related aether tpsl implementation/search =="
rg -n "TpSlOrder|TpSl|AetherTPSL|orders\\(|triggerPriceX18|twapWindow|slippageBps|deadline" packages/contracts -S --glob '*.sol' --glob '*.t.sol' | head -200Repository: irfndi/AetherDEX
Length of output: 19171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== AetherTPSL.sol relevant definitions =="
sed -n '48,90p' packages/contracts/src/tpsl/AetherTPSL.sol
echo
echo "== AetherTPSL.sol events and call sites =="
sed -n '88,190p' packages/contracts/src/tpsl/AetherTPSL.sol
echo
echo "== read-only tuple-length/type verifier =="
python3 - <<'PY'
from pathlib import Path
import re
struct_re = re.compile(r'struct TpSlOrder \{(?P<body>.*?)\n\s*\}', re.S)
maps = dict(re.findall(r'mapping\(.*\) \s* public \s* (?P<name>\w+)\s*;', str(Path('packages/contracts/src/tpsl/AetherTPSL.sol').read_text())))
text = Path('packages/contracts/test/unit/AetherTPSL.t.sol').read_text()
def field_index(name, line):
return line.index(f', {name},') if f', {name},' in line else line.index(f'{name}, ') if f'{name}, ' in line else None
for kind in ['tpsl.orders(orderId)']:
lines = sorted({m.start()==117:'line 117', m.start()==209:'line 209'}.items(), key=lambda x:x[0])
for line_no in (117, 209):
line = text.splitlines()[line_no-1]
print(f'\n--- line {line_no} ---')
print(line)
# Count explicit commas, accounting for empty fields as consecutive commas.
# Solidity tuple length = number of fields = commas + 1.
fields = [f.strip() for f in line.split(',') if f.strip()]
# Better: count empty placeholders as explicit commas with at least one other valid expression total
full = line[line.index('('):line.rindex(')')+1]
parens = 0
commas = 0
seen_expr = False
for ch in full:
if ch == '(':
parens += 1
elif ch == ')':
parens -= 1
elif ch == ',' and parens == 0:
commas += 1
elif ch not in ' \t':
seen_expr = True
count_commas = None
for i,ch in enumerate(full):
pass
fields = full.strip().strip('()').split(',')
placeholder_count = sum(1 for f in fields if f.strip() == '')
fields_with_types = [f.strip() for f in fields if f.strip() != '']
print(f'tuple field count: {len(fields)}')
print(f'empty placeholders: {placeholder_count}')
print(f'non-empty field expressions: {len(fields_with_types)}')
print(f'non-empty fields: {fields_with_types}')
PYRepository: irfndi/AetherDEX
Length of output: 5589
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
text = Path('packages/contracts/test/unit/AetherTPSL.t.sol').read_text().splitlines()
for line_no in (117, 209):
line = text[line_no - 1]
start = line.index('(')
end = line.rindex(')')
full = line[start:end + 1]
fields = [f.strip() for f in full.strip('()').split(',')]
# count all comma-separated fields including placeholders
count = 0
comma_pos = -1
while True:
next_comma = full.find(',', comma_pos + 1)
if next_comma == -1:
break
left = full[comma_pos + 1:next_comma]
right = full[next_comma + 1:]
# Solidity tuples allow leading empty field only via (), but this already has () in line
count += 1
comma_pos = next_comma
# include last field after final comma
if full.count(',') > 0:
count += 1
print(f'--- line {line_no} ---')
print(f'fields={fields}')
print(f'field_count={count}')
print(line)
PY
echo
echo "== exact tuple parsing =="
rg -n 'orders\(orderId\)\);' packages/contracts/test/unit/AetherTPSL.t.sol -C 2Repository: irfndi/AetherDEX
Length of output: 763
Fix the tuple destructuring arities to match TpSlOrder.
orders(orderId) returns 14 fields, but the destructures at lines 117 and 209 declare 12 and 10 fields, so these tests will not compile. Add the missing placeholders after triggerPrice / through createdAt before status, or use an explicit getOrder(uint256) accessor.
🤖 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/contracts/test/unit/AetherTPSL.t.sol` at line 117, Update the tuple
destructuring in the tests around tpsl.orders(orderId) at both affected
locations to match all 14 fields returned by TpSlOrder. Add placeholders for the
omitted fields, especially between triggerPrice and status and through
createdAt, while preserving the existing owner, orderType, zeroForOne,
triggerPrice, and status bindings; alternatively, use the explicit
getOrder(uint256) accessor.
1 participant
Summary
Phase 2 implementation of V4-native automation for AetherDEX:
Contracts
API Services
Workers
Infrastructure
Known Limitations
Summary by CodeRabbit
New Features
Bug Fixes
Tests