Conversation
Implement POST /v1/swap/register and GET /v1/swap/status for affiliate swap tracking. QuoteStore (in-memory Map with dual TTL) stores quotes after generation and binds txHash on registration. - QuoteStore: 15min quote TTL, 60min post-tx TTL, automatic sweep - Register: validates quoteId, chainId, affiliate, duplicate txHash - Status: returns swap state with affiliate attribution data - Rate limiting: 10 req/s per affiliate on register endpoint - Abuse vectors: 404 fabricated, 409 replay, 403 cross-affiliate, 400 chain mismatch - OpenAPI docs updated with new endpoints
…n tracking, and security hardening - Public API: configurable BPS via X-Affiliate-Bps header, default 10 BPS base fee - Public API: swap lifecycle (quote → status+txHash → swap-service registration) - Public API: affiliate stats proxy endpoint - Widget: affiliateBps prop, redirect URL affiliate param appended correctly - Web app: sends affiliateBps, affiliateAddress, origin to swap-service - Swapper: affiliate fee asset tracking per swapper strategy - Dashboard: standalone Vite+React affiliate earnings dashboard - Security: swap creation moved from quote-time to status-time (prevents phantom inflation) - Lint: all errors resolved in public-api and dashboard packages
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a cross-cutting affiliate system: new affiliate dashboard package, affiliate tracking hook and UI wiring, new backend endpoints/middleware and quote persistence, affiliate fee calculation and propagation across swappers and widget, and several typings/build/package updates. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,230,255,0.5)
actor User
end
participant Dashboard as Affiliate Dashboard
participant TradeUI as Trade UI
participant PublicAPI as Public API
participant Swapper as Swappers
participant SwapSvc as Swap Service
User->>Dashboard: submit affiliate address & period
Dashboard->>PublicAPI: GET /v1/affiliate/stats?address=0x...&startDate...endDate...
PublicAPI->>SwapSvc: fetch affiliate stats
SwapSvc-->>PublicAPI: stats response
PublicAPI-->>Dashboard: AffiliateStatsResponse
User->>TradeUI: open trade (affiliate in URL)
TradeUI->>TradeUI: useAffiliateTracking() persists affiliate
TradeUI->>PublicAPI: GET /v1/swap/rates (X-Affiliate-Address/Bps)
PublicAPI->>Swapper: request quotes with affiliateAddress/Bps
Swapper->>Swapper: buildAffiliateFee(...) adds affiliateFee to steps
Swapper-->>PublicAPI: quotes with affiliateFee
PublicAPI->>PublicAPI: quoteStore.set(quoteId, quote + affiliate metadata)
PublicAPI-->>TradeUI: rates + quoteId
User->>TradeUI: confirm swap (txHash)
TradeUI->>PublicAPI: POST /v1/swap/status (quoteId, txHash)
PublicAPI->>PublicAPI: update quote (txHash, extend TTL)
PublicAPI->>SwapSvc: POST /swaps with affiliate metadata
SwapSvc-->>PublicAPI: recorded swap
PublicAPI-->>TradeUI: SwapStatusResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/swapper/src/swappers/AvnuSwapper/swapperApi/getTradeRate.ts (1)
107-118:⚠️ Potential issue | 🟠 MajorAVNU affiliate fee should use
'buy_asset'strategy to matchprotocolFeesrepresentation in the same step.Lines 107–118 calculate
protocolFeeson the buy-side (frombuyAmountAfterFeesCryptoBaseUnitusingbuyAsset), but lines 146–153 usestrategy: 'sell_asset'foraffiliateFee, which calculates the fee on the sell-side usingsellAmountCryptoBaseUnitandsellAsset. This creates conflicting fee metadata within a single trade step, where bothfeeData.protocolFeesandaffiliateFeeshould represent the same fee consistently.All other swappers (Across, Sunio, ZRX, Stonfi) use
'buy_asset'strategy. Align AVNU with this pattern to ensure fee consistency.Suggested fix
affiliateFee: buildAffiliateFee({ - strategy: 'sell_asset', + strategy: 'buy_asset', affiliateBps, sellAsset, buyAsset, sellAmountCryptoBaseUnit: sellAmount, buyAmountCryptoBaseUnit: buyAmountAfterFeesCryptoBaseUnit, }),Also applies to: getTradeQuote.ts lines 180–187
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swapper/src/swappers/AvnuSwapper/swapperApi/getTradeRate.ts` around lines 107 - 118, The protocolFees calculation currently builds fees on the buy side using buyAmountAfterFeesCryptoBaseUnit and buyAsset, but affiliateFee is constructed with strategy: 'sell_asset' using sellAmountCryptoBaseUnit and sellAsset; change affiliateFee to use the 'buy_asset' strategy and compute its amount from buyAmountAfterFeesCryptoBaseUnit and buyAsset so both fee representations match. Update the affiliateFee construction in getTradeRate.ts (the affiliateFee object where strategy is set) to use strategy: 'buy_asset' and the same bn(...) computation as protocolFees; make the same change in getTradeQuote.ts for its affiliateFee to ensure consistency across both files.packages/swap-widget/src/hooks/useSwapRates.ts (1)
31-31:⚠️ Potential issue | 🟠 MajorAdd affiliate dimensions to the rates cache key.
The
useSwapRateshook cache key does not include affiliate dimensions. Since affiliate headers (x-affiliate-address,x-affiliate-bps) affect API responses but are not part of the query key, different affiliate contexts will incorrectly share cached rates. The same issue exists inuseSwapQuote. AddaffiliateAddressandaffiliateBpsto both hooks' cache keys, or ensure they're passed as hook parameters rather than only in theapiClientconfiguration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swap-widget/src/hooks/useSwapRates.ts` at line 31, The cache key for useSwapRates (queryKey: ['swapRates', sellAssetId, buyAssetId, sellAmountCryptoBaseUnit, allowedSwapperNames]) omits affiliate dimensions so responses returned under different affiliate headers can be incorrectly shared; update useSwapRates (and mirror the same change in useSwapQuote) to include affiliateAddress and affiliateBps in the queryKey array (e.g., add affiliateAddress and affiliateBps alongside sellAssetId, buyAssetId, etc.) or alternatively accept affiliateAddress and affiliateBps as explicit hook parameters and use those values in the queryKey so cached results are segregated by affiliate context.src/components/MultiHopTrade/hooks/useGetTradeRateInput.ts (1)
130-156:⚠️ Potential issue | 🟠 MajorRate-input cache key is missing the affiliate dimension.
tradeInputQueryParamsincludesaffiliateAddress(line 111) with it in the dependency array (line 116), buttradeInputQueryKey(lines 130–156) omits it entirely. This causes the query to reuse cached results when the affiliate address changes, since React Query's cache is keyed bytradeInputQueryKey(line 160).🧩 Suggested fix
const tradeInputQueryKey = useMemo( () => ({ buyAsset, sellAmountCryptoPrecision, sellAsset, userSlippageTolerancePercentageDecimal, + affiliateAddress, // TODO(gomes): all the below are what's causing trade input to refentially invalidate on wallet connect // We will need to find a way to have our cake and eat it, by ensuring we get bip44 and other addy-related data to // referentially invalidate, while ensuring the *initial* connection of a wallet when quotes were gotten without one, doesn't invalidate anything sellAccountMetadata, receiveAccountMetadata, sellAccountId, isBuyAssetChainSupported, receiveAddress, }), [ + affiliateAddress, buyAsset, isBuyAssetChainSupported, receiveAccountMetadata, receiveAddress, sellAccountId, sellAccountMetadata, sellAmountCryptoPrecision, sellAsset, userSlippageTolerancePercentageDecimal, ], )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MultiHopTrade/hooks/useGetTradeRateInput.ts` around lines 130 - 156, tradeInputQueryKey is missing the affiliate dimension, causing cached rate inputs to be reused across affiliate changes; update the useMemo that builds tradeInputQueryKey to include affiliateAddress in the returned object and add affiliateAddress to its dependency array so the memo and the React Query key invalidate when affiliateAddress changes (tradeInputQueryParams already contains affiliateAddress, so keep them consistent).src/components/MultiHopTrade/hooks/useGetTradeQuotes/useGetTradeQuotes.tsx (1)
236-255:⚠️ Potential issue | 🟠 MajorAdd
affiliateAddressto theuseQuerycache key.
affiliateAddressaffects the query result (passed togetTradeQuoteOrRateInput) and is already in thequeryFnOrSkipdependency array, but it's missing from thequeryKeyobject. When the affiliate changes, the query function is recreated but the cache identity remains the same, which can cause stale cached results to be served.Suggested fix
const { data: tradeQuoteInput } = useQuery({ queryKey: [ 'getTradeQuoteInput', { buyAsset, dispatch, receiveAddress, sellAccountMetadata, sellAmountCryptoPrecision, sellAsset, receiveAccountMetadata, userSlippageTolerancePercentageDecimal, sellAssetUsdRate, sellAccountId, isBuyAssetChainSupported, hopExecutionMetadata, activeTrade, + affiliateAddress, }, ], queryFn: queryFnOrSkip, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MultiHopTrade/hooks/useGetTradeQuotes/useGetTradeQuotes.tsx` around lines 236 - 255, The cache key for the useQuery call in useGetTradeQuotes (the queryKey object passed into useQuery) is missing affiliateAddress, causing stale results when affiliate changes; update the queryKey to include affiliateAddress alongside buyAsset, sellAsset, sellAmountCryptoPrecision, receiveAddress, sellAccountId, hopExecutionMetadata, activeTrade, etc., so the key reflects the same dependencies used to build queryFnOrSkip and getTradeQuoteOrRateInput and triggers a fresh fetch when affiliateAddress changes.
🧹 Nitpick comments (5)
packages/swap-widget/vite.config.ts (1)
7-13: Remove newly added inline comments in this config.These comments are not required for functionality and conflict with the repo rule that disallows adding code comments unless explicitly requested.
As per coding guidelines: "Never add code comments unless explicitly requested."
Also applies to: 46-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swap-widget/vite.config.ts` around lines 7 - 13, Remove the newly added inline comment block starting with "Externals for the library build." and any other added comments in this file (including the comment at the other noted location containing the same style) so the file contains only code per repo rules; locate the top-of-file comment and the secondary comment and delete them (they are purely explanatory, not functional) ensuring the remaining exports/logic in vite.config.ts (the externals-handling function and related code) are unchanged.packages/swap-widget/package.json (1)
55-56: Constrain React peer range to tested majors.Lines 55-56 use
>=18.0.0, which also permits future React majors that may be untested. Consider pinning to supported majors (e.g.,^18 || ^19) to avoid accidental incompatible installs.🔧 Proposed fix
- "react": ">=18.0.0", - "react-dom": ">=18.0.0", + "react": "^18.0.0 || ^19.0.0", + "react-dom": "^18.0.0 || ^19.0.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swap-widget/package.json` around lines 55 - 56, The peerDependencies for "react" and "react-dom" currently use open-ended ranges (">=18.0.0"); restrict these to the tested major versions by replacing those ranges with explicit supported-major ranges (for example use "^18 || ^19" or whatever major sets you have validated) in packages/swap-widget package.json so installs won't pull untested future major React releases; update both the "react" and "react-dom" entries to the chosen constrained ranges and bump the package version/lockfile if required.packages/swapper/src/swappers/utils/affiliateFee.ts (1)
7-7: Use a string enum forAffiliateFeeStrategyto match project TS constant conventions.Please switch this new strategy constant type from a string-union to a string enum for consistency with the repository rule set.
As per coding guidelines: “ALWAYS use enums for constants in TypeScript” and “ALWAYS use string enums for better debugging in TypeScript.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swapper/src/swappers/utils/affiliateFee.ts` at line 7, Replace the string-union type AffiliateFeeStrategy with a exported string enum named AffiliateFeeStrategy and map each current literal to a descriptive enum member (e.g., BUY_ASSET = 'buy_asset', SELL_ASSET = 'sell_asset', FIXED_ASSET = 'fixed_asset'); update any existing references to use the enum (AffiliateFeeStrategy.BUY_ASSET, etc.) so code and imports continue to compile and follow the project's string-enum convention.packages/swap-widget/src/config/wagmi.ts (1)
14-26: Type safety reduced forSupportedChainId.Changing
SupportedChainIdfrom a union of literal chain IDs tonumberremoves compile-time validation of chain IDs. This allows any number to pass type checks where a supported chain ID is expected.If this is intentional for AppKit compatibility, consider adding a runtime validation guard or documenting this tradeoff.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swap-widget/src/config/wagmi.ts` around lines 14 - 26, SUPPORTED_CHAINS is a fixed readonly array, so changing SupportedChainId to number loses compile-time validation; update SupportedChainId to derive a union of the actual chain id literals from SUPPORTED_CHAINS (for example: export type SupportedChainId = typeof SUPPORTED_CHAINS[number]['id']) so only supported IDs are allowed by the type system, and if AppKit compatibility requires a plain number keep the number alias but add a runtime guard function (e.g., isSupportedChainId(id): id is SupportedChainId) that checks against SUPPORTED_CHAINS to validate values at runtime.packages/public-api/src/middleware/rateLimit.ts (1)
14-24: Consider a shared backend for production rate-limiting.This in-memory map is process-local. In multi-instance deployments, effective limits scale with replica count. A shared store (e.g., Redis) will keep limits consistent across nodes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/affiliate-dashboard/src/hooks/useAffiliateStats.ts`:
- Around line 38-85: The fetchStats function can let stale async responses
overwrite newer state; fix it by introducing a per-request guard (e.g., a
requestId counter ref or an AbortController) inside the useAffiliateStats hook:
increment a requestId (or abort previous controller) at the start of fetchStats,
capture the current id (or signal) before awaiting the fetch, and only call
setStats, setError or setIsLoading if the captured id still matches the latest
id (or the fetch was not aborted). Update symbols: modify fetchStats in
useAffiliateStats to use a requestIdRef (or controllerRef) and check it before
setStats/setError in the try/catch/finally blocks so older responses cannot
overwrite newer state.
In `@packages/public-api/src/docs/openapi.ts`:
- Around line 431-446: The OpenAPI schema SwapStatusResponseSchema (registered
as 'SwapStatusResponse') is missing the buyTxHash and isAffiliateVerified fields
returned by the /v1/swap/status handler; update the z.object passed to
registry.register in SwapStatusResponseSchema to include buyTxHash (a
string.optional() representing the buy transaction hash) and isAffiliateVerified
(a boolean.optional() representing affiliate verification), keeping naming
exactly buyTxHash and isAffiliateVerified and preserving existing
optional/required semantics so the schema matches the handler response.
In `@packages/public-api/src/lib/quoteStore.ts`:
- Line 45: The static unsubmitted-quote TTL constant QUOTE_TTL_MS is set to 2
minutes but should match the documented 15-minute validity window; update the
value of static readonly QUOTE_TTL_MS (in quoteStore.ts / QuoteStore) to 15 * 60
* 1000 (15 minutes) and adjust any related tests or comments that assume the
previous 2-minute value so unsubmitted quotes do not expire prematurely.
In `@packages/public-api/src/middleware/rateLimit.ts`:
- Around line 25-27: The limiter key currently uses the raw affiliate address
(getKey) which allows easy bypass; change getKey to build a hardened key by
normalizing the affiliate address (trim and toLowerCase, optionally strip 0x
prefix) and concatenating a stable network identifier (use
req.affiliateInfo?.network if available, else fall back to a request header like
req.headers['x-network'] or 'unknown-net'), e.g.
"<network>:<normalizedAddress>"; only if no affiliate info exists fall back to
using req.ip or req.socket.remoteAddress, and always return a single
deterministic string to avoid bucket-splitting (refer to getKey and the uses of
req.affiliateInfo?.affiliateAddress, req.affiliateInfo?.network, req.ip, and
req.socket.remoteAddress).
In `@packages/public-api/src/routes/affiliate.ts`:
- Around line 15-17: The schema currently allows startDate and endDate
individually but doesn't prevent startDate being after endDate; update the Zod
schema that defines startDate and endDate (the object with keys startDate and
endDate in packages/public-api/src/routes/affiliate.ts) to add a cross-field
validation (use superRefine or refine on the parsed object) that checks when
both dates are present that new Date(startDate) <= new Date(endDate), and return
a clear validation error on the appropriate path (e.g., addIssue for "startDate"
or "endDate") so invalid ranges are rejected early with a helpful message.
- Around line 65-67: The fetch to swap-service using backendUrl and assigning
backendResponse is missing a timeout; wrap the request with an AbortController
(use the same timeout constant GAS_FEES_TIMEOUT_MS from swapperDeps.ts), call
setTimeout to controller.abort() after the timeout, pass controller.signal to
fetch(backendUrl.toString(), { signal }), and clear the timeout when the fetch
resolves; also handle the abort case in the catch (treat AbortError or
DOMException name 'AbortError' as a timeout) so the handler returns an
appropriate timeout error instead of hanging.
In `@packages/public-api/src/routes/docs.ts`:
- Line 11: Remove the newly added inline comment "// Serve favicon" from
packages/public-api/src/routes/docs.ts; locate that exact comment in the file
(it was added near the top of the route definitions) and delete the line so the
file conforms to the repository guideline that forbids adding non-requested
comments.
In `@packages/public-api/src/routes/status.ts`:
- Around line 33-41: getSwapStatus currently returns stored quotes without
verifying affiliate ownership; after retrieving storedQuote via
quoteStore.get(quoteId) check the caller's affiliate context (the
request-affiliate value your code uses, e.g., req.affiliateAddress or
res.locals.affiliate) against storedQuote.affiliateAddress and if they differ
deny access (return 404 or 403 with the same ErrorResponse shape). Update the
lookup path in getSwapStatus to perform this equality check before sending the
quote, using storedQuote and storedQuote.affiliateAddress to locate the data and
the request-affiliate variable your app populates as the authority for the
caller.
- Around line 54-75: The TOCTOU happens because you mutate storedQuote after an
earlier check on current; instead perform an atomic re-check before writing:
re-read the latest quote from quoteStore (e.g., call quoteStore.get(quoteId)
into latest) immediately before assigning storedQuote.txHash/registeredAt/status
and, if latest?.txHash exists and latest.txHash !== txHash, return the existing
response (like the earlier branch); if latest.txHash === txHash also return
without resetting metadata; only then set quoteStore.set(quoteId, updatedQuote).
This implements a compare-and-swap style guard around storedQuote/quoteStore to
prevent overwriting when another request already bound the txHash.
In `@packages/swap-widget/src/config/standaloneWagmi.ts`:
- Around line 1-6: Remove the top-level docblock comment at the top of the file
that describes "Standalone wagmi configuration for read-only RPC access" (the
multi-line /** ... */ block) so the file no longer contains a top-level block
comment; leave any inline or function-level comments intact and ensure
exports/identifiers such as SwapWidgetWithExternalWallet or standalone wagmi
configuration code remain unchanged.
In `@packages/swapper/src/swappers/AcrossSwapper/utils/getTrade.ts`:
- Around line 426-433: affiliateFee is always computed via buildAffiliateFee
even when app/app-fee was intentionally disabled earlier; change the assignment
so affiliateFee is only set when the same flag used to enable app fees in this
function (reuse the existing symbol used earlier, e.g., appFee or appFeeEnabled)
is truthy—otherwise set affiliateFee to undefined (or zero if callers expect a
number). Concretely, wrap or conditionalize the buildAffiliateFee call (the
affiliateFee property) behind that existing app-fee eligibility check so
buildAffiliateFee(...) is only invoked when app fees are enabled.
In
`@packages/swapper/src/swappers/CowSwapper/getCowSwapTradeQuote/getCowSwapTradeQuote.ts`:
- Around line 190-197: The affiliateFee built in getCowSwapTradeQuote is missing
the isEstimate flag; update the buildAffiliateFee call (the affiliateFee
assignment inside getCowSwapTradeQuote) to include isEstimate: true so the
returned AffiliateFee object matches other swappers (e.g., PortalsSwapper,
BebopSwapper) and signals this is an estimated fee.
---
Outside diff comments:
In `@packages/swap-widget/src/hooks/useSwapRates.ts`:
- Line 31: The cache key for useSwapRates (queryKey: ['swapRates', sellAssetId,
buyAssetId, sellAmountCryptoBaseUnit, allowedSwapperNames]) omits affiliate
dimensions so responses returned under different affiliate headers can be
incorrectly shared; update useSwapRates (and mirror the same change in
useSwapQuote) to include affiliateAddress and affiliateBps in the queryKey array
(e.g., add affiliateAddress and affiliateBps alongside sellAssetId, buyAssetId,
etc.) or alternatively accept affiliateAddress and affiliateBps as explicit hook
parameters and use those values in the queryKey so cached results are segregated
by affiliate context.
In `@packages/swapper/src/swappers/AvnuSwapper/swapperApi/getTradeRate.ts`:
- Around line 107-118: The protocolFees calculation currently builds fees on the
buy side using buyAmountAfterFeesCryptoBaseUnit and buyAsset, but affiliateFee
is constructed with strategy: 'sell_asset' using sellAmountCryptoBaseUnit and
sellAsset; change affiliateFee to use the 'buy_asset' strategy and compute its
amount from buyAmountAfterFeesCryptoBaseUnit and buyAsset so both fee
representations match. Update the affiliateFee construction in getTradeRate.ts
(the affiliateFee object where strategy is set) to use strategy: 'buy_asset' and
the same bn(...) computation as protocolFees; make the same change in
getTradeQuote.ts for its affiliateFee to ensure consistency across both files.
In `@src/components/MultiHopTrade/hooks/useGetTradeQuotes/useGetTradeQuotes.tsx`:
- Around line 236-255: The cache key for the useQuery call in useGetTradeQuotes
(the queryKey object passed into useQuery) is missing affiliateAddress, causing
stale results when affiliate changes; update the queryKey to include
affiliateAddress alongside buyAsset, sellAsset, sellAmountCryptoPrecision,
receiveAddress, sellAccountId, hopExecutionMetadata, activeTrade, etc., so the
key reflects the same dependencies used to build queryFnOrSkip and
getTradeQuoteOrRateInput and triggers a fresh fetch when affiliateAddress
changes.
In `@src/components/MultiHopTrade/hooks/useGetTradeRateInput.ts`:
- Around line 130-156: tradeInputQueryKey is missing the affiliate dimension,
causing cached rate inputs to be reused across affiliate changes; update the
useMemo that builds tradeInputQueryKey to include affiliateAddress in the
returned object and add affiliateAddress to its dependency array so the memo and
the React Query key invalidate when affiliateAddress changes
(tradeInputQueryParams already contains affiliateAddress, so keep them
consistent).
---
Nitpick comments:
In `@packages/swap-widget/package.json`:
- Around line 55-56: The peerDependencies for "react" and "react-dom" currently
use open-ended ranges (">=18.0.0"); restrict these to the tested major versions
by replacing those ranges with explicit supported-major ranges (for example use
"^18 || ^19" or whatever major sets you have validated) in packages/swap-widget
package.json so installs won't pull untested future major React releases; update
both the "react" and "react-dom" entries to the chosen constrained ranges and
bump the package version/lockfile if required.
In `@packages/swap-widget/src/config/wagmi.ts`:
- Around line 14-26: SUPPORTED_CHAINS is a fixed readonly array, so changing
SupportedChainId to number loses compile-time validation; update
SupportedChainId to derive a union of the actual chain id literals from
SUPPORTED_CHAINS (for example: export type SupportedChainId = typeof
SUPPORTED_CHAINS[number]['id']) so only supported IDs are allowed by the type
system, and if AppKit compatibility requires a plain number keep the number
alias but add a runtime guard function (e.g., isSupportedChainId(id): id is
SupportedChainId) that checks against SUPPORTED_CHAINS to validate values at
runtime.
In `@packages/swap-widget/vite.config.ts`:
- Around line 7-13: Remove the newly added inline comment block starting with
"Externals for the library build." and any other added comments in this file
(including the comment at the other noted location containing the same style) so
the file contains only code per repo rules; locate the top-of-file comment and
the secondary comment and delete them (they are purely explanatory, not
functional) ensuring the remaining exports/logic in vite.config.ts (the
externals-handling function and related code) are unchanged.
In `@packages/swapper/src/swappers/utils/affiliateFee.ts`:
- Line 7: Replace the string-union type AffiliateFeeStrategy with a exported
string enum named AffiliateFeeStrategy and map each current literal to a
descriptive enum member (e.g., BUY_ASSET = 'buy_asset', SELL_ASSET =
'sell_asset', FIXED_ASSET = 'fixed_asset'); update any existing references to
use the enum (AffiliateFeeStrategy.BUY_ASSET, etc.) so code and imports continue
to compile and follow the project's string-enum convention.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (80)
.env.developmentpackages/affiliate-dashboard/index.htmlpackages/affiliate-dashboard/package.jsonpackages/affiliate-dashboard/src/App.tsxpackages/affiliate-dashboard/src/hooks/useAffiliateStats.tspackages/affiliate-dashboard/src/main.tsxpackages/affiliate-dashboard/tsconfig.jsonpackages/affiliate-dashboard/tsconfig.node.jsonpackages/affiliate-dashboard/vite.config.tspackages/chain-adapters/package.jsonpackages/contracts/package.jsonpackages/errors/package.jsonpackages/public-api/src/config.tspackages/public-api/src/docs/openapi.tspackages/public-api/src/index.tspackages/public-api/src/lib/quoteStore.tspackages/public-api/src/middleware/auth.tspackages/public-api/src/middleware/rateLimit.tspackages/public-api/src/routes/affiliate.tspackages/public-api/src/routes/docs.tspackages/public-api/src/routes/quote.tspackages/public-api/src/routes/rates.tspackages/public-api/src/routes/status.tspackages/public-api/src/types.tspackages/swap-widget/package.jsonpackages/swap-widget/src/api/client.tspackages/swap-widget/src/components/SwapWidget.csspackages/swap-widget/src/components/SwapWidget.tsxpackages/swap-widget/src/config/appkit.tspackages/swap-widget/src/config/standaloneWagmi.tspackages/swap-widget/src/config/wagmi.tspackages/swap-widget/src/demo/App.csspackages/swap-widget/src/hooks/useAssets.tspackages/swap-widget/src/hooks/useBalances.tspackages/swap-widget/src/hooks/useMarketData.tspackages/swap-widget/src/hooks/useSwapDisplayValues.tspackages/swap-widget/src/hooks/useSwapHandlers.tspackages/swap-widget/src/hooks/useSwapQuote.tspackages/swap-widget/src/hooks/useSwapRates.tspackages/swap-widget/src/types/index.tspackages/swap-widget/src/utils/redirect.tspackages/swap-widget/tsconfig.build.jsonpackages/swap-widget/vite.config.tspackages/swapper/package.jsonpackages/swapper/src/swappers/AcrossSwapper/utils/getTrade.tspackages/swapper/src/swappers/AvnuSwapper/swapperApi/getTradeQuote.tspackages/swapper/src/swappers/AvnuSwapper/swapperApi/getTradeRate.tspackages/swapper/src/swappers/BebopSwapper/getBebopTradeQuote/getBebopTradeQuote.tspackages/swapper/src/swappers/BebopSwapper/getBebopTradeRate/getBebopTradeRate.tspackages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.tspackages/swapper/src/swappers/ButterSwap/swapperApi/getTradeRate.tspackages/swapper/src/swappers/CetusSwapper/swapperApi/getTradeQuote.tspackages/swapper/src/swappers/CetusSwapper/swapperApi/getTradeRate.tspackages/swapper/src/swappers/ChainflipSwapper/utils/getQuoteOrRate.tspackages/swapper/src/swappers/CowSwapper/getCowSwapTradeQuote/getCowSwapTradeQuote.tspackages/swapper/src/swappers/CowSwapper/getCowSwapTradeRate/getCowSwapTradeRate.tspackages/swapper/src/swappers/JupiterSwapper/swapperApi/getTradeQuote.tspackages/swapper/src/swappers/JupiterSwapper/swapperApi/getTradeRate.tspackages/swapper/src/swappers/NearIntentsSwapper/swapperApi/getTradeQuote.tspackages/swapper/src/swappers/NearIntentsSwapper/swapperApi/getTradeRate.tspackages/swapper/src/swappers/PortalsSwapper/getPortalsTradeQuote/getPortalsTradeQuote.tspackages/swapper/src/swappers/RelaySwapper/utils/getTrade.tspackages/swapper/src/swappers/StonfiSwapper/swapperApi/getTradeQuote.tspackages/swapper/src/swappers/StonfiSwapper/swapperApi/getTradeRate.tspackages/swapper/src/swappers/SunioSwapper/utils/getQuoteOrRate.tspackages/swapper/src/swappers/ZrxSwapper/getZrxTradeQuote/getZrxTradeQuote.tspackages/swapper/src/swappers/ZrxSwapper/getZrxTradeRate/getZrxTradeRate.tspackages/swapper/src/swappers/utils/affiliateFee.tspackages/swapper/src/thorchain-utils/getL1RateOrQuote.tspackages/swapper/src/types.tspackages/types/package.jsonpackages/unchained-client/package.jsonpackages/utils/package.jsonsrc/components/MultiHopTrade/hooks/useGetTradeQuotes/getTradeQuoteOrRateInput.tssrc/components/MultiHopTrade/hooks/useGetTradeQuotes/useGetTradeQuotes.tsxsrc/components/MultiHopTrade/hooks/useGetTradeRateInput.tssrc/hooks/useAffiliateTracking/index.tssrc/hooks/useAffiliateTracking/useAffiliateTracking.tssrc/lib/tradeExecution.tssrc/pages/Trade/tabs/TradeTab.tsx
packages/swapper/src/swappers/CowSwapper/getCowSwapTradeQuote/getCowSwapTradeQuote.ts
Show resolved
Hide resolved
gomes-bot
left a comment
gomes-bot
left a comment
There was a problem hiding this comment.
Deep Review: B2B Affiliate Tracking System
Big feature, lots of moving parts. Overall this is architecturally sound and well-thought-out for a v1. The OpenAPI docs are genuinely impressive - full widget SDK integration guide with code examples, theming, and architecture notes baked right into Scalar. That's above and beyond.
Grouped feedback below by area. A few things I'd want addressed before merge, rest are suggestions/questions.
Architecture & Design
The flow is clean: Client -> Public API -> quote store (in-memory) -> swap-service backend. The quote store with dual TTL (unsubmitted vs submitted) is a nice pattern. The status endpoint doubling as the txHash registration point (no separate /register call) simplifies the integrator's life.
The in-memory quote store is the right call for v1, and the Redis migration path is well-documented. 10k max quotes with oldest-first eviction is reasonable for launch.
The OpenAPI spec serves double duty as both API reference and Widget SDK documentation - unconventional but convenient for a single Scalar endpoint.
Security Concerns
Rate limiting needs attention (see inline comments). The 10 req/sec limit is very permissive, and the IP-based fallback may not work correctly behind reverse proxies without trust proxy configuration.
No authentication for affiliates - anyone can claim any Arbitrum address. This is fine if affiliate fees are verified on-chain (which the companion microservices PR handles), but worth documenting that the API is "trust but verify".
Input validation is decent (Zod schemas, EVM address regex) but txHash validation is minimal (min(1)) - could benefit from format/length constraints.
Code Quality
- TypeScript types are complete and well-structured. The
AffiliateFeetype with strategy-based construction (buildAffiliateFee) is clean - The
buildAffiliateFeeutility is applied consistently across all 15+ swapper implementations - good mechanical work useAffiliateTrackinghook handles hash-routing URL parsing correctly (important for the SPA)- Error handling in the API routes is thorough with proper HTTP status codes and error codes
Default Affiliate BPS Change
The default changed from 60 (0.6%) to 10 (0.1%). This affects ALL swaps through the public API, not just affiliate-tracked ones. Want to make sure this is intentional and not meant to only apply to affiliate swaps.
Swap Widget Changes
affiliateBpsprop threading looks correct- The
standaloneWagmiConfigfor external wallet mode is a good solution for balance fetching without AppKit - Redirect URL format changed from query params to hash-based route format - this is correct for the web app's routing
- Package properly restructured for npm publishing (exports, peerDeps,
vite-plugin-dts)
Testing
No tests included for the new public-api routes, quote store, rate limiter, or affiliate tracking hook. The quote store in particular is very testable and handles edge cases (TTL, eviction, TOCTOU) that would benefit from unit tests. The companion microservices PR reportedly has tests, but the public-api surface area here is substantial.
What I'd Want Before Merge
- Rate limiter hardening - trust proxy config, consider more conservative defaults
- Quote TTL comment/value alignment - comment says 15 min, code says 2 min
tradeExecution.tsaffiliate expiry bypass - direct localStorage read skips TTL check- Confirm DEFAULT_AFFILIATE_BPS change is intentional for all swaps
Everything else is suggestions/questions. Solid work on a complex feature ser 🤝
|
|
||
| static readonly QUOTE_TTL_MS = 2 * 60 * 1000 | ||
| static readonly EXECUTION_TTL_MS = 60 * 60 * 1000 | ||
| static readonly CLEANUP_INTERVAL_MS = 60 * 1000 |
There was a problem hiding this comment.
suggestion: QUOTE_TTL_MS is 2 minutes but the comment above (line 34) says "15 minutes for unsubmitted quotes". Either the comment is stale or this was reduced during development. For a real-world quote validity window, 2 minutes is tight - users signing on hardware wallets or multi-sig setups may take longer.
Recommend aligning the comment and reconsidering the TTL.
There was a problem hiding this comment.
Fixed — TTL has been increased to 15 minutes to match the documented window. Comment and code are now aligned.
| static readonly CLEANUP_INTERVAL_MS = 60 * 1000 | ||
| static readonly MAX_QUOTES = 10000 | ||
|
|
||
| constructor() { |
There was a problem hiding this comment.
q: 10k max quotes in-memory - is there a capacity plan? Under load from multiple affiliates, 10k could fill fast. The eviction strategy (oldest-first) is fine for now, but this should be documented as a known scaling limit.
The Redis migration path mentioned in the comment is good foresight.
There was a problem hiding this comment.
Good call. 10k is a documented scaling limit for v1. The eviction strategy (oldest-first) keeps memory bounded, and the Redis migration path is ready when we need it. At current expected affiliate volume this should be fine, but we'll monitor capacity in prod metrics.
|
|
||
| const WINDOW_MS = 1000 | ||
| const MAX_REQUESTS_PER_WINDOW = 10 | ||
| const CLEANUP_INTERVAL_MS = 60 * 1000 |
There was a problem hiding this comment.
preferably-blocking: The rate limit is 10 requests per 1 second per key. This is extremely permissive for a production API that proxies to backend services.
More importantly, getKey() falls back to req.ip which behind a reverse proxy will be the proxy's IP unless trust proxy is configured in Express. This means ALL unauthenticated traffic could share a single rate limit bucket.
Action:
- Set
app.set('trust proxy', true)(or appropriate value) inindex.tsif behind a load balancer - Consider a more conservative default (e.g., 30 req per 60 sec for unauthenticated)
There was a problem hiding this comment.
Addressed — the custom in-memory rate limiter has been replaced with express-rate-limit (from develop merge). Per-endpoint limits are now: global 300/min, swap rates 60/min, swap quote 45/min, swap status 60/min, affiliate stats 30/min, data endpoints 120/min. Trust proxy is configured via TRUST_PROXY env var.
| } | ||
| }, CLEANUP_INTERVAL_MS) | ||
|
|
||
| const getKey = (req: Request): string => { |
There was a problem hiding this comment.
suggestion: The fallback to 'unknown' means if affiliateAddress, req.ip, AND req.socket.remoteAddress are all undefined, all such requests share one bucket. Worth a log warning when this happens.
There was a problem hiding this comment.
Addressed — the custom rate limiter has been replaced with express-rate-limit from the develop merge. The unknown key issue is no longer applicable since express-rate-limit uses req.ip directly with proper trust proxy configuration.
| export const StatusRequestSchema = z.object({ | ||
| quoteId: z.string().uuid(), | ||
| txHash: z.string().min(1).optional(), | ||
| }) |
There was a problem hiding this comment.
q: txHash validation is z.string().min(1) - should this validate format more strictly? At minimum a max length check would prevent abuse:
txHash: z.string().min(1).max(128).optional(),EVM hashes are 66 chars, BTC txids 64, Solana sigs ~88. 128 covers all with margin.
There was a problem hiding this comment.
Fixed — added max(128) validation on txHash in 26a8517.
| import { getConfig } from '@/config' | ||
| import { queryClient } from '@/context/QueryClientProvider/queryClient' | ||
| import { AFFILIATE_STORAGE_KEY } from '@/hooks/useAffiliateTracking/useAffiliateTracking' | ||
| import { fetchIsSmartContractAddressQuery } from '@/hooks/useIsSmartContractAddress/useIsSmartContractAddress' |
There was a problem hiding this comment.
preferably-blocking: Direct localStorage.getItem(AFFILIATE_STORAGE_KEY) bypasses the useAffiliateTracking hook's TTL/expiry check. If the affiliate address is expired (>30 days), this will still send it to the swap-service.
Consider extracting and exporting readStoredAffiliate() from useAffiliateTracking.ts and using it here instead.
There was a problem hiding this comment.
Fixed — switched to use exported readStoredAffiliate() from useAffiliateTracking.ts which respects TTL/expiry. 26a8517.
| url.searchParams.set('sellAmount', params.sellAmount) | ||
| } | ||
| return url.toString() | ||
| const { sellAssetId, buyAssetId, sellAmountBaseUnit, affiliateAddress } = params |
There was a problem hiding this comment.
suggestion: Manual string splitting with indexOf('/') to parse CAIP-19 asset IDs. The @shapeshiftoss/caip package has fromAssetId() which does this properly. Consider using it for consistency with the rest of the codebase.
There was a problem hiding this comment.
Good point. The swap-widget package intentionally keeps a minimal dependency footprint to reduce bundle size for integrators. Using @shapeshiftoss/caip would pull in the full caip package as a dependency. The manual parsing here handles the specific CAIP-19 format we need. Can revisit if caip becomes a peer dep.
| return { | ||
| assetId: sellAsset.assetId, | ||
| amountCryptoBaseUnit: feeAmount, | ||
| asset: sellAsset, |
There was a problem hiding this comment.
q: The fixed_asset strategy always returns amountCryptoBaseUnit: '0' and isEstimate: true. Is this because the actual fee for Relay/NearIntents is collected on-chain and can't be pre-calculated? If so, worth a comment explaining why the amount is hardcoded to zero.
There was a problem hiding this comment.
Correct — for Relay and NearIntents the affiliate fee is collected on-chain by the protocol itself, so there's no pre-calculable amount at quote time. The '0' amount with isEstimate: true signals to the UI that a fee exists but the exact amount is determined at execution time.
| import { createConfig, http } from 'wagmi' | ||
|
|
||
| const chains = [mainnet, polygon, arbitrum, optimism, base, avalanche, bsc, gnosis] as const | ||
|
|
There was a problem hiding this comment.
suggestion: Default http() transports = public RPC endpoints with aggressive rate limits. Fine for MVP but will cause issues at scale. Consider allowing integrators to pass custom transports or at minimum documenting this limitation.
There was a problem hiding this comment.
Known MVP limitation. The widget's apiBaseUrl prop could be extended to accept custom transport config in a future version. For v1, public RPCs are acceptable since the widget only uses them for balance reads, not high-frequency operations.
| @@ -0,0 +1,430 @@ | |||
| import { useCallback, useEffect, useMemo, useState } from 'react' | |||
|
|
|||
There was a problem hiding this comment.
suggestion: ~200 lines of inline style objects at the bottom of this file. For a standalone package, even a single CSS file would be cleaner to maintain. Also no responsive breakpoints - the stats grid may not look great on mobile.
No big deal for a v1 dashboard though.
There was a problem hiding this comment.
Agreed, acceptable for v1. The dashboard is an internal tool for affiliates to check their stats. Will clean up if it grows in scope.
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
Code Review — PR #12012
Verdict: ⚠️ COMMENT — Needs full review, providing initial assessment
Summary: Complete B2B affiliate tracking pipeline — Public API, Swap Widget updates, Web App integration, and standalone Affiliate Dashboard. 81 files, 3469 additions. This is a major feature requiring careful security and architecture review.
Architecture Assessment
The flow makes sense:
- Client gets quote from DEX aggregators via Public API
- Client signs + broadcasts
- Status endpoint (
GET /v1/swap/status?quoteId=...&txHash=0x...) creates swap record - Swap Service polls chain, verifies affiliate on-chain
- Affiliate fees tracked and queryable
Key design decisions:
- No separate register call — status endpoint handles txHash binding on first call (simpler API surface)
- Rate limiting per affiliate address — good for abuse prevention
- localStorage persistence — affiliate address persists across sessions via URL param → localStorage
- Period-based stats — monthly periods starting on the 5th
What I Verified (High-level)
- All swappers updated to thread
affiliateAddressthrough quote/rate inputs (14+ swappers touched) - Public API has auth middleware, rate limiting, OpenAPI docs
- Widget passes
affiliate=0xAddressvia redirect URLs - Web app reads
?affiliate=from URL params and persists in localStorage - Package bumps for
@shapeshiftoss/*packages to support new affiliate fields
Areas Needing Deeper Review
- Security of affiliate parameter injection: How is the
affiliateURL param validated? Arbitrary address injection could redirect fees. - Rate limiting implementation: Need to verify the rate limiter is per-endpoint AND per-affiliate, not just global.
- localStorage trust: If affiliate address comes from URL param → localStorage, a malicious link could override a legitimate affiliate's tracking.
- API key management: The Public API has auth middleware — how are API keys distributed and rotated?
- Affiliate fee calculation: Where exactly are fees computed? On-chain verification vs off-chain trust.
Risk Assessment
Medium-high — New API surface, affiliate fee attribution, localStorage-based session tracking. No CI pipeline run visible (only CodeRabbit). Needs a second reviewer with domain expertise in the affiliate/revenue system.
Browser Test
This PR adds UI-facing changes (URL param handling, affiliate dashboard). Browser testing would be valuable but requires the Public API backend running.
Recommendation
This PR would benefit from a second thorough review given its scope (81 files, new API surface, financial attribution). I've provided architectural observations but a line-by-line review of the Public API auth/rate-limiting and the affiliate fee flow would be prudent.
- Fix quoteStore TTL to match documented 15-minute validity window - Harden rate limiter key with address normalization and IP composite - Add trust proxy config for correct IP detection behind reverse proxy - Add date range validation and fetch timeout to affiliate stats endpoint - Add missing buyTxHash/isAffiliateVerified to OpenAPI schema - Add affiliate ownership check and fix TOCTOU guard in swap status - Add txHash max length validation - Remove non-functional comments per repo conventions - Gate Across affiliateFee behind appFee eligibility - Add isEstimate flag to CowSwap affiliate fee - Fix AVNU affiliate fee strategy from sell_asset to buy_asset - Add affiliate dimensions to widget/web app query cache keys - Fix tradeExecution.ts to use readStoredAffiliate() with TTL check - Add stale response guard to affiliate dashboard hook - Add dev URL warning log, constrain React peer range
# Conflicts: # packages/public-api/src/docs/openapi.ts # packages/public-api/src/index.ts # packages/public-api/src/middleware/rateLimit.ts # packages/swapper/src/types.ts # yarn.lock
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/public-api/src/docs/openapi.ts (2)
494-525: Missing 429 rate limit response in OpenAPI spec.The
/v1/swap/statusendpoint usesswapStatusLimiterbut the OpenAPI spec doesn't document the 429 response. This inconsistency affects API consumers who rely on the spec for error handling.📝 Suggested fix
responses: { description: 'Swap status', content: { 'application/json': { schema: SwapStatusResponseSchema, }, }, }, description: 'Invalid request parameters', }, description: 'Quote not found or expired', }, description: 'Transaction hash mismatch', }, + 429: rateLimitResponse, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/public-api/src/docs/openapi.ts` around lines 494 - 525, The OpenAPI registration for getSwapStatus (registry.registerPath with operationId 'getSwapStatus') is missing a 429 response for the rate limiter; update the responses object for the '/v1/swap/status' path to include a 429 entry (e.g., description 'Too many requests - rate limit exceeded') and, if appropriate, add a response body schema (application/json) describing the rate-limit error so clients know to handle swapStatusLimiter-triggered throttling.
538-565: Missing 429 rate limit response in OpenAPI spec.The
/v1/affiliate/statsendpoint usesaffiliateStatsLimiterbut the OpenAPI spec doesn't document the 429 response.📝 Suggested fix
responses: { description: 'Affiliate statistics', content: { 'application/json': { schema: AffiliateStatsResponseSchema, }, }, }, description: 'Invalid address format', }, description: 'Swap service unavailable', }, + 429: rateLimitResponse, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/public-api/src/docs/openapi.ts` around lines 538 - 565, The OpenAPI registration for the getAffiliateStats path (registry.registerPath with operationId 'getAffiliateStats') is missing a 429 response to reflect the affiliateStatsLimiter; update the responses object for that route to add a 429 entry (e.g., description 'Too many requests') and include the standard Retry-After header (and optional error schema if you have a common RateLimitError schema) so the spec matches the runtime rate limiting behavior.packages/swapper/src/types.ts (1)
201-201: Use a stronger domain type for affiliate address.Line 201 currently uses
string; for an Arbitrum affiliate identifier, preferAddress(or a nominal alias) to prevent invalid values from flowing into swapper inputs.Proposed change
type CommonTradeInputBase = { sellAsset: Asset buyAsset: Asset sellAmountIncludingProtocolFeesCryptoBaseUnit: string affiliateBps: string - affiliateAddress?: string + affiliateAddress?: Address allowMultiHop: boolean slippageTolerancePercentageDecimal?: string }#!/bin/bash set -euo pipefail # Verify how affiliateAddress is produced/consumed and whether it is normalized/validated as an EVM address. rg -n -C3 '\baffiliateAddress\b' -g '**/*.{ts,tsx,js,jsx}' rg -n -C3 '\b(isAddress|getAddress)\s*\(' -g '**/*.{ts,tsx,js,jsx}'Expected verification result: callsites passing
affiliateAddressshould already normalize/validate EVM addresses, making the stronger type safe to adopt.
As per coding guidelines: "**/*.ts: UseNominaltypes for domain identifiers (e.g.,WalletId,AccountId)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swapper/src/types.ts` at line 201, The affiliateAddress field is typed as a plain string and should be narrowed to the stronger domain type (e.g., Address or a Nominal alias) to prevent invalid EVM addresses from entering swapper inputs; change the type of affiliateAddress in the SwapInput/related interface(s) to Address (or a new Nominal type like AffiliateAddress) and update imports/exports accordingly, then run the suggested grep checks to verify all call sites referencing affiliateAddress (and any normalization helpers like isAddress/getAddress) already validate/normalize values so this stronger type is safe to adopt; update any call sites that construct affiliateAddress to normalize/validate before passing the value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/swapper/src/thorchain-utils/getL1RateOrQuote.ts`:
- Around line 306-313: Replace the recomputed affiliate fee (built from
buyAmountAfterFeesCryptoBaseUnit via buildAffiliateFee) with the
Thornode-provided affiliate fee: take route.quote.fees.affiliate and convert it
to the asset's crypto-base-unit precision (the same conversion used elsewhere in
this file) and assign that value to steps[0].affiliateFee instead of calling
buildAffiliateFee; keep the rest of the step structure unchanged so
steps[0].affiliateFee uses the authoritative route.quote.fees.affiliate value.
In `@src/hooks/useAffiliateTracking/useAffiliateTracking.ts`:
- Around line 31-39: The code returns the stored affiliate address without
validating its format; update the return path (where it currently returns
address) to first call isAddress(address) and if that returns false call
clearAffiliateStorage() and return null (optionally log a clear validation
message), otherwise return the validated address; keep existing expiry check
with isAffiliateExpired(timestamp) and reuse clearAffiliateStorage() on
invalid/expired data.
---
Nitpick comments:
In `@packages/public-api/src/docs/openapi.ts`:
- Around line 494-525: The OpenAPI registration for getSwapStatus
(registry.registerPath with operationId 'getSwapStatus') is missing a 429
response for the rate limiter; update the responses object for the
'/v1/swap/status' path to include a 429 entry (e.g., description 'Too many
requests - rate limit exceeded') and, if appropriate, add a response body schema
(application/json) describing the rate-limit error so clients know to handle
swapStatusLimiter-triggered throttling.
- Around line 538-565: The OpenAPI registration for the getAffiliateStats path
(registry.registerPath with operationId 'getAffiliateStats') is missing a 429
response to reflect the affiliateStatsLimiter; update the responses object for
that route to add a 429 entry (e.g., description 'Too many requests') and
include the standard Retry-After header (and optional error schema if you have a
common RateLimitError schema) so the spec matches the runtime rate limiting
behavior.
In `@packages/swapper/src/types.ts`:
- Line 201: The affiliateAddress field is typed as a plain string and should be
narrowed to the stronger domain type (e.g., Address or a Nominal alias) to
prevent invalid EVM addresses from entering swapper inputs; change the type of
affiliateAddress in the SwapInput/related interface(s) to Address (or a new
Nominal type like AffiliateAddress) and update imports/exports accordingly, then
run the suggested grep checks to verify all call sites referencing
affiliateAddress (and any normalization helpers like isAddress/getAddress)
already validate/normalize values so this stronger type is safe to adopt; update
any call sites that construct affiliateAddress to normalize/validate before
passing the value.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (25)
.env.developmentpackages/affiliate-dashboard/src/hooks/useAffiliateStats.tspackages/public-api/src/config.tspackages/public-api/src/docs/openapi.tspackages/public-api/src/index.tspackages/public-api/src/lib/quoteStore.tspackages/public-api/src/middleware/rateLimit.tspackages/public-api/src/routes/affiliate.tspackages/public-api/src/routes/docs.tspackages/public-api/src/routes/status.tspackages/swap-widget/package.jsonpackages/swap-widget/src/config/standaloneWagmi.tspackages/swap-widget/src/hooks/useSwapRates.tspackages/swap-widget/vite.config.tspackages/swapper/src/swappers/AcrossSwapper/utils/getTrade.tspackages/swapper/src/swappers/AvnuSwapper/swapperApi/getTradeQuote.tspackages/swapper/src/swappers/AvnuSwapper/swapperApi/getTradeRate.tspackages/swapper/src/swappers/CowSwapper/getCowSwapTradeQuote/getCowSwapTradeQuote.tspackages/swapper/src/thorchain-utils/getL1RateOrQuote.tspackages/swapper/src/types.tspackages/unchained-client/package.jsonsrc/components/MultiHopTrade/hooks/useGetTradeQuotes/useGetTradeQuotes.tsxsrc/components/MultiHopTrade/hooks/useGetTradeRateInput.tssrc/hooks/useAffiliateTracking/useAffiliateTracking.tssrc/lib/tradeExecution.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/components/MultiHopTrade/hooks/useGetTradeQuotes/useGetTradeQuotes.tsx
- src/lib/tradeExecution.ts
- packages/swap-widget/src/config/standaloneWagmi.ts
- packages/swapper/src/swappers/CowSwapper/getCowSwapTradeQuote/getCowSwapTradeQuote.ts
- packages/swapper/src/swappers/AvnuSwapper/swapperApi/getTradeQuote.ts
- packages/affiliate-dashboard/src/hooks/useAffiliateStats.ts
- packages/public-api/src/lib/quoteStore.ts
- packages/public-api/src/routes/status.ts
- packages/swapper/src/swappers/AcrossSwapper/utils/getTrade.ts
| affiliateFee: buildAffiliateFee({ | ||
| strategy: 'buy_asset', | ||
| affiliateBps: route.affiliateBps, | ||
| sellAsset, | ||
| buyAsset, | ||
| sellAmountCryptoBaseUnit, | ||
| buyAmountCryptoBaseUnit: buyAmountAfterFeesCryptoBaseUnit, | ||
| }), |
There was a problem hiding this comment.
Use Thornode’s quote.fees.affiliate as the affiliate fee source
At Line [312], the fee is recomputed from buyAmountAfterFeesCryptoBaseUnit (net output). This can diverge from Thor’s actual affiliate fee because rounding/fee ordering may differ. Since route.quote.fees.affiliate is already provided and used elsewhere in this file as authoritative, use that value (after precision conversion) for steps[0].affiliateFee.
Suggested fix
- affiliateFee: buildAffiliateFee({
- strategy: 'buy_asset',
- affiliateBps: route.affiliateBps,
- sellAsset,
- buyAsset,
- sellAmountCryptoBaseUnit,
- buyAmountCryptoBaseUnit: buyAmountAfterFeesCryptoBaseUnit,
- }),
+ affiliateFee: (() => {
+ const affiliateAmountCryptoBaseUnit = convertPrecision({
+ value: route.quote.fees.affiliate,
+ inputExponent: buyAssetNativePrecision,
+ outputExponent: buyAsset.precision,
+ }).toFixed()
+
+ if (bnOrZero(affiliateAmountCryptoBaseUnit).lte(0)) return undefined
+
+ return {
+ assetId: buyAsset.assetId,
+ amountCryptoBaseUnit: affiliateAmountCryptoBaseUnit,
+ asset: buyAsset,
+ }
+ })(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/swapper/src/thorchain-utils/getL1RateOrQuote.ts` around lines 306 -
313, Replace the recomputed affiliate fee (built from
buyAmountAfterFeesCryptoBaseUnit via buildAffiliateFee) with the
Thornode-provided affiliate fee: take route.quote.fees.affiliate and convert it
to the asset's crypto-base-unit precision (the same conversion used elsewhere in
this file) and assign that value to steps[0].affiliateFee instead of calling
buildAffiliateFee; keep the rest of the step structure unchanged so
steps[0].affiliateFee uses the authoritative route.quote.fees.affiliate value.
| if (!address) return null | ||
|
|
||
| if (isAffiliateExpired(timestamp)) { | ||
| clearAffiliateStorage() | ||
| return null | ||
| } | ||
|
|
||
| return address | ||
| } catch (error) { |
There was a problem hiding this comment.
Validate stored affiliate address before returning it.
Line 38 returns localStorage data without format validation. Since storage is untrusted, a malformed value can be forwarded downstream and trigger persistent affiliate-related request failures. Validate with isAddress(address) and clear storage on invalid values before returning.
Proposed fix
export const readStoredAffiliate = (): string | null => {
if (typeof window === 'undefined') return null
try {
const address = window.localStorage.getItem(AFFILIATE_STORAGE_KEY)
const timestamp = window.localStorage.getItem(AFFILIATE_TIMESTAMP_KEY)
if (!address) return null
+ if (!isAddress(address)) {
+ clearAffiliateStorage()
+ return null
+ }
if (isAffiliateExpired(timestamp)) {
clearAffiliateStorage()
return null
}
return addressAs per coding guidelines: "ALWAYS validate inputs before processing with clear validation error messages and use early returns for validation failures".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useAffiliateTracking/useAffiliateTracking.ts` around lines 31 - 39,
The code returns the stored affiliate address without validating its format;
update the return path (where it currently returns address) to first call
isAddress(address) and if that returns false call clearAffiliateStorage() and
return null (optionally log a clear validation message), otherwise return the
validated address; keep existing expiry check with isAffiliateExpired(timestamp)
and reuse clearAffiliateStorage() on invalid/expired data.
# Conflicts: # yarn.lock
🧪 QA ReportCI Status: ❌ FAILINGFailed step: Lint (Static checks) PR: B2B affiliate tracking systemThis is a larger feature PR. Please:
Once CI PassesTesting checklist:
|
2 participants
Summary
Implements the complete B2B affiliate tracking pipeline — enabling integrators (widget users, API consumers, web redirect partners) to track swap volumes and affiliate fees via their Arbitrum address.
Epic: ss-aff — B2B Affiliate Tracking System
Architecture
graph TB subgraph Clients WEB[Web App<br/>app.shapeshift.com] WIDGET[Swap Widget<br/>@shapeshiftoss/swap-widget] DASH[Affiliate Dashboard] API_USER[API Integrators] end subgraph Public API PAPI[Public API Server<br/>Express · /v1/*] DOCS[Scalar API Docs<br/>/docs] end subgraph Backend Microservices SWAP[Swap Service<br/>NestJS · 46 chains · 18 swappers] USER[User Service<br/>NestJS · Referrals] NOTIF[Notifications Service<br/>NestJS · Push + WS] end subgraph Storage DB_SWAP[(swap_service DB)] DB_USER[(user_service DB)] DB_NOTIF[(notifications DB)] end subgraph External SWAPPERS[DEX Aggregators<br/>THORChain · 0x · Jupiter<br/>Relay · Portals · Chainflip<br/>CoW · Bebop · AVNU · ...] CHAINS[Blockchain Nodes<br/>Unchained APIs · RPC] end WEB & WIDGET & DASH & API_USER -->|HTTPS| PAPI PAPI -->|/swaps/*| SWAP PAPI -->|quotes + rates| SWAPPERS SWAP -->|referral lookup| USER SWAP -->|status alerts| NOTIF SWAP --> DB_SWAP USER --> DB_USER NOTIF --> DB_NOTIF SWAP -->|on-chain verify| CHAINS NOTIF -->|device tokens| USERFlow: Client → Public API → gets quote from DEX aggregators → client signs + broadcasts → client calls
GET /v1/swap/status?quoteId=...&txHash=0x...→ Public API creates swap record in Swap Service → Swap Service polls chain, verifies affiliate on-chain → affiliate fees tracked.Changes
Public API (
packages/public-api/)GET /v1/affiliate/stats?address=...)/v1/swap/register— status endpoint handles txHash binding on first callSWAP_SERVICE_BASE_URL→dev-api.swap-service.shapeshift.comSwap Widget (
packages/swap-widget/)affiliate=0xAddressin redirect URLs toapp.shapeshift.com/tradeaffiliateAddressprop through component hierarchyWeb App (
src/)?affiliate=0xAddrfrom URL paramsAffiliate Dashboard (
packages/affiliate-dashboard/)Package Bumps
@shapeshiftoss/*packages to support new affiliate fieldsCompanion PR
Backend: shapeshift/microservices#19 — Swap Service with affiliate verification, 46 chain adapters, polling, and comprehensive test suite.
How to test:
Summary by CodeRabbit
New Features
Improvements
Backend Updates