fix(swap-service): correct Mayachain affiliate fee asset (CACAO) and value by native precision#47
Open
kaladinlight wants to merge 5 commits into
Open
fix(swap-service): correct Mayachain affiliate fee asset (CACAO) and value by native precision#47kaladinlight wants to merge 5 commits into
kaladinlight wants to merge 5 commits into
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>
📝 WalkthroughWalkthroughThis PR fixes inconsistent Mayachain affiliate fee reporting by changing the Mayachain fee asset strategy to a fixed native asset ID, adding native-asset precision handling (RUNE/CACAO) in fee USD resolution, and applying chain-specific precision conversion when computing affiliate fee amounts from Midgard data, with one test expectation updated accordingly. ChangesAffiliate fee precision fix
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)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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
Comment on lines
+393
to
+396
| actualAffiliateFeeAmountCryptoBaseUnit: | ||
| hasAffiliate && feeOut | ||
| ? thorchainToNativePrecision(feeOut.coins[0].amount, config.feeAssetPrecision) | ||
| : undefined, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep the missing-coin guard when converting the affiliate fee.
Line 395 now dereferences feeOut.coins[0].amount unconditionally. If Midgard returns an affiliate out entry with no first coin, this throws and verifySwap() falls back to PENDING instead of leaving actualAffiliateFeeAmountCryptoBaseUnit undefined like before.
Suggested fix
actualAffiliateFeeAmountCryptoBaseUnit:
- hasAffiliate && feeOut
- ? thorchainToNativePrecision(feeOut.coins[0].amount, config.feeAssetPrecision)
+ hasAffiliate && feeOut?.coins?.[0]?.amount
+ ? thorchainToNativePrecision(feeOut.coins[0].amount, config.feeAssetPrecision)
: undefined,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| actualAffiliateFeeAmountCryptoBaseUnit: | |
| hasAffiliate && feeOut | |
| ? thorchainToNativePrecision(feeOut.coins[0].amount, config.feeAssetPrecision) | |
| : undefined, | |
| actualAffiliateFeeAmountCryptoBaseUnit: | |
| hasAffiliate && feeOut?.coins?.[0]?.amount | |
| ? thorchainToNativePrecision(feeOut.coins[0].amount, config.feeAssetPrecision) | |
| : 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
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.
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