feat(swap-service): Mayachain swap verification#39
Merged
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR refactors THORChain and Maya swap verification to use a shared Midgard-based helper function, replacing separate legacy implementations. It includes comprehensive Maya verification test coverage with fixtures and updates test infrastructure to reflect the unified Midgard URL approach. ChangesMaya Swap Verification via Midgard Unification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/swap-service/src/verification/swap-verification.service.ts (1)
376-381:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard Midgard amount fields before nested indexing.
action.in[0].coins[0].amountandbuyOut.coins[0].amountare assumed present. If Midgard returns partial data, this throws and collapses into a generic top-level catch path instead of a deterministic verification reason.Suggested hardening
+ const inAmount = action.in?.[0]?.coins?.[0]?.amount + if (!inAmount) return noAffiliateResult('PENDING', 'Missing inbound amount in Midgard action') + + const buyAmount = buyOut.coins?.[0]?.amount + if (!buyAmount) return noAffiliateResult('PENDING', 'Missing outbound amount in Midgard action') + + const affiliateFeeAmount = feeOut?.coins?.[0]?.amount + return { verificationStatus: 'SUCCESS', hasAffiliate, affiliateBps: hasAffiliate ? parseInt(swapMetadata.affiliateFee) : undefined, affiliateAddress: hasAffiliate ? affiliateAddress : undefined, - verifiedSellAmountCryptoBaseUnit: thorchainToNativePrecision( - action.in[0].coins[0].amount, - swap.sellAsset.precision, - ), - actualBuyAmountCryptoBaseUnit: thorchainToNativePrecision(buyOut.coins[0].amount, swap.buyAsset.precision), - actualAffiliateFeeAmountCryptoBaseUnit: hasAffiliate ? feeOut?.coins[0].amount : undefined, + verifiedSellAmountCryptoBaseUnit: thorchainToNativePrecision(inAmount, swap.sellAsset.precision), + actualBuyAmountCryptoBaseUnit: thorchainToNativePrecision(buyAmount, swap.buyAsset.precision), + actualAffiliateFeeAmountCryptoBaseUnit: hasAffiliate ? affiliateFeeAmount : undefined, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/swap-service/src/verification/swap-verification.service.ts` around lines 376 - 381, The code assumes nested arrays exist when calling thorchainToNativePrecision on action.in[0].coins[0].amount and buyOut.coins[0].amount (and reads feeOut?.coins[0].amount); add explicit guards that check action.in && action.in[0] && action.in[0].coins && action.in[0].coins[0] and buyOut && buyOut.coins && buyOut.coins[0] (and feeOut?.coins?.[0]) before indexing, and if any are missing return/throw a deterministic verification error (or set the corresponding verified/actual fields to undefined in the same verification-result shape) so the verification logic produces a clear, recoverable reason instead of a generic catch; update the places using thorchainToNativePrecision and the variables verifiedSellAmountCryptoBaseUnit, actualBuyAmountCryptoBaseUnit, and actualAffiliateFeeAmountCryptoBaseUnit accordingly.
🤖 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.
Outside diff comments:
In `@apps/swap-service/src/verification/swap-verification.service.ts`:
- Around line 376-381: The code assumes nested arrays exist when calling
thorchainToNativePrecision on action.in[0].coins[0].amount and
buyOut.coins[0].amount (and reads feeOut?.coins[0].amount); add explicit guards
that check action.in && action.in[0] && action.in[0].coins &&
action.in[0].coins[0] and buyOut && buyOut.coins && buyOut.coins[0] (and
feeOut?.coins?.[0]) before indexing, and if any are missing return/throw a
deterministic verification error (or set the corresponding verified/actual
fields to undefined in the same verification-result shape) so the verification
logic produces a clear, recoverable reason instead of a generic catch; update
the places using thorchainToNativePrecision and the variables
verifiedSellAmountCryptoBaseUnit, actualBuyAmountCryptoBaseUnit, and
actualAffiliateFeeAmountCryptoBaseUnit accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1101ccd-7548-4ada-8bd9-55dfaf679262
📒 Files selected for processing (8)
apps/swap-service/src/verification/__tests__/fixtures/maya/response.jsonapps/swap-service/src/verification/__tests__/fixtures/maya/swap.tsapps/swap-service/src/verification/__tests__/maya.test.tsapps/swap-service/src/verification/__tests__/relay.test.tsapps/swap-service/src/verification/__tests__/setup.tsapps/swap-service/src/verification/__tests__/thorchain.test.tsapps/swap-service/src/verification/swap-verification.service.tsapps/swap-service/src/verification/types.ts
💤 Files with no reviewable changes (1)
- apps/swap-service/src/verification/types.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds Mayachain swap verification by extracting the Thorchain Midgard verifier into a shared
verifyMidgardSwaphelper parameterized on Midgard URL and affiliate code (ssfor Thorchain,ssmayafor Maya). The previous Maya path (node-API memo regex against/mayachain/tx/<hash>) is removed in favor of Midgard parity with Thorchain — same buy-out-by-memo-destination matching, same affiliate-out detection, same precision conversion.Also includes minor cleanup carried in alongside: generalized error strings (
Missing sell txHash,Swap action failed,No request data returned from Relay API) and renamed the test env var fromVITE_MAYACHAIN_NODE_URLtoVITE_MAYACHAIN_MIDGARD_URL.Testing
cd apps/swap-service && npx jest src/verification/__tests__/maya.test.ts— 14 tests pass, mirroringthorchain.test.tscoveragecd apps/swap-service && npx jest src/verification/__tests__/thorchain.test.ts— unchanged behavior on Thorchain side after the shared-helper extractioncd apps/swap-service && npx jest src/verification/__tests__/relay.test.ts— updated error-string assertion still passesSummary by CodeRabbit
Bug Fixes
Tests
Refactor