feat(affiliate): add trader activity endpoint#218
Merged
kernelwhisperer merged 5 commits intomainfrom May 6, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds trader-activity support: new DUNE query config and .env example entry, types and raw/type guards, normalization helpers, service implementation with per-address caching and getTraderActivity, guarded API GET route with schemas, tests for behavior and errors, and a one-line logger.error addition in a ref-codes handler. ChangesTrader Activity Feature
Error Logging Enhancement
Sequence Diagram(s)sequenceDiagram
participant Client
participant APIRoute as API Route (trader-activity)
participant Service as AffiliateStatsServiceImpl
participant Cache as Address Cache
participant Dune as Dune Analytics
Client->>APIRoute: GET /affiliate/trader-activity/:address
APIRoute->>Service: getTraderActivity(address)
Service->>Service: normalize address (getAddressKey)
Service->>Cache: check cache for key
alt cache hit
Cache-->>Service: cached TraderActivityResult
else cache miss
Service->>Dune: execute latest traderActivity query
Dune-->>Service: raw rows + lastUpdatedAt
Service->>Service: isTraderActivityRowRaw & normalizeTraderActivityRow
Service->>Service: filter by normalized address & limit 50
Service->>Cache: store TraderActivityResult
end
Service-->>APIRoute: TraderActivityResult
APIRoute-->>Client: 200 JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
kernelwhisperer
force-pushed
the
fix/auto-fix-lint
branch
from
404fa93 to
4317481
Compare
kernelwhisperer
force-pushed
the
feat/affiliate-trader-activity-table
branch
from
ae1cccd to
07a32da
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Danziger
reviewed
May 5, 2026
Danziger
approved these changes
May 5, 2026
kernelwhisperer
force-pushed
the
feat/affiliate-trader-activity-table
branch
from
bf709b8 to
3aeb458
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
libs/services/src/AffiliateStatsService/AffiliateStatsService.utils.ts (1)
141-150: 💤 Low valuePrefer
=== undefinedover a truthy check ingetChainId.
if (!chainId)would also treat a chain ID of0as "unsupported". No SupportedChainId today is0, so this is not a real bug, but=== undefinedis more robust to enum changes and is consistent with how aRecordlookup is normally handled.- const chainId = BLOCKCHAIN_TO_CHAIN_ID[normalizedBlockchain] - - if (!chainId) { + const chainId = BLOCKCHAIN_TO_CHAIN_ID[normalizedBlockchain] + + if (chainId === undefined) { throw new Error(`Unsupported affiliate trader activity blockchain: ${blockchain}`) }🤖 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 `@libs/services/src/AffiliateStatsService/AffiliateStatsService.utils.ts` around lines 141 - 150, The lookup in getChainId uses a truthy check which would misclassify a valid chainId of 0 as unsupported; change the conditional that checks BLOCKCHAIN_TO_CHAIN_ID[normalizedBlockchain] to explicitly compare to undefined (e.g., if (chainId === undefined)) so only missing entries trigger the error, keeping the Error throw message and return of SupportedChainId intact; update the references around getChainId and BLOCKCHAIN_TO_CHAIN_ID accordingly.libs/services/src/AffiliateStatsService/AffiliateStatsServiceImpl.ts (2)
70-87: 💤 Low valueRedundant error logging.
getCachedQueryalready logsAffiliate stats Dune query failed (...)on the sameerrorbefore rethrowing (line 177). The outer try/catch here re-logs essentially the same failure with a different cache-key prefix, so each Dune failure for trader activity will produce two error log entries. Either drop the outer try/catch and let the error propagate, or remove the inner log for paths that have a more specific outer handler.♻️ Possible simplification
- try { - const { rows: duneRows, lastUpdatedAt } = await this.getCachedQuery<TraderActivityRowRaw, TraderActivityDuneRow>({ - cacheKey: 'affiliate-trader-activity', - queryId: getDuneQueryIds().traderActivity, - typeAssertion: isTraderActivityRowRaw, - mapRow: normalizeTraderActivityRow, - }) - const filteredRows = duneRows.filter((row) => getAddressKey(row.trader_address) === normalizedAddress) - const rows = filteredRows.slice(0, DEFAULT_TRADER_ACTIVITY_LIMIT) - - this.setCache(cacheKey, rows, lastUpdatedAt) - - return { rows, lastUpdatedAt } - } catch (error) { - logger.error({ error }, `Affiliate trader activity Dune query failed (${cacheKey}).`) - throw error - } + const { rows: duneRows, lastUpdatedAt } = await this.getCachedQuery<TraderActivityRowRaw, TraderActivityDuneRow>({ + cacheKey: 'affiliate-trader-activity', + queryId: getDuneQueryIds().traderActivity, + typeAssertion: isTraderActivityRowRaw, + mapRow: normalizeTraderActivityRow, + }) + const filteredRows = duneRows.filter((row) => getAddressKey(row.trader_address) === normalizedAddress) + const rows = filteredRows.slice(0, DEFAULT_TRADER_ACTIVITY_LIMIT) + + this.setCache(cacheKey, rows, lastUpdatedAt) + + return { rows, lastUpdatedAt }🤖 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 `@libs/services/src/AffiliateStatsService/AffiliateStatsServiceImpl.ts` around lines 70 - 87, The outer try/catch around the block that calls getCachedQuery(...) causes duplicate error logs because getCachedQuery already logs failures; remove the outer try/catch (and its logger.error + rethrow) in the method in AffiliateStatsServiceImpl so errors from getCachedQuery propagate naturally, leaving the existing logic that filters with getAddressKey, slices with DEFAULT_TRADER_ACTIVITY_LIMIT, calls setCache(cacheKey, rows, lastUpdatedAt), and returns { rows, lastUpdatedAt } intact.
56-87: ⚖️ Poor tradeoffOperational note: full-dataset scan for a single-trader query.
getCachedQuerypaginates through the entire trader-activity Dune result (up toDUNE_MAX_ROWS = 1_000_000) before this method filters down to the requested address. The shared underlying cache amortises the cost, but on a cold cache a single request triggers a full-dataset fetch and walks every row in memory. As the dataset grows you may want to push the address filter into the Dune query (parameterised query) or into a server-side index, so per-request work scales with the trader's activity instead of the global dataset.🤖 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 `@libs/services/src/AffiliateStatsService/AffiliateStatsServiceImpl.ts` around lines 56 - 87, getTraderActivity currently fetches the entire Dune trader-activity result then filters in-memory causing full-dataset scans; change it to push the trader_address into the Dune query and cache key so the query returns only that trader's rows. Update the call to getCachedQuery (used in getTraderActivity) to pass a per-trader cacheKey (e.g., include normalizedAddress) and supply query parameters (trader_address: normalizedAddress) to the Dune query identified by getDuneQueryIds().traderActivity, keep typeAssertion isTraderActivityRowRaw and mapRow normalizeTraderActivityRow, and still apply DEFAULT_TRADER_ACTIVITY_LIMIT to the returned rows; remove the large in-memory filter over duneRows so cold-cache requests no longer paginate DUNE_MAX_ROWS.libs/services/src/AffiliateStatsService/AffiliateStatsService.config.ts (1)
1-34: 💤 Low valueConsider extracting the env-var parsing into a helper.
The "read, error if missing, parseInt, error if NaN" pattern is now duplicated three times and will grow as more Dune queries are added. A small helper would centralize the validation messages and reduce noise.
♻️ Possible refactor
+function readIntEnv(name: string): number { + const raw = process.env[name] + if (!raw) { + throw new Error(`${name} is not set`) + } + const value = Number.parseInt(raw, 10) + if (Number.isNaN(value)) { + throw new Error(`${name} must be an integer`) + } + return value +} + export function getDuneQueryIds(): { traderStats: number traderActivity: number affiliateStats: number } { - const traderRaw = process.env.DUNE_QUERY_ID_TRADER_STATS - if (!traderRaw) { - throw new Error('DUNE_QUERY_ID_TRADER_STATS is not set') - } - const traderStats = Number.parseInt(traderRaw, 10) - if (Number.isNaN(traderStats)) { - throw new Error('DUNE_QUERY_ID_TRADER_STATS must be an integer') - } - - const traderActivityRaw = process.env.DUNE_QUERY_ID_TRADER_ACTIVITY - if (!traderActivityRaw) { - throw new Error('DUNE_QUERY_ID_TRADER_ACTIVITY is not set') - } - const traderActivity = Number.parseInt(traderActivityRaw, 10) - if (Number.isNaN(traderActivity)) { - throw new Error('DUNE_QUERY_ID_TRADER_ACTIVITY must be an integer') - } - - const affiliateRaw = process.env.DUNE_QUERY_ID_AFFILIATE_STATS - if (!affiliateRaw) { - throw new Error('DUNE_QUERY_ID_AFFILIATE_STATS is not set') - } - const affiliateStats = Number.parseInt(affiliateRaw, 10) - if (Number.isNaN(affiliateStats)) { - throw new Error('DUNE_QUERY_ID_AFFILIATE_STATS must be an integer') - } - - return { traderStats, traderActivity, affiliateStats } + return { + traderStats: readIntEnv('DUNE_QUERY_ID_TRADER_STATS'), + traderActivity: readIntEnv('DUNE_QUERY_ID_TRADER_ACTIVITY'), + affiliateStats: readIntEnv('DUNE_QUERY_ID_AFFILIATE_STATS'), + } }🤖 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 `@libs/services/src/AffiliateStatsService/AffiliateStatsService.config.ts` around lines 1 - 34, Extract the repeated "read env, throw if missing, parseInt, throw if NaN" logic into a small helper (e.g., getEnvInt or parseEnvInt) and update getDuneQueryIds to call that helper for each variable (DUNE_QUERY_ID_TRADER_STATS, DUNE_QUERY_ID_TRADER_ACTIVITY, DUNE_QUERY_ID_AFFILIATE_STATS); the helper should accept the env var name, throw the same missing/invalid-int errors as currently used, and return a number so getDuneQueryIds simply returns { traderStats: getEnvInt('DUNE_QUERY_ID_TRADER_STATS'), traderActivity: getEnvInt('DUNE_QUERY_ID_TRADER_ACTIVITY'), affiliateStats: getEnvInt('DUNE_QUERY_ID_AFFILIATE_STATS') }.libs/services/src/AffiliateStatsService/AffiliateStatsServiceImpl.spec.ts (1)
114-122: 💤 Low value
jest.resetModules()has no effect here and can be removed.All module imports are static (resolved at file load time), so
resetModules()never causesAffiliateStatsServiceImplor its dependencies to re-evaluate. The call is misleading and unnecessary; settingprocess.envbefore eachnew AffiliateStatsServiceImpl(...)instantiation is sufficient.♻️ Proposed cleanup
beforeEach(() => { - jest.resetModules() process.env = { ...originalEnv, DUNE_QUERY_ID_TRADER_STATS: '101', DUNE_QUERY_ID_TRADER_ACTIVITY: '102', DUNE_QUERY_ID_AFFILIATE_STATS: '103', } })🤖 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 `@libs/services/src/AffiliateStatsService/AffiliateStatsServiceImpl.spec.ts` around lines 114 - 122, The call to jest.resetModules() in the beforeEach block is unnecessary because imports are static and it doesn't force re-evaluation of AffiliateStatsServiceImpl or its dependencies; remove the jest.resetModules() line and keep only the process.env assignment so tests set DUNE_QUERY_ID_TRADER_STATS, DUNE_QUERY_ID_TRADER_ACTIVITY, and DUNE_QUERY_ID_AFFILIATE_STATS before instantiating new AffiliateStatsServiceImpl(...) (and ensure any test setup relying on module cache is handled by resetting env or re-instantiating objects, not resetModules).apps/api/src/app/routes/affiliate/trader-activity/_address/index.ts (1)
30-39: 💤 Low valueMove DI resolution outside the request handler.
apiContainer.get(...)is called on every request for a service registered as a singleton. Resolving it once at plugin scope fails loudly at startup if the binding is missing, instead of being silently swallowed as a 500.♻️ Proposed refactor
if (!isDuneEnabled) { fastify.log.warn('DUNE_API_KEY is not set. Skipping affiliate trader activity endpoint.') return } + const affiliateStatsService = apiContainer.get<AffiliateStatsService>(affiliateStatsServiceSymbol) + fastify.get<{ Params: ParamsSchema; Reply: ResponseSchema }>( '/', { ... }, async function (request, reply) { try { - const affiliateStatsService = apiContainer.get<AffiliateStatsService>(affiliateStatsServiceSymbol) const result = await affiliateStatsService.getTraderActivity(request.params.address) return reply.send(result) } catch (error) {🤖 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/api/src/app/routes/affiliate/trader-activity/_address/index.ts` around lines 30 - 39, The handler resolves the singleton via apiContainer.get on every request which hides missing bindings at runtime; move the DI resolution out of the async request handler into the surrounding module/plugin scope so the container resolves AffiliateStatsService (using affiliateStatsServiceSymbol and the AffiliateStatsService type) once at startup and the handler (the async function(request, reply)) simply calls affiliateStatsService.getTraderActivity(request.params.address); this makes resolution fail loudly at startup if the binding is missing and avoids per-request lookups.
🤖 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 `@libs/services/src/AffiliateStatsService/AffiliateStatsService.ts`:
- Around line 16-56: TraderActivityRow and TraderActivityDuneRow are identical;
remove duplication by consolidating them into a single exported type (e.g., keep
TraderActivityRow and make TraderActivityDuneRow an alias, or delete one and
export the remaining), then update usages in
AffiliateStatsServiceImpl.getTraderActivity to use the single type and remove
the cast on cached.rows (the cast "as TraderActivityRow[]" should no longer be
needed). Ensure all imports across the codebase reference the consolidated
symbol (TraderActivityRow or the chosen alias) and update any tests/types that
referenced the removed duplicate.
In `@libs/services/src/AffiliateStatsService/AffiliateStatsServiceImpl.ts`:
- Around line 77-78: filteredRows are sliced without a client-side sort so
"recent trader activity" can be inconsistent; in AffiliateStatsServiceImpl
locate where filteredRows is computed (variable filteredRows) and sort it by
creation_date in descending order before taking the first
DEFAULT_TRADER_ACTIVITY_LIMIT entries (e.g., perform a compare on
row.creation_date -> sort((a,b)=>
b.creation_date.localeCompare(a.creation_date)) or parse to Date then compare)
so rows = filteredRows.slice(...) becomes rows = filteredRows.sort(/*desc by
creation_date*/).slice(0, DEFAULT_TRADER_ACTIVITY_LIMIT).
---
Nitpick comments:
In `@apps/api/src/app/routes/affiliate/trader-activity/_address/index.ts`:
- Around line 30-39: The handler resolves the singleton via apiContainer.get on
every request which hides missing bindings at runtime; move the DI resolution
out of the async request handler into the surrounding module/plugin scope so the
container resolves AffiliateStatsService (using affiliateStatsServiceSymbol and
the AffiliateStatsService type) once at startup and the handler (the async
function(request, reply)) simply calls
affiliateStatsService.getTraderActivity(request.params.address); this makes
resolution fail loudly at startup if the binding is missing and avoids
per-request lookups.
In `@libs/services/src/AffiliateStatsService/AffiliateStatsService.config.ts`:
- Around line 1-34: Extract the repeated "read env, throw if missing, parseInt,
throw if NaN" logic into a small helper (e.g., getEnvInt or parseEnvInt) and
update getDuneQueryIds to call that helper for each variable
(DUNE_QUERY_ID_TRADER_STATS, DUNE_QUERY_ID_TRADER_ACTIVITY,
DUNE_QUERY_ID_AFFILIATE_STATS); the helper should accept the env var name, throw
the same missing/invalid-int errors as currently used, and return a number so
getDuneQueryIds simply returns { traderStats:
getEnvInt('DUNE_QUERY_ID_TRADER_STATS'), traderActivity:
getEnvInt('DUNE_QUERY_ID_TRADER_ACTIVITY'), affiliateStats:
getEnvInt('DUNE_QUERY_ID_AFFILIATE_STATS') }.
In `@libs/services/src/AffiliateStatsService/AffiliateStatsService.utils.ts`:
- Around line 141-150: The lookup in getChainId uses a truthy check which would
misclassify a valid chainId of 0 as unsupported; change the conditional that
checks BLOCKCHAIN_TO_CHAIN_ID[normalizedBlockchain] to explicitly compare to
undefined (e.g., if (chainId === undefined)) so only missing entries trigger the
error, keeping the Error throw message and return of SupportedChainId intact;
update the references around getChainId and BLOCKCHAIN_TO_CHAIN_ID accordingly.
In `@libs/services/src/AffiliateStatsService/AffiliateStatsServiceImpl.spec.ts`:
- Around line 114-122: The call to jest.resetModules() in the beforeEach block
is unnecessary because imports are static and it doesn't force re-evaluation of
AffiliateStatsServiceImpl or its dependencies; remove the jest.resetModules()
line and keep only the process.env assignment so tests set
DUNE_QUERY_ID_TRADER_STATS, DUNE_QUERY_ID_TRADER_ACTIVITY, and
DUNE_QUERY_ID_AFFILIATE_STATS before instantiating new
AffiliateStatsServiceImpl(...) (and ensure any test setup relying on module
cache is handled by resetting env or re-instantiating objects, not
resetModules).
In `@libs/services/src/AffiliateStatsService/AffiliateStatsServiceImpl.ts`:
- Around line 70-87: The outer try/catch around the block that calls
getCachedQuery(...) causes duplicate error logs because getCachedQuery already
logs failures; remove the outer try/catch (and its logger.error + rethrow) in
the method in AffiliateStatsServiceImpl so errors from getCachedQuery propagate
naturally, leaving the existing logic that filters with getAddressKey, slices
with DEFAULT_TRADER_ACTIVITY_LIMIT, calls setCache(cacheKey, rows,
lastUpdatedAt), and returns { rows, lastUpdatedAt } intact.
- Around line 56-87: getTraderActivity currently fetches the entire Dune
trader-activity result then filters in-memory causing full-dataset scans; change
it to push the trader_address into the Dune query and cache key so the query
returns only that trader's rows. Update the call to getCachedQuery (used in
getTraderActivity) to pass a per-trader cacheKey (e.g., include
normalizedAddress) and supply query parameters (trader_address:
normalizedAddress) to the Dune query identified by
getDuneQueryIds().traderActivity, keep typeAssertion isTraderActivityRowRaw and
mapRow normalizeTraderActivityRow, and still apply DEFAULT_TRADER_ACTIVITY_LIMIT
to the returned rows; remove the large in-memory filter over duneRows so
cold-cache requests no longer paginate DUNE_MAX_ROWS.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 163e8b4a-f9db-4d75-bcc4-0f23b84967cb
📒 Files selected for processing (10)
.env.exampleapps/api/src/app/routes/affiliate/trader-activity/_address/index.tsapps/api/src/app/routes/affiliate/trader-activity/_address/traderActivity.schemas.tsapps/api/src/app/routes/ref-codes/_code/index.tslibs/services/src/AffiliateStatsService/AffiliateStatsService.config.tslibs/services/src/AffiliateStatsService/AffiliateStatsService.tslibs/services/src/AffiliateStatsService/AffiliateStatsService.types.tslibs/services/src/AffiliateStatsService/AffiliateStatsService.utils.tslibs/services/src/AffiliateStatsService/AffiliateStatsServiceImpl.spec.tslibs/services/src/AffiliateStatsService/AffiliateStatsServiceImpl.ts
limitofzero
reviewed
May 6, 2026
| async getTraderActivity(address: string): Promise<TraderActivityResult> { | ||
| const normalizedAddress = getAddressKey(address) | ||
| const cacheKey = `affiliate-trader-activity:${normalizedAddress}:${DEFAULT_TRADER_ACTIVITY_LIMIT}` | ||
| const cached = this.getCache<TraderActivityRow>(cacheKey) |
Contributor
There was a problem hiding this comment.
question: we don't restrict the cache size, and we don't have any mechanism of handling memory leaks per cache? right now the endpoint is public and any request put data to the cache. Is it ok in this case ?
just thinking: we can create some cap per cache f.e. with fifo or something like that
Contributor
Author
There was a problem hiding this comment.
right now the endpoint is public and any request put data to the cache
We only set the cache if Dune returned rows for that trader. I don't think it can be DDOS-ed.
limitofzero
reviewed
May 6, 2026
limitofzero
reviewed
May 6, 2026
limitofzero
approved these changes
May 6, 2026
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
3 participants
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
Adds GET /affiliate/trader-activity/:address, backed by Dune, returning recent trader activity with token symbols,
volume/eligibility data, and cache metadata.
To Test
.env0x9425596dc3a37aF56D8D531d74c22EB675ebBd11Deployment
DUNE_QUERY_ID_TRADER_ACTIVITY should be set to
6561349on staging and6683651on prodSummary by CodeRabbit
New Features
Enhancements
Bug Fixes
Tests
Chores