johba/harb
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: parse slot0 response as array — viem v2 compat (#2) #3

Merged
dev-qwen merged 1 commit from fix/issue-2 into master 2026-04-05 16:54:01 +00:00
Conversation 5 Commits 1 Files changed 1 +3 -1
johba commented 2026-04-05 16:14:59 +00:00
Owner
Copy link

Closes #2

Summary

  • viem v2 returns multi-output contract calls as plain arrays, not named objects
  • tick is at index , not property
  • Handles both array and record shapes for forward compatibility

Test plan

  • Open /app/cheats — Liquidity Snapshot should show current tick and positions
  • Refresh button should work without errors
Closes #2 ## Summary - viem v2 returns multi-output contract calls as plain arrays, not named objects - tick is at index , not property - Handles both array and record shapes for forward compatibility ## Test plan - Open /app/cheats — Liquidity Snapshot should show current tick and positions - Refresh button should work without errors
johba added 1 commit 2026-04-05 16:15:00 +00:00 
viem v2 readContract returns multi-output functions as plain arrays.
slot0() tick is at index [1], not .tick property. Handles both array
and record shapes for forward compatibility.
dev-qwen closed this pull request 2026-04-05 16:18:51 +00:00
johba reopened this pull request 2026-04-05 16:22:18 +00:00
review-bot commented 2026-04-05 16:47:47 +00:00
Collaborator
Copy link

AI Review

Summary

Small, focused bug fix. viem v2 decodes multi-output contract calls as plain arrays; the ABI confirms tick is output index 1 ([sqrtPriceX96, tick, observationIndex, ...]). The fix is correct.

Findings

None. The implementation is sound:

  • slot0Response[1] correctly targets tick per UNISWAP_POOL_ABI (useSwapKrk.ts:75)
  • Number() coercion is safe — int24 range (−8,388,608 to 8,388,607) is well within JS safe integers
  • The record-shape fallback is a reasonable defensive guard for forward compatibility
  • Error is thrown for any other shape, so failures are explicit rather than silent

Verdict

APPROVE — Correct viem v2 fix: tick is at array index 1 per the ABI, Number() coercion is safe for int24 range, fallback preserves compatibility.

Reviewed at 0eb1816 | AGENTS.md

## AI Review <!-- reviewed: 0eb1816c22cc9b18313c8af6e281bd0aad4ebea5 --> ### Summary Small, focused bug fix. viem v2 decodes multi-output contract calls as plain arrays; the ABI confirms `tick` is output index 1 (`[sqrtPriceX96, tick, observationIndex, ...]`). The fix is correct. ### Findings None. The implementation is sound: - `slot0Response[1]` correctly targets `tick` per `UNISWAP_POOL_ABI` (`useSwapKrk.ts:75`) - `Number()` coercion is safe — `int24` range (−8,388,608 to 8,388,607) is well within JS safe integers - The record-shape fallback is a reasonable defensive guard for forward compatibility - Error is thrown for any other shape, so failures are explicit rather than silent ### Verdict **APPROVE** — Correct viem v2 fix: tick is at array index 1 per the ABI, Number() coercion is safe for int24 range, fallback preserves compatibility. --- *Reviewed at `0eb1816` | [AGENTS.md](AGENTS.md)*
review-bot approved these changes 2026-04-05 16:47:47 +00:00
Dismissed
review-bot left a comment
Collaborator
Copy link

AI Review: APPROVE — Correct viem v2 fix: tick is at array index 1 per the ABI, Number() coercion is safe for int24 range, fallback preserves compatibility.

AI Review: **APPROVE** — Correct viem v2 fix: tick is at array index 1 per the ABI, Number() coercion is safe for int24 range, fallback preserves compatibility.
review-bot commented 2026-04-05 16:47:51 +00:00
Collaborator
Copy link

AI Review

Summary\n\nSmall, focused fix for viem v2's breaking change: readContract now returns multi-output calls as plain tuples rather than named objects.\n\n### Correctness\n\nVerified against UNISWAP_POOL_ABI in useSwapKrk.ts:\n\n\noutputs[0] = sqrtPriceX96 (uint160)\noutputs[1] = tick (int24)\n\n\nIndex 1 is correct. Number(slot0Response[1]) is safe — int24 is well within JavaScript safe-integer range regardless of whether viem surfaces it as number or bigint.\n\n### Code quality\n\n- Changing currentTick from number | null to number (throwing instead of leaving null) is a cleaner type.\n- The comment explaining the viem v2 tuple layout is helpful and accurate.\n- The isRecord fallback for named-object shape is a reasonable forward-compat guard.\n\nNo issues found.

Verdict

APPROVE — Correct array-index fix for viem v2 tuple return; fallback preserves forward compat

Reviewed at 0eb1816 | AGENTS.md

## AI Review <!-- reviewed: 0eb1816c22cc9b18313c8af6e281bd0aad4ebea5 --> ### Summary\n\nSmall, focused fix for viem v2's breaking change: `readContract` now returns multi-output calls as plain tuples rather than named objects.\n\n### Correctness\n\nVerified against `UNISWAP_POOL_ABI` in `useSwapKrk.ts`:\n\n```\noutputs[0] = sqrtPriceX96 (uint160)\noutputs[1] = tick (int24)\n```\n\nIndex 1 is correct. `Number(slot0Response[1])` is safe — `int24` is well within JavaScript safe-integer range regardless of whether viem surfaces it as `number` or `bigint`.\n\n### Code quality\n\n- Changing `currentTick` from `number | null` to `number` (throwing instead of leaving `null`) is a cleaner type.\n- The comment explaining the viem v2 tuple layout is helpful and accurate.\n- The `isRecord` fallback for named-object shape is a reasonable forward-compat guard.\n\nNo issues found. ### Verdict **APPROVE** — Correct array-index fix for viem v2 tuple return; fallback preserves forward compat --- *Reviewed at `0eb1816` | [AGENTS.md](AGENTS.md)*
review-bot approved these changes 2026-04-05 16:47:51 +00:00
Dismissed
review-bot left a comment
Collaborator
Copy link

AI Review: APPROVE — Correct array-index fix for viem v2 tuple return; fallback preserves forward compat

AI Review: **APPROVE** — Correct array-index fix for viem v2 tuple return; fallback preserves forward compat
dev-qwen merged commit 41a3d52319 into master 2026-04-05 16:54:01 +00:00
dev-qwen deleted branch fix/issue-2 2026-04-05 16:54:01 +00:00
review-bot commented 2026-04-05 16:54:07 +00:00
Collaborator
Copy link

AI Re-review (round 3)

Previous Findings\n\nNo findings in the previous review — it was an APPROVE at 0eb1816. The author pushed additional commits after approval, and this re-review covers those incremental changes.\n\n### Incremental Changes Assessment\n\nThe incremental diff (0eb1816 → f1e4c37) made the following changes to already-approved code:\n\n#### 1. typeof slot0Response[1] === 'number' replaces Number(slot0Response[1])\n\n- medium CheatsView.vue:559: The approved version used Number(slot0Response[1]), which the previous review explicitly noted was safe "regardless of whether viem surfaces it as number or bigint". The new typeof === 'number' guard silently rejects a bigint value — if viem returns int24 as bigint (or ever changes to do so), the condition is false, currentTick stays null, and the function throws "Unable to determine current pool tick" — the exact bug this PR was meant to fix. In practice viem 2.x maps int24 to number, so it works today, but the guard is now fragile. Restore Number(slot0Response[1]) (or check typeof === 'number' || typeof === 'bigint') to be safe.\n\n#### 2. Explanatory comment removed\n\n- minor CheatsView.vue:558: The approved version included:\n \n This context is valuable for the next person touching this code. Removing it leaves a bare index access with no explanation of the tuple layout or the viem v2 reason for it.\n\n#### 3. Type regressed from number to number | null\n\n- minor CheatsView.vue:558: The previous review praised let currentTick: number (with inline throw) as cleaner. The revert to number | null is functionally equivalent (null check throws on the next line), but is a step backward in type clarity for no gain.\n\n### Summary\n\nThe core Array.isArray fix is correct and the index is right. But the incremental changes after approval made the code less robust and less readable than the version that was approved. Recommend restoring Number() coercion and the viem v2 comment, and preferring let currentTick: number with the throw in the else branch.

Verdict

REQUEST_CHANGES — Incremental commits regressed from the approved version: dropped bigint coercion safety and removed the explanatory comment

Reviewed at f1e4c37 | Previous: 0eb1816 | AGENTS.md

## AI Re-review (round 3) <!-- reviewed: f1e4c3772ef3a20fd13c42b5ed5bb41286f8a6de --> ### Previous Findings\n\nNo findings in the previous review — it was an APPROVE at `0eb1816`. The author pushed additional commits after approval, and this re-review covers those incremental changes.\n\n### Incremental Changes Assessment\n\nThe incremental diff (`0eb1816` → `f1e4c37`) made the following changes to already-approved code:\n\n#### 1. `typeof slot0Response[1] === 'number'` replaces `Number(slot0Response[1])`\n\n- **medium** `CheatsView.vue:559`: The approved version used `Number(slot0Response[1])`, which the previous review explicitly noted was safe *"regardless of whether viem surfaces it as `number` or `bigint`"*. The new `typeof === 'number'` guard silently rejects a `bigint` value — if viem returns `int24` as `bigint` (or ever changes to do so), the condition is false, `currentTick` stays `null`, and the function throws `"Unable to determine current pool tick"` — the exact bug this PR was meant to fix. In practice viem 2.x maps `int24` to `number`, so it works today, but the guard is now fragile. Restore `Number(slot0Response[1])` (or check `typeof === 'number' || typeof === 'bigint'`) to be safe.\n\n#### 2. Explanatory comment removed\n\n- **minor** `CheatsView.vue:558`: The approved version included:\n \n This context is valuable for the next person touching this code. Removing it leaves a bare index access with no explanation of the tuple layout or the viem v2 reason for it.\n\n#### 3. Type regressed from `number` to `number | null`\n\n- **minor** `CheatsView.vue:558`: The previous review praised `let currentTick: number` (with inline throw) as cleaner. The revert to `number | null` is functionally equivalent (null check throws on the next line), but is a step backward in type clarity for no gain.\n\n### Summary\n\nThe core `Array.isArray` fix is correct and the index is right. But the incremental changes after approval made the code less robust and less readable than the version that was approved. Recommend restoring `Number()` coercion and the viem v2 comment, and preferring `let currentTick: number` with the throw in the `else` branch. ### Verdict **REQUEST_CHANGES** — Incremental commits regressed from the approved version: dropped bigint coercion safety and removed the explanatory comment --- *Reviewed at `f1e4c37` | Previous: `0eb1816` | [AGENTS.md](AGENTS.md)*
review-bot requested changes 2026-04-05 16:54:08 +00:00
review-bot left a comment
Collaborator
Copy link

AI Re-review (round 3): REQUEST_CHANGES — Incremental commits regressed from the approved version: dropped bigint coercion safety and removed the explanatory comment

AI Re-review (round 3): **REQUEST_CHANGES** — Incremental commits regressed from the approved version: dropped bigint coercion safety and removed the explanatory comment
Sign in to join this conversation.
Reviewers
No reviewers
review-bot
Labels
Clear labels   
action

   
backlog

   
blocked

   
in-progress

   
priority

   
tech-debt

   
underspecified

   
vision
No labels
action
backlog
blocked
in-progress
priority
tech-debt
underspecified
vision
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: johba/harb#3
No description provided.
Delete branch "fix/issue-2"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?