fix(swap-service): correct Mayachain affiliate fee asset (CACAO) and value by native precision#47
Merged
Conversation
…sset MayaChain, like Thorchain, collects the affiliate fee in its native asset (CACAO), reported by the shared Midgard verifier as feeOut.coins[0].amount. The fee-asset strategy mapped Mayachain to 'sell_asset', so the stored affiliateFeeAssetId was the sell asset while the amount was CACAO base units. resolveActualFeeUsd then priced the CACAO amount with the sell asset's price and precision, producing wildly wrong USD fees (observed $39,020 on a $100 swap, which flowed into /stats and would have driven a ~$29k erroneous payout). Map Mayachain to mayachainAssetId, mirroring Thorchain's thorchainAssetId. New Maya swaps now fall back to the bps-implied fee (CACAO is neither sell nor buy, so precision is unknown) exactly like Thorchain — bounded and correct. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drafted one-off backfill to correct pre-fix MayaChain swaps whose affiliateFeeAssetId was set to the sell asset instead of CACAO. For each swap it reads the Midgard action (block height + CACAO affiliate fee out), fetches the ETH.USDC pool at that height for historical CACAO/USD, and writes: affiliateFeeAssetId = CACAO actualAffiliateFeeAmountCryptoBaseUnit = Midgard 1e8 amount → CACAO native 1e10 affiliateAssetUsd = historical CACAO/USD Dry-run by default; --apply to write. Not yet applied. The backfilled affiliateAssetUsd/amount are correct but remain unused until resolveActualFeeUsd resolves the native fee-asset precision (deferred); until then Maya fees fall back to the bps-implied value, which is bounded and correct. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
resolveActualFeeUsd previously returned null when the affiliate fee asset was neither the sell nor buy asset, so the actual on-chain fee for Thorchain (RUNE) and Maya (CACAO) was never valued — affiliateAssetUsd was a fetched-but-dead column. Add explicit cases for thorchainAssetId (precision 8) and mayachainAssetId (precision 10), pricing the stored amount with affiliateAssetUsd. The stored amount must be in native base units for those precisions. Midgard reports in 1e8, so the shared verifier now scales the affiliate fee to the fee asset's native precision (RUNE 8 = no-op, CACAO 10 = x100). Historical Maya rows are corrected by scripts/backfill-maya-fee-asset.ts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Maya fee-asset backfill has been applied; the one-off script (and its yarn entry / tsconfig include) is no longer needed. Also reformats verifyThorchain config formatting in the Midgard verifier. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates Mayachain affiliate fee asset resolution, fee USD calculation for native THORChain/Mayachain assets, and Midgard fee amount conversion so affiliate fee values use chain-specific precision. ChangesAffiliate fee precision fix
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SwapVerificationService
participant verifyMidgardSwap
participant thorchainToNativePrecision
participant resolveActualFeeUsd
SwapVerificationService->>verifyMidgardSwap: verify swap (feeAssetPrecision=8/10)
verifyMidgardSwap->>thorchainToNativePrecision: convert feeOut coin amount
thorchainToNativePrecision-->>verifyMidgardSwap: actualAffiliateFeeAmountCryptoBaseUnit
verifyMidgardSwap-->>SwapVerificationService: verification result with fee amount
SwapVerificationService->>resolveActualFeeUsd: resolve affiliate fee USD
resolveActualFeeUsd->>resolveActualFeeUsd: switch on affiliateFeeAssetId
resolveActualFeeUsd-->>SwapVerificationService: fee USD (with correct precision)
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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: 1
🤖 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/swap-service/src/verification/swap-verification.service.ts`:
- Around line 393-396: `verifySwap()` in `swap-verification.service.ts` should
preserve the missing-coin guard when computing
`actualAffiliateFeeAmountCryptoBaseUnit`; the current `feeOut.coins[0].amount`
access can throw if `feeOut` has no first coin. Update the affiliate-fee mapping
so it only calls `thorchainToNativePrecision` when `hasAffiliate`, `feeOut`, and
`feeOut.coins[0]` are all present, otherwise leave the field `undefined`. Use
the existing `verifySwap` affiliate-fee block and
`feeOut`/`thorchainToNativePrecision` symbols to locate the change.
🪄 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
Run ID: 1174e7b6-9fb6-4197-a1b9-5cfd74a46e5d
📒 Files selected for processing (4)
apps/swap-service/src/swaps/utils.tsapps/swap-service/src/utils/affiliateFeeAsset.tsapps/swap-service/src/verification/__tests__/maya.test.tsapps/swap-service/src/verification/swap-verification.service.ts
Coins array is non-empty per Midgard types, but guard against an empty array defensively so a malformed affiliate out leaves the fee amount undefined instead of throwing and dropping the swap to PENDING. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
MayaChain collects the affiliate fee in its native asset CACAO (like THORChain collects RUNE), but the fee-asset strategy mapped
Mayachainto'sell_asset'. So swaps storedaffiliateFeeAssetId= the sell asset whileactualAffiliateFeeAmountCryptoBaseUnitheld a CACAO amount.resolveActualFeeUsdthen priced that CACAO amount with the sell asset's price/precision, producing wildly wrong USD fees — observed $39,020 on a $100 swap (and a $57,712 row), which fed straight into the affiliate/statstotals and would have driven an erroneous payout.Changes:
Mayachain → mayachainAssetId(CACAO), mirroringThorchain → thorchainAssetId(RUNE).resolveActualFeeUsdnow has explicit cases for the native fee assets — RUNE (precision 8) and CACAO (precision 10) — pricing the on-chain amount viaaffiliateAssetUsd(previously a fetched-but-unused column). Implemented as aswitchonaffiliateFeeAssetId.ETH.USDCpool at each swap's block height). The script was applied and then removed.Note: THORChain now values its actual RUNE fee instead of falling back to bps-implied — more accurate, but existing THORChain
/statsnumbers will shift slightly.Linear: https://linear.app/shapeshift-dao/issue/SS-5709/mayachain-fee-inconsistent (SS-5709)
Closes #46
Testing
423777900000).calculateFeeForSwap: the $39,020 and $57,712 garbage rows collapse to ~$0.40 / ~$0.60, and every backfilled actual fee lands within <0.25% of its bps-implied value (well inside the payout deviation guard).Summary by CodeRabbit