Skip to content

fixed wormhole WBTC deposit/withdraw issue#52

Merged
harshaalphafi merged 2 commits intomainfrom
bugfix/wormhole-wbtc-type-fixes
Mar 17, 2026
Merged

fixed wormhole WBTC deposit/withdraw issue#52
harshaalphafi merged 2 commits intomainfrom
bugfix/wormhole-wbtc-type-fixes

Conversation

@riteshR14
Copy link
Copy Markdown
Contributor

No description provided.

@riteshR14 riteshR14 requested review from 11felix and Zorag44 March 17, 2026 11:19
@riteshR14 riteshR14 requested a review from jangid as a code owner March 17, 2026 11:19
@jangid
Copy link
Copy Markdown
Contributor

jangid commented Mar 17, 2026

Code Review — PR #52 "fixed wormhole WBTC deposit/withdraw issue"

HIGH — 2 of 3 wBTC address occurrences not fixed in pool registry

Only the SUI/wBTC entry in poolMap.ts was updated from 0x27792d... to 0x027792d.... However, the same incorrect address appears in two more entries that were NOT fixed:

  • wBTC / wUSDC (Cetus) pair key
  • wBTC / USDC (Bluefin + Cetus) pair key

If 0x027792d... is the correct canonical address (which this PR asserts by fixing SUI/wBTC), then these other two entries will also fail type-based lookups. The fix should be applied consistently across all occurrences.


MEDIUM — getPoolIdByTypesAndProtocol is marked async unnecessarily

getPoolIdsByTypes is synchronous (returns ProtocolPoolIds, not a Promise), but the new method uses async/await on it. This was likely copy-pasted from getPoolIdBySymbolsAndProtocol which is legitimately async. Not a bug (await on a non-Promise resolves immediately), but misleading. Consider removing async/await or adding a comment.


LOW — Only 2 call sites changed to type-based lookup

The PR changes two getPoolIdBySymbolsAndProtocol calls in depositBluefinSuiSecondTx and withdrawBluefinSuiSecondTx to use the new type-based method, but only for the 'cetus' protocol argument. Worth confirming no other strategies have the same pattern where poolLabel.assetA.name / poolLabel.assetB.name is used with symbol-based lookup and would hit the same issue.


LOW — Error message says "or" instead of "/"

throw new Error(
    `Pool for protocol: ${protocol} not found for coin pair: ${typeA} or ${typeB}`,
);

Should be "typeA / typeB" or "typeA and typeB" since it's a pair, not an alternative.


Summary

# Severity Finding
1 HIGH 2 of 3 wBTC address occurrences not fixed in pool registry
2 MEDIUM Unnecessary async/await on synchronous call
3 LOW Other symbol-based call sites may have same issue
4 LOW Error message wording
5 INFO No regression test for the fix

@riteshR14 riteshR14 requested review from rg-alpha and sid-alphafi and removed request for Zorag44 March 17, 2026 11:38
@harshaalphafi harshaalphafi merged commit 3395dc0 into main Mar 17, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants