feat(swap-service): explicit bps split, partner rename, and fee math fix#41
Open
kaladinlight wants to merge 3 commits into
Open
feat(swap-service): explicit bps split, partner rename, and fee math fix#41kaladinlight wants to merge 3 commits into
kaladinlight wants to merge 3 commits into
Conversation
…Address rename Break out fee tracking into three explicit fields (affiliateBps, shapeshiftBps, partnerBps) so the invariant affiliateBps = shapeshiftBps + partnerBps holds without derivation. Rename affiliateAddress → partnerAddress to distinguish the partner's payout address from on-chain affiliate addresses in verification. - Add partnerBps column with @default(0), make receiveAddress required - Migration backfills partnerBps from existing data, fixes shapeshiftBps for non-partner swaps, renames column + index - Callers now pass all three bps values (snapshotted at quote time) to eliminate drift between quote and registration - Update DTO, service, controller, affiliate service, and test fixtures Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Make buyAccountId required — the receive address is always the buy account. Remove hashAccountId from swap service since raw addresses are already stored alongside (receiveAddress, partnerAddress) making the hashing pointless. - buyAccountId: String? → String in schema, migration backfills from receiveAddress - Remove hashAccountId usage from createSwap and getSwapsByAccountId - Update test fixtures with non-null buyAccountId Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rename getAffiliateFeeRate → getPartnerFeeRate with partner-first semantics: partner gets their agreed partnerBps share first (capped at 100%), ShapeShift absorbs the shortfall if on-chain bps is less than expected. - Use min(partnerBps / verifiedBps, 1) instead of (verifiedBps - shapeshiftBps) / verifiedBps - Clean up verifiers: remove redundant ?? undefined on affiliateBps (now always non-null) - Simplify hasAffiliate checks (affiliateBps > 0 instead of !== undefined && > 0) - Update test fixtures with raw addresses instead of hashes Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR refactors swap service addressing and fee models from affiliate-based to partner-based semantics. It updates data contracts, database schema, and core service logic to resolve, store, and aggregate swap fees using partner address and partner basis points alongside a fixed ShapeShift basis points allocation. ChangesSwap Service Partner Migration
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 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.
Actionable comments posted: 4
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)
320-327:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly return fee details for verified ShapeShift Portals orders.
When
context.partneris present but not our treasury, this branch still returnsswap.affiliateBpsandfeeAmount. That records a third-party partner fee inside ahasAffiliate: falseresult.Suggested change
return { verificationStatus: 'SUCCESS', hasAffiliate: hasShapeshiftAffiliate, - affiliateBps: swap.affiliateBps, + affiliateBps: hasShapeshiftAffiliate ? swap.affiliateBps : undefined, affiliateAddress: hasShapeshiftAffiliate ? expectedTreasuryAddress : undefined, verifiedSellAmountCryptoBaseUnit, actualBuyAmountCryptoBaseUnit: undefined, - actualAffiliateFeeAmountCryptoBaseUnit: orderData?.context?.feeAmount, + actualAffiliateFeeAmountCryptoBaseUnit: hasShapeshiftAffiliate + ? orderData?.context?.feeAmount + : 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 320 - 327, The returned object is including affiliate fields even when hasShapeshiftAffiliate is false; update the return in the verification result so that swap.affiliateBps, affiliateAddress (expectedTreasuryAddress) and actualAffiliateFeeAmountCryptoBaseUnit (orderData?.context?.feeAmount) are only set when hasShapeshiftAffiliate is true—otherwise omit or set them to undefined and ensure hasAffiliate reflects hasShapeshiftAffiliate; locate the return object in the function that computes hasShapeshiftAffiliate and adjust those properties conditionally.
🤖 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/affiliate/affiliate.service.ts`:
- Around line 151-155: The return currently exposes the legacy total fee
(affiliate.bps) as partnerBps; change resolvePartnerCode() to compute the
partner's share by subtracting the SHAPESHIFT_BPS from the stored affiliate.bps
(e.g., partnerBps = Math.max(0, affiliate.bps - SHAPESHIFT_BPS)) so callers get
the partner portion, not the unsplit total; keep
createAffiliate()/updateAffiliate() storing the single legacy bps field but
ensure any returned partnerBps uses this subtraction and guards against negative
values.
In `@apps/swap-service/src/swaps/swaps.service.ts`:
- Around line 220-221: getSwapsByAccountId currently only matches raw
sellAccountId/buyAccountId which drops legacy rows stored with hashed account
ids; update getSwapsByAccountId to include a legacy lookup path before removing
migration support by extending the predicate passed to paginateSwaps to OR also
match legacy columns (e.g., sellAccountIdHash and buyAccountIdHash) using the
same hash of the incoming accountId, and additionally include the pre-migration
receiveAddress match for buy-side lookups (i.e., OR { buyAccountId: accountId }
or { receiveAddress: accountId } and OR { sellAccountIdHash: hash(accountId) } /
{ buyAccountIdHash: hash(accountId) } ), using the existing paginateSwaps helper
and the same hashing utility used elsewhere so historical swaps continue to be
returned during migration.
In `@packages/shared-types/src/index.ts`:
- Around line 77-82: CreateSwapDto (and any DTO/type in this file that declares
affiliateBps, shapeshiftBps, isStreaming, metadata, partnerAddress, partnerBps)
currently marks partnerBps as optional; change partnerBps to be required (remove
the optional marker) so callers must pass it at quote time. Update the type
declaration for partnerBps in the CreateSwapDto/interface in this file and then
search for any call sites or tests that construct CreateSwapDto to ensure they
supply partnerBps (adjust mocks/fixtures accordingly) to prevent compile errors.
In `@prisma/migrations/20260527000000_swap_bps_and_partner_fields/migration.sql`:
- Around line 8-16: Add a DB-level invariant to ensure affiliateBps =
shapeshiftBps + partnerBps on the "swaps" table: after backfilling partnerBps
(using the existing UPDATE logic) run a verification SELECT to detect any rows
where "affiliateBps" <> ("shapeshiftBps" + "partnerBps") and fix them (or fail
the migration), then add an ALTER TABLE "swaps" ADD CONSTRAINT ... CHECK
("affiliateBps" = "shapeshiftBps" + "partnerBps") so future writes cannot
violate the three-way split; reference the columns partnerBps, affiliateBps,
shapeshiftBps and the "swaps" table when implementing.
---
Outside diff comments:
In `@apps/swap-service/src/verification/swap-verification.service.ts`:
- Around line 320-327: The returned object is including affiliate fields even
when hasShapeshiftAffiliate is false; update the return in the verification
result so that swap.affiliateBps, affiliateAddress (expectedTreasuryAddress) and
actualAffiliateFeeAmountCryptoBaseUnit (orderData?.context?.feeAmount) are only
set when hasShapeshiftAffiliate is true—otherwise omit or set them to undefined
and ensure hasAffiliate reflects hasShapeshiftAffiliate; locate the return
object in the function that computes hasShapeshiftAffiliate and adjust those
properties conditionally.
🪄 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: dbc51ef3-3a63-4a6e-b2df-9417d60b2f07
📒 Files selected for processing (14)
apps/swap-service/src/affiliate/affiliate.controller.tsapps/swap-service/src/affiliate/affiliate.service.tsapps/swap-service/src/swaps/constants.tsapps/swap-service/src/swaps/swaps.controller.tsapps/swap-service/src/swaps/swaps.service.tsapps/swap-service/src/swaps/utils.tsapps/swap-service/src/verification/__tests__/fixtures/maya/swap.tsapps/swap-service/src/verification/__tests__/fixtures/near/swap.tsapps/swap-service/src/verification/__tests__/fixtures/relay/swap.tsapps/swap-service/src/verification/__tests__/fixtures/thorchain/swap.tsapps/swap-service/src/verification/swap-verification.service.tspackages/shared-types/src/index.tsprisma/migrations/20260527000000_swap_bps_and_partner_fields/migration.sqlprisma/schema/swap-service.prisma
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.
Summary
affiliateBps,shapeshiftBps,partnerBps) so the invariantaffiliateBps = shapeshiftBps + partnerBpsholds without derivation. Callers now pass all three values snapshotted at quote time to eliminate drift between quote and registration.affiliateAddress→partnerAddress: Distinguishes the partner's payout address from on-chain affiliate addresses found during verification. Update all service code, controllers, queries, and test fixtures.receiveAddressandbuyAccountId: Both are always known at swap registration time. Migration backfillsbuyAccountIdfromreceiveAddress.hashAccountIdremoved from swap service — raw addresses are already stored alongside (receiveAddress, partnerAddress), making the hashing pointless. Also fixes thecheckTradeStatusbug where a hashed value was passed to swappers as an address.getAffiliateFeeRate→getPartnerFeeRate. Partner gets their agreedpartnerBpsshare first (capped at 100%), ShapeShift absorbs any shortfall if on-chain bps is less than expected.?? undefinedonaffiliateBps(now always non-null), simplifyhasAffiliatechecks.Testing
affiliateBps = shapeshiftBps + partnerBps)tradeExecution.tsanduseAffiliateTracking.ts)🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
affiliateAddresstoaddress.Breaking Changes
buyAccountIdandreceiveAddressnow required in swap requests.