Merged
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: New stats endpoint missing address name resolution
- The stats endpoint now resolves
.chainnames throughPortfolioService.resolveAccountAddressbefore querying trading stats so name-based lookups hit storedak_addresses.
- The stats endpoint now resolves
- ✅ Fixed: Top win mixes AE and USD from different trades
- The trading-stats SQL now selects
top_win_aeandtop_win_usdfrom a single highest-gain sell row via atop_winCTE instead of independent MAX aggregations.
- The trading-stats SQL now selects
Preview
diff --git a/src/account/controllers/accounts.controller.spec.ts b/src/account/controllers/accounts.controller.spec.ts
--- a/src/account/controllers/accounts.controller.spec.ts
+++ b/src/account/controllers/accounts.controller.spec.ts
@@ -23,6 +23,12 @@ describe('AccountsController', () => {
let accountService: {
getChainNameForAccount: jest.Mock;
};
+ let portfolioService: {
+ resolveAccountAddress: jest.Mock;
+ };
+ let bclPnlService: {
+ calculateTradingStats: jest.Mock;
+ };
let profileReadService: {
getProfile: jest.Mock;
};
@@ -23,6 +23,12 @@ describe('AccountsController', () => {
let accountService: {
getChainNameForAccount: jest.Mock;
};
+ let portfolioService: {
+ resolveAccountAddress: jest.Mock;
+ };
+ let bclPnlService: {
+ calculateTradingStats: jest.Mock;
+ };
let profileReadService: {
getProfile: jest.Mock;
};
@@ -37,28 +43,46 @@ describe('AccountsController', () => {
accountService = {
getChainNameForAccount: jest.fn(),
};
+ portfolioService = {
+ resolveAccountAddress: jest.fn(),
+ };
+ bclPnlService = {
+ calculateTradingStats: jest.fn(),
+ };
profileReadService = {
getProfile: jest.fn(),
};
controller = new AccountsController(
accountRepository as any,
- {} as any,
+ portfolioService as any,
+ bclPnlService as any,
accountService as any,
profileReadService as any,
);
});
it('returns paginated accounts', async () => {
- const result = await controller.listAll(undefined, 1, 100, 'total_volume', 'DESC');
+ const result = await controller.listAll(
+ undefined,
+ 1,
+ 100,
+ 'total_volume',
+ 'DESC',
+ );
- expect(accountRepository.createQueryBuilder).toHaveBeenCalledWith('account');
+ expect(accountRepository.createQueryBuilder).toHaveBeenCalledWith(
+ 'account',
+ );
expect(queryBuilder.leftJoin).not.toHaveBeenCalled();
expect(queryBuilder.orderBy).toHaveBeenCalledWith(
'account.total_volume',
'DESC',
);
- expect(paginate).toHaveBeenCalledWith(queryBuilder, { page: 1, limit: 100 });
+ expect(paginate).toHaveBeenCalledWith(queryBuilder, {
+ page: 1,
+ limit: 100,
+ });
expect(result).toEqual({ items: [], meta: {} });
});
@@ -37,28 +43,46 @@ describe('AccountsController', () => {
accountService = {
getChainNameForAccount: jest.fn(),
};
+ portfolioService = {
+ resolveAccountAddress: jest.fn(),
+ };
+ bclPnlService = {
+ calculateTradingStats: jest.fn(),
+ };
profileReadService = {
getProfile: jest.fn(),
};
controller = new AccountsController(
accountRepository as any,
- {} as any,
+ portfolioService as any,
+ bclPnlService as any,
accountService as any,
profileReadService as any,
);
});
it('returns paginated accounts', async () => {
- const result = await controller.listAll(undefined, 1, 100, 'total_volume', 'DESC');
+ const result = await controller.listAll(
+ undefined,
+ 1,
+ 100,
+ 'total_volume',
+ 'DESC',
+ );
- expect(accountRepository.createQueryBuilder).toHaveBeenCalledWith('account');
+ expect(accountRepository.createQueryBuilder).toHaveBeenCalledWith(
+ 'account',
+ );
expect(queryBuilder.leftJoin).not.toHaveBeenCalled();
expect(queryBuilder.orderBy).toHaveBeenCalledWith(
'account.total_volume',
'DESC',
);
- expect(paginate).toHaveBeenCalledWith(queryBuilder, { page: 1, limit: 100 });
+ expect(paginate).toHaveBeenCalledWith(queryBuilder, {
+ page: 1,
+ limit: 100,
+ });
expect(result).toEqual({ items: [], meta: {} });
});
@@ -91,4 +115,35 @@ describe('AccountsController', () => {
NotFoundException,
);
});
+
+ it('resolves .chain addresses for portfolio stats queries', async () => {
+ portfolioService.resolveAccountAddress.mockResolvedValue('ak_resolved');
+ bclPnlService.calculateTradingStats.mockResolvedValue({
+ topWin: { ae: 5, usd: 10 },
+ unrealizedProfit: { ae: 20, usd: 40 },
+ winRate: 50,
+ avgDurationSeconds: 3600,
+ totalTrades: 4,
+ winningTrades: 2,
+ });
+
+ const result = await controller.getPortfolioStats('alice.chain', {});
+
+ expect(portfolioService.resolveAccountAddress).toHaveBeenCalledWith(
+ 'alice.chain',
+ );
+ expect(bclPnlService.calculateTradingStats).toHaveBeenCalledWith(
+ 'ak_resolved',
+ expect.any(Date),
+ expect.any(Date),
+ );
+ expect(result).toEqual({
+ top_win: { ae: 5, usd: 10 },
+ unrealized_profit: { ae: 20, usd: 40 },
+ win_rate: 50,
+ avg_duration_seconds: 3600,
+ total_trades: 4,
+ winning_trades: 2,
+ });
+ });
});
@@ -91,4 +115,35 @@ describe('AccountsController', () => {
NotFoundException,
);
});
+
+ it('resolves .chain addresses for portfolio stats queries', async () => {
+ portfolioService.resolveAccountAddress.mockResolvedValue('ak_resolved');
+ bclPnlService.calculateTradingStats.mockResolvedValue({
+ topWin: { ae: 5, usd: 10 },
+ unrealizedProfit: { ae: 20, usd: 40 },
+ winRate: 50,
+ avgDurationSeconds: 3600,
+ totalTrades: 4,
+ winningTrades: 2,
+ });
+
+ const result = await controller.getPortfolioStats('alice.chain', {});
+
+ expect(portfolioService.resolveAccountAddress).toHaveBeenCalledWith(
+ 'alice.chain',
+ );
+ expect(bclPnlService.calculateTradingStats).toHaveBeenCalledWith(
+ 'ak_resolved',
+ expect.any(Date),
+ expect.any(Date),
+ );
+ expect(result).toEqual({
+ top_win: { ae: 5, usd: 10 },
+ unrealized_profit: { ae: 20, usd: 40 },
+ win_rate: 50,
+ avg_duration_seconds: 3600,
+ total_trades: 4,
+ winning_trades: 2,
+ });
+ });
});
diff --git a/src/account/controllers/accounts.controller.ts b/src/account/controllers/accounts.controller.ts
--- a/src/account/controllers/accounts.controller.ts
+++ b/src/account/controllers/accounts.controller.ts
@@ -208,9 +208,11 @@ export class AccountsController {
? moment(query.startDate).toDate()
: moment().subtract(30, 'days').toDate();
const end = query.endDate ? moment(query.endDate).toDate() : new Date();
+ const resolvedAddress =
+ await this.portfolioService.resolveAccountAddress(address);
const stats = await this.bclPnlService.calculateTradingStats(
- address,
+ resolvedAddress,
start,
end,
);
@@ -208,9 +208,11 @@ export class AccountsController {
? moment(query.startDate).toDate()
: moment().subtract(30, 'days').toDate();
const end = query.endDate ? moment(query.endDate).toDate() : new Date();
+ const resolvedAddress =
+ await this.portfolioService.resolveAccountAddress(address);
const stats = await this.bclPnlService.calculateTradingStats(
- address,
+ resolvedAddress,
start,
end,
);
diff --git a/src/account/services/bcl-pnl.service.spec.ts b/src/account/services/bcl-pnl.service.spec.ts
--- a/src/account/services/bcl-pnl.service.spec.ts
+++ b/src/account/services/bcl-pnl.service.spec.ts
@@ -134,10 +134,7 @@ describe('BclPnlService', () => {
},
]);
- const result = await service.calculateTokenPnlsBatch(
- 'ak_test',
- [200, 300],
- );
+ const result = await service.calculateTokenPnlsBatch('ak_test', [200, 300]);
// Single DB round-trip regardless of number of heights
expect(transactionRepository.query).toHaveBeenCalledTimes(1);
@@ -134,10 +134,7 @@ describe('BclPnlService', () => {
},
]);
- const result = await service.calculateTokenPnlsBatch(
- 'ak_test',
- [200, 300],
- );
+ const result = await service.calculateTokenPnlsBatch('ak_test', [200, 300]);
// Single DB round-trip regardless of number of heights
expect(transactionRepository.query).toHaveBeenCalledTimes(1);
@@ -151,7 +148,9 @@ describe('BclPnlService', () => {
expect(sql).not.toContain('JOIN transactions tx'); // no repeated scan of transactions
expect(sql).toContain('LEFT JOIN LATERAL');
expect(sql).toContain('LIMIT 1');
- expect(sql).not.toContain('DISTINCT ON (agg.snapshot_height, agg.sale_address)');
+ expect(sql).not.toContain(
+ 'DISTINCT ON (agg.snapshot_height, agg.sale_address)',
+ );
expect(params).toEqual(['ak_test', [200, 300]]);
expect(result).toBeInstanceOf(Map);
@@ -151,7 +148,9 @@ describe('BclPnlService', () => {
expect(sql).not.toContain('JOIN transactions tx'); // no repeated scan of transactions
expect(sql).toContain('LEFT JOIN LATERAL');
expect(sql).toContain('LIMIT 1');
- expect(sql).not.toContain('DISTINCT ON (agg.snapshot_height, agg.sale_address)');
+ expect(sql).not.toContain(
+ 'DISTINCT ON (agg.snapshot_height, agg.sale_address)',
+ );
expect(params).toEqual(['ak_test', [200, 300]]);
expect(result).toBeInstanceOf(Map);
@@ -291,7 +290,7 @@ describe('BclPnlService', () => {
{
sale_address: 'ct_pre_bought',
current_holdings: '0',
- total_volume_bought: '0', // no buys in range
+ total_volume_bought: '0', // no buys in range
total_amount_spent_ae: '0', // no buys in range
total_amount_spent_usd: '0',
total_amount_received_ae: '80',
@@ -291,7 +290,7 @@ describe('BclPnlService', () => {
{
sale_address: 'ct_pre_bought',
current_holdings: '0',
- total_volume_bought: '0', // no buys in range
+ total_volume_bought: '0', // no buys in range
total_amount_spent_ae: '0', // no buys in range
total_amount_spent_usd: '0',
total_amount_received_ae: '80',
@@ -474,6 +473,10 @@ describe('BclPnlService', () => {
expect(sql).toContain('address_txs AS MATERIALIZED');
expect(sql).toContain('token_agg');
expect(sql).toContain('range_sells');
+ expect(sql).toContain('top_win AS');
+ expect(sql).toContain('ORDER BY gain_ae DESC, sell_at DESC');
+ expect(sql).not.toContain('MAX(gain_ae) FILTER (WHERE gain_ae > 0)');
+ expect(sql).not.toContain('MAX(gain_usd) FILTER (WHERE gain_usd > 0)');
expect(sql).toContain('unrealized');
expect(sql).toContain('CROSS JOIN unrealized');
expect(params[0]).toBe('ak_test');
@@ -474,6 +473,10 @@ describe('BclPnlService', () => {
expect(sql).toContain('address_txs AS MATERIALIZED');
expect(sql).toContain('token_agg');
expect(sql).toContain('range_sells');
+ expect(sql).toContain('top_win AS');
+ expect(sql).toContain('ORDER BY gain_ae DESC, sell_at DESC');
+ expect(sql).not.toContain('MAX(gain_ae) FILTER (WHERE gain_ae > 0)');
+ expect(sql).not.toContain('MAX(gain_usd) FILTER (WHERE gain_usd > 0)');
expect(sql).toContain('unrealized');
expect(sql).toContain('CROSS JOIN unrealized');
expect(params[0]).toBe('ak_test');
@@ -516,6 +519,30 @@ describe('BclPnlService', () => {
expect(result.winningTrades).toBe(2);
});
+ it('calculateTradingStats keeps top_win ae and usd from same sell row', async () => {
+ const { service, transactionRepository } = createService();
+
+ transactionRepository.query.mockResolvedValue([
+ {
+ top_win_ae: '100',
+ top_win_usd: '5',
+ winning_sells: '2',
+ total_sells: '2',
+ avg_hold_secs: '1000',
+ unrealized_ae: '0',
+ unrealized_usd: '0',
+ },
+ ]);
+
+ const result = await service.calculateTradingStats(
+ 'ak_test',
+ new Date('2026-01-01'),
+ new Date('2026-01-31'),
+ );
+
+ expect(result.topWin).toEqual({ ae: 100, usd: 5 });
+ });
+
it('calculateTradingStats returns zero win_rate when no sells in range', async () => {
const { service, transactionRepository } = createService();
@@ -516,6 +519,30 @@ describe('BclPnlService', () => {
expect(result.winningTrades).toBe(2);
});
+ it('calculateTradingStats keeps top_win ae and usd from same sell row', async () => {
+ const { service, transactionRepository } = createService();
+
+ transactionRepository.query.mockResolvedValue([
+ {
+ top_win_ae: '100',
+ top_win_usd: '5',
+ winning_sells: '2',
+ total_sells: '2',
+ avg_hold_secs: '1000',
+ unrealized_ae: '0',
+ unrealized_usd: '0',
+ },
+ ]);
+
+ const result = await service.calculateTradingStats(
+ 'ak_test',
+ new Date('2026-01-01'),
+ new Date('2026-01-31'),
+ );
+
+ expect(result.topWin).toEqual({ ae: 100, usd: 5 });
+ });
+
it('calculateTradingStats returns zero win_rate when no sells in range', async () => {
const { service, transactionRepository } = createService();
diff --git a/src/account/services/bcl-pnl.service.ts b/src/account/services/bcl-pnl.service.ts
--- a/src/account/services/bcl-pnl.service.ts
+++ b/src/account/services/bcl-pnl.service.ts
@@ -217,16 +217,24 @@ export class BclPnlService {
),
range_sells_with_gain AS (
SELECT
+ sell_at,
proceeds_ae - cost_ae AS gain_ae,
proceeds_usd - cost_usd AS gain_usd,
hold_secs
FROM range_sells
),
+ top_win AS (
+ SELECT
+ gain_ae AS top_win_ae,
+ gain_usd AS top_win_usd
+ FROM range_sells_with_gain
+ WHERE gain_ae > 0
+ ORDER BY gain_ae DESC, sell_at DESC
+ LIMIT 1
+ ),
-- Aggregate sell stats over the range
sell_stats AS (
SELECT
- COALESCE(MAX(gain_ae) FILTER (WHERE gain_ae > 0), 0) AS top_win_ae,
- COALESCE(MAX(gain_usd) FILTER (WHERE gain_usd > 0), 0) AS top_win_usd,
COUNT(*) FILTER (WHERE gain_ae > 0) AS winning_sells,
COUNT(*) AS total_sells,
COALESCE(AVG(hold_secs) FILTER (WHERE hold_secs IS NOT NULL AND hold_secs >= 0), 0) AS avg_hold_secs
@@ -217,16 +217,24 @@ export class BclPnlService {
),
range_sells_with_gain AS (
SELECT
+ sell_at,
proceeds_ae - cost_ae AS gain_ae,
proceeds_usd - cost_usd AS gain_usd,
hold_secs
FROM range_sells
),
+ top_win AS (
+ SELECT
+ gain_ae AS top_win_ae,
+ gain_usd AS top_win_usd
+ FROM range_sells_with_gain
+ WHERE gain_ae > 0
+ ORDER BY gain_ae DESC, sell_at DESC
+ LIMIT 1
+ ),
-- Aggregate sell stats over the range
sell_stats AS (
SELECT
- COALESCE(MAX(gain_ae) FILTER (WHERE gain_ae > 0), 0) AS top_win_ae,
- COALESCE(MAX(gain_usd) FILTER (WHERE gain_usd > 0), 0) AS top_win_usd,
COUNT(*) FILTER (WHERE gain_ae > 0) AS winning_sells,
COUNT(*) AS total_sells,
COALESCE(AVG(hold_secs) FILTER (WHERE hold_secs IS NOT NULL AND hold_secs >= 0), 0) AS avg_hold_secs
@@ -254,19 +262,22 @@ export class BclPnlService {
WHERE ta.current_holdings > 0
)
SELECT
- ss.top_win_ae,
- ss.top_win_usd,
+ COALESCE(tw.top_win_ae, 0) AS top_win_ae,
+ COALESCE(tw.top_win_usd, 0) AS top_win_usd,
ss.winning_sells,
ss.total_sells,
ss.avg_hold_secs,
u.unrealized_ae,
u.unrealized_usd
FROM sell_stats ss
CROSS JOIN unrealized u
+ LEFT JOIN top_win tw ON true
`;
}
- private mapTradingStats(row: Record<string, any> | undefined): TradingStatsResult {
+ private mapTradingStats(
+ row: Record<string, any> | undefined,
+ ): TradingStatsResult {
if (!row) {
return {
topWin: { ae: 0, usd: 0 },
@@ -254,19 +262,22 @@ export class BclPnlService {
WHERE ta.current_holdings > 0
)
SELECT
- ss.top_win_ae,
- ss.top_win_usd,
+ COALESCE(tw.top_win_ae, 0) AS top_win_ae,
+ COALESCE(tw.top_win_usd, 0) AS top_win_usd,
ss.winning_sells,
ss.total_sells,
ss.avg_hold_secs,
u.unrealized_ae,
u.unrealized_usd
FROM sell_stats ss
CROSS JOIN unrealized u
+ LEFT JOIN top_win tw ON true
`;
}
- private mapTradingStats(row: Record<string, any> | undefined): TradingStatsResult {
+ private mapTradingStats(
+ row: Record<string, any> | undefined,
+ ): TradingStatsResult {
if (!row) {
return {
topWin: { ae: 0, usd: 0 },
@@ -895,7 +906,6 @@ export class BclPnlService {
tokenPnls: Array<Record<string, any>>,
isRangeBased: boolean,
): TokenPnlResult {
-
const result: TokenPnlResult['pnls'] = {};
let totalCostBasisAe = 0;
let totalCostBasisUsd = 0;
@@ -895,7 +906,6 @@ export class BclPnlService {
tokenPnls: Array<Record<string, any>>,
isRangeBased: boolean,
): TokenPnlResult {
-
const result: TokenPnlResult['pnls'] = {};
let totalCostBasisAe = 0;
let totalCostBasisUsd = 0;
@@ -975,7 +985,8 @@ export class BclPnlService {
costBasisUsd = totalAmountSpentUsd;
gainAe = totalAmountReceivedAe + currentValueAe - totalAmountSpentAe;
- gainUsd = totalAmountReceivedUsd + currentValueUsd - totalAmountSpentUsd;
+ gainUsd =
+ totalAmountReceivedUsd + currentValueUsd - totalAmountSpentUsd;
}
totalCostBasisAe += costBasisAe;
@@ -975,7 +985,8 @@ export class BclPnlService {
costBasisUsd = totalAmountSpentUsd;
gainAe = totalAmountReceivedAe + currentValueAe - totalAmountSpentAe;
- gainUsd = totalAmountReceivedUsd + currentValueUsd - totalAmountSpentUsd;
+ gainUsd =
+ totalAmountReceivedUsd + currentValueUsd - totalAmountSpentUsd;
}
totalCostBasisAe += costBasisAe;
diff --git a/src/account/services/portfolio.service.ts b/src/account/services/portfolio.service.ts
--- a/src/account/services/portfolio.service.ts
+++ b/src/account/services/portfolio.service.ts
@@ -378,7 +378,7 @@ export class PortfolioService {
return data;
}
- private async resolveAccountAddress(address: string): Promise<string> {
+ async resolveAccountAddress(address: string): Promise<string> {
if (!address || address.startsWith('ak_') || !address.includes('.')) {
return address;
}
@@ -378,7 +378,7 @@ export class PortfolioService {
return data;
}
- private async resolveAccountAddress(address: string): Promise<string> {
+ async resolveAccountAddress(address: string): Promise<string> {
if (!address || address.startsWith('ak_') || !address.includes('.')) {
return address;
}This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| address, | ||
| start, | ||
| end, | ||
| ); |
There was a problem hiding this comment.
New stats endpoint missing address name resolution
Medium Severity
The new getPortfolioStats endpoint passes the raw address param directly to bclPnlService.calculateTradingStats. The existing getPortfolioHistory endpoint resolves .chain names to ak_ addresses via resolveAccountAddress() in PortfolioService, but the new endpoint bypasses this. Users passing a .chain name will get all-zero results because the SQL WHERE address = $1 will never match (transactions store ak_ addresses).
… values are derived from the same trade
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.
Note
Medium Risk
Changes PnL accounting semantics and adds new SQL-heavy aggregation paths, which could impact correctness/performance of portfolio endpoints. Also introduces a new public stats API route whose results depend on complex query logic.
Overview
Adds a new
GET /accounts/:address/portfolio/statsendpoint (withTradingStats*DTOs) that returns win rate, top realized win (paired AE/USD), average hold duration, trade counts, and current unrealized profit for an address over a date range.Reworks portfolio “range PnL” to be a daily realized PnL calendar:
PortfolioServicenow builds per-snapshot[previousTs, snapshotTs)windows and calls newBclPnlService.calculateDailyPnlBatch()(timestamp-based SQL) while keeping cumulative value/PnL viacalculateTokenPnlsBatch().Updates
BclPnlServicePnL mapping semantics: cumulative PnL now treatsinvestedas total spent andgainas sells + current value − spent (including closed positions), while range/daily PnL reports realized gains only using all-time average cost for tokens sold in-window; queries were extended to include cumulative buy totals and to return tokens with sells even if holdings are zero.Hardens ops/runtime behavior:
scripts/db-restore.shresets the local DB user password after restore, andAePricingService.pullAndSaveCoinCurrencyRates()falls back to the last saved rates if saving new rates fails.Written by Cursor Bugbot for commit ffc0cbc. This will update automatically on new commits. Configure here.