fix(sdk): classify sealevel cross-collateral tokens#8632
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThis PR fixes misclassification of Sealevel cross-collateral tokens during WarpCore route building, updates token classification logic, and adds a test that verifies WarpCore generates a transferRemoteTo-style route for Sealevel ↔ EVM cross-collateral pairs. Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
typescript/sdk/src/token/TokenMetadata.ts (1)
155-160: Use the central cross-collateral set here.This fixes Sealevel, but the duplicated list is how this drift happened.
TOKEN_CROSS_COLLATERAL_STANDARDSalready has EVM/Sealevel/Tron.♻️ Proposed cleanup
import { PROTOCOL_TO_HYP_NATIVE_STANDARD, PROTOCOL_TO_NATIVE_STANDARD, TOKEN_COLLATERALIZED_STANDARDS, + TOKEN_CROSS_COLLATERAL_STANDARDS, TOKEN_HYP_STANDARDS, TOKEN_MULTI_CHAIN_STANDARDS, TOKEN_NFT_STANDARDS, @@ isCrossCollateralToken(): boolean { - return ( - this.standard === TokenStandard.EvmHypCrossCollateralRouter || - this.standard === TokenStandard.TronHypCrossCollateralRouter || - this.standard === TokenStandard.SealevelHypCrossCollateral - ); + return TOKEN_CROSS_COLLATERAL_STANDARDS.has(this.standard); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@typescript/sdk/src/token/TokenMetadata.ts` around lines 155 - 160, The isCrossCollateralToken method in TokenMetadata currently hardcodes TokenStandard comparisons; replace that duplicated list with a membership check against the central TOKEN_CROSS_COLLATERAL_STANDARDS constant (e.g., use TOKEN_CROSS_COLLATERAL_STANDARDS.includes(this.standard) or .has if it's a Set) so EVM/Tron/Sealevel stay in sync with the canonical set; update the method to return the membership boolean and remove the explicit TokenStandard comparisons.typescript/sdk/src/warp/WarpCore.test.ts (1)
1190-1195: Drop theanycast.
isCrossCollateralTransferis public, so this can call it directly.♻️ Proposed cleanup
expect( - (crossCollateralWarpCore as any).isCrossCollateralTransfer( + crossCollateralWarpCore.isCrossCollateralTransfer( sealevelCrossCollateral, evmCrossCollateral, ),As per coding guidelines, No
asassertions - everyascast is a potential bug.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@typescript/sdk/src/warp/WarpCore.test.ts` around lines 1190 - 1195, The test uses an unnecessary `as any` cast when invoking the public method isCrossCollateralTransfer; remove the cast and call (crossCollateralWarpCore).isCrossCollateralTransfer(sealevelCrossCollateral, evmCrossCollateral) directly, and if TypeScript complains, fix the test variable typing for crossCollateralWarpCore (or its construction) so it is typed as the class exposing isCrossCollateralTransfer rather than forcing any. Ensure the call uses the real method signature of isCrossCollateralTransfer with sealevelCrossCollateral and evmCrossCollateral.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@typescript/sdk/src/token/TokenMetadata.ts`:
- Around line 155-160: The isCrossCollateralToken method in TokenMetadata
currently hardcodes TokenStandard comparisons; replace that duplicated list with
a membership check against the central TOKEN_CROSS_COLLATERAL_STANDARDS constant
(e.g., use TOKEN_CROSS_COLLATERAL_STANDARDS.includes(this.standard) or .has if
it's a Set) so EVM/Tron/Sealevel stay in sync with the canonical set; update the
method to return the membership boolean and remove the explicit TokenStandard
comparisons.
In `@typescript/sdk/src/warp/WarpCore.test.ts`:
- Around line 1190-1195: The test uses an unnecessary `as any` cast when
invoking the public method isCrossCollateralTransfer; remove the cast and call
(crossCollateralWarpCore).isCrossCollateralTransfer(sealevelCrossCollateral,
evmCrossCollateral) directly, and if TypeScript complains, fix the test variable
typing for crossCollateralWarpCore (or its construction) so it is typed as the
class exposing isCrossCollateralTransfer rather than forcing any. Ensure the
call uses the real method signature of isCrossCollateralTransfer with
sealevelCrossCollateral and evmCrossCollateral.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d23a1634-7f57-4a1c-aa00-86692a4a5179
📒 Files selected for processing (3)
.changeset/fix-sealevel-cross-collateral-predicate.mdtypescript/sdk/src/token/TokenMetadata.tstypescript/sdk/src/warp/WarpCore.test.ts
paulbalaji
left a comment
There was a problem hiding this comment.
Review Summary
Minimal, correct fix. Replacing the hand-maintained disjunction in TokenMetadata.isCrossCollateralToken() with a lookup against the canonical TOKEN_CROSS_COLLATERAL_STANDARDS removes the drift risk that caused the regression — now the predicate cannot silently lag behind TokenStandard.ts. Regression test is focused and asserts both the classification and the routing outcome.
Prior bot feedback (Devin, CodeRabbit) about reusing the canonical set is already resolved in head. No unresolved threads.
Related PRs
- #8629 adds the same regression test standalone without the production fix — redundant once this merges.
- #8628 adds the same regression test plus an unrelated registry version bump.
LGTM.
…oss-collateral-predicate-clean
Summary
TokenMetadata.isCrossCollateralToken()to includeTokenStandard.SealevelHypCrossCollateraltransferRemoteTopath instead of generictransferRemoteRoot cause
WarpCore.isCrossCollateralTransfer()relies onoriginToken.isCrossCollateralToken()anddestinationToken.isCrossCollateralToken().After the
TokenMetadatarefactor, currentmainclassified only:EvmHypCrossCollateralRouterTronHypCrossCollateralRouterIt omitted
SealevelHypCrossCollateral, so a Sealevel cross-collateral source token was misclassified as non-cross-collateral. That caused WarpCore to skip the cross-collateral route and fall back to the generic transfer path.Repro
On current
main, this targeted test fails:Failure before fix:
Validation
After this change:
Result:
1 passingNotes
main, not a release-only workaround.