Skip to content

Commit 828ff2a

Browse files
rz1989srz1989s
authored
Merge pull request #2 from RECTOR-LABS/dev
fix: address remaining code review findings (I4, S1-S5)
2 parents a2dbb3a + f999aad commit 828ff2a

12 files changed

Lines changed: 494 additions & 58 deletions

‎src/app.test.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,35 @@ describe('DevSOL App', () => {
173173
db.close();
174174
});
175175

176+
it('applies stricter rate limit (10/min) on /sell endpoint', async () => {
177+
const { db, pricing } = makeDeps();
178+
const treasury = makeTreasuryStub();
179+
const x402 = makeX402();
180+
const payout = makePayoutStub();
181+
const { app } = createApp({
182+
pricing, db, treasury: treasury as any, x402, payout,
183+
});
184+
185+
const sellBody = JSON.stringify({ wallet: 'TestWa11et111XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX', amount_sol: 1 });
186+
const headers = { 'Content-Type': 'application/json', 'x-forwarded-for': '5.5.5.5' };
187+
188+
// 10 sell requests should pass
189+
for (let i = 0; i < 10; i++) {
190+
const res = await app.request('/sell', { method: 'POST', headers, body: sellBody });
191+
expect(res.status).toBe(200);
192+
}
193+
194+
// 11th should be rate limited
195+
const limited = await app.request('/sell', { method: 'POST', headers, body: sellBody });
196+
expect(limited.status).toBe(429);
197+
198+
// Other endpoints still work for the same IP
199+
const healthRes = await app.request('/health', { headers });
200+
expect(healthRes.status).toBe(200);
201+
202+
db.close();
203+
});
204+
176205
it('accepts payout as optional dep and wires to routes', async () => {
177206
const { db, pricing } = makeDeps();
178207
const treasury = makeTreasuryStub();
@@ -196,12 +225,13 @@ describe('DevSOL App', () => {
196225
const sellBody = await sellRes.json();
197226
expect(sellBody.status).toBe('pending');
198227

199-
// Health detail should include payout info
228+
// Health detail should include payout + facilitator info
200229
const healthRes = await app.request('/health/detail');
201230
expect(healthRes.status).toBe(200);
202231
const healthBody = await healthRes.json();
203232
expect(healthBody.payout_usdc).toBe(500);
204233
expect(healthBody.payout_wallet).toBe('PayoutWa11et1111111111111111111111111111111');
234+
expect(healthBody.facilitator_reachable).toBe(true);
205235

206236
db.close();
207237
});

‎src/app.ts‎

Lines changed: 63 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ import { TransactionDB } from './db/sqlite.js';
1414
import { config } from './config.js';
1515

1616
const RATE_LIMIT_MAX = 60;
17+
const RATE_LIMIT_STRICT_MAX = 10;
1718
const RATE_LIMIT_WINDOW_MS = 60_000;
18-
const RATE_LIMIT_CLEANUP_THRESHOLD = 10_000;
19+
const RATE_LIMIT_CLEANUP_INTERVAL = 100;
1920

2021
interface AppDeps {
2122
pricing?: PricingService;
@@ -37,23 +38,56 @@ export function createApp(deps?: AppDeps) {
3738

3839
// Rate limiting (simple in-memory)
3940
const rateLimits = new Map<string, { count: number; resetAt: number }>();
40-
app.use('*', async (c, next) => {
41-
const forwarded = c.req.header('x-forwarded-for');
42-
const ip = forwarded ? forwarded.split(',').pop()!.trim() : 'unknown';
43-
const now = Date.now();
44-
const entry = rateLimits.get(ip);
41+
const strictRateLimits = new Map<string, { count: number; resetAt: number }>();
42+
let requestCounter = 0;
43+
44+
// First IP in X-Forwarded-For is the original client; subsequent entries
45+
// are proxies that appended themselves. Spoofable, but rate limiting by
46+
// first IP is the standard approach behind a trusted reverse proxy.
47+
function getClientIp(forwarded: string | undefined): string {
48+
if (!forwarded) return 'unknown';
49+
const first = forwarded.split(',')[0]?.trim();
50+
return first || 'unknown';
51+
}
52+
53+
function checkRateLimit(
54+
map: Map<string, { count: number; resetAt: number }>,
55+
ip: string,
56+
max: number,
57+
now: number,
58+
): boolean {
59+
const entry = map.get(ip);
4560
if (entry && entry.resetAt > now) {
46-
if (entry.count >= RATE_LIMIT_MAX) {
47-
return c.json({ error: 'Rate limit exceeded' }, 429);
48-
}
61+
if (entry.count >= max) return false;
4962
entry.count++;
5063
} else {
51-
rateLimits.set(ip, { count: 1, resetAt: now + RATE_LIMIT_WINDOW_MS });
52-
if (rateLimits.size > RATE_LIMIT_CLEANUP_THRESHOLD) {
53-
for (const [key, val] of rateLimits) {
54-
if (val.resetAt <= now) rateLimits.delete(key);
55-
}
56-
}
64+
map.set(ip, { count: 1, resetAt: now + RATE_LIMIT_WINDOW_MS });
65+
}
66+
return true;
67+
}
68+
69+
function evictExpired(now: number) {
70+
for (const [key, val] of rateLimits) {
71+
if (val.resetAt <= now) rateLimits.delete(key);
72+
}
73+
for (const [key, val] of strictRateLimits) {
74+
if (val.resetAt <= now) strictRateLimits.delete(key);
75+
}
76+
}
77+
78+
app.use('*', async (c, next) => {
79+
const ip = getClientIp(c.req.header('x-forwarded-for'));
80+
const now = Date.now();
81+
82+
// Periodic cleanup
83+
requestCounter++;
84+
if (requestCounter % RATE_LIMIT_CLEANUP_INTERVAL === 0) {
85+
evictExpired(now);
86+
}
87+
88+
if (!checkRateLimit(rateLimits, ip, RATE_LIMIT_MAX, now)) {
89+
console.warn(`Rate limit hit: ${ip} on ${c.req.path}`);
90+
return c.json({ error: 'Rate limit exceeded' }, 429);
5791
}
5892
await next();
5993
});
@@ -64,7 +98,20 @@ export function createApp(deps?: AppDeps) {
6498
app.route('/', txRoutes(db));
6599

66100
if (deps?.treasury && deps?.x402) {
67-
app.route('/', treasuryRoutes(deps.treasury, deps.payout));
101+
// Stricter rate limit for state-changing endpoints
102+
const strictPrefixes = ['/buy', '/sell'];
103+
app.use('*', async (c, next) => {
104+
if (!strictPrefixes.some((p) => c.req.path === p || c.req.path.startsWith(p + '/'))) return next();
105+
const ip = getClientIp(c.req.header('x-forwarded-for'));
106+
const now = Date.now();
107+
if (!checkRateLimit(strictRateLimits, ip, RATE_LIMIT_STRICT_MAX, now)) {
108+
console.warn(`Strict rate limit hit: ${ip} on ${c.req.path}`);
109+
return c.json({ error: 'Rate limit exceeded' }, 429);
110+
}
111+
await next();
112+
});
113+
114+
app.route('/', treasuryRoutes(deps.treasury, deps.payout, deps.x402?.facilitator));
68115
app.route('/', buyRoutes({ db, pricing, treasury: deps.treasury, x402: deps.x402 }));
69116
app.route('/', sellRoutes({ db, pricing, treasuryAddress: deps.treasury.address, payout: deps.payout }));
70117
}

‎src/deposit-handler.test.ts‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { describe, it, expect, vi, beforeEach } from 'vitest';
2+
import { handleDeposit } from './deposit-handler.js';
3+
import type { Transaction } from './db/sqlite.js';
4+
5+
function makeTx(overrides?: Partial<Transaction>): Transaction {
6+
return {
7+
id: 'sell-001',
8+
type: 'sell',
9+
wallet: 'Se11erWa11etXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX',
10+
sol_amount: 5,
11+
usdc_amount: 4.75,
12+
mainnet_tx: null,
13+
devnet_tx: 'devnet_sig_123',
14+
mainnet_payout_tx: null,
15+
memo: 'devsol-test1',
16+
status: 'completed',
17+
created_at: '2026-01-01T00:00:00Z',
18+
updated_at: '2026-01-01T00:00:00Z',
19+
...overrides,
20+
};
21+
}
22+
23+
function makeDeps() {
24+
return {
25+
payout: {
26+
canAffordPayout: vi.fn(async () => true),
27+
sendUsdc: vi.fn(async () => 'mainnet_payout_sig'),
28+
},
29+
treasury: {
30+
sendSol: vi.fn(async () => 'refund_sig_123'),
31+
},
32+
db: {
33+
update: vi.fn(),
34+
},
35+
};
36+
}
37+
38+
describe('handleDeposit', () => {
39+
beforeEach(() => vi.clearAllMocks());
40+
41+
it('logs warning and returns when no payout service', async () => {
42+
const deps = makeDeps();
43+
const tx = makeTx();
44+
45+
await handleDeposit(tx, 'devnet_sig', { ...deps, payout: undefined });
46+
47+
expect(deps.payout.sendUsdc).not.toHaveBeenCalled();
48+
expect(deps.db.update).not.toHaveBeenCalled();
49+
});
50+
51+
it('sends USDC payout and updates DB on success', async () => {
52+
const deps = makeDeps();
53+
const tx = makeTx();
54+
55+
await handleDeposit(tx, 'devnet_sig', deps);
56+
57+
expect(deps.payout.canAffordPayout).toHaveBeenCalledWith(4.75);
58+
expect(deps.payout.sendUsdc).toHaveBeenCalledWith(tx.wallet, 4.75);
59+
expect(deps.db.update).toHaveBeenCalledWith('sell-001', { mainnet_payout_tx: 'mainnet_payout_sig' });
60+
});
61+
62+
it('refunds SOL when reserves insufficient', async () => {
63+
const deps = makeDeps();
64+
deps.payout.canAffordPayout.mockResolvedValue(false);
65+
const tx = makeTx();
66+
67+
await handleDeposit(tx, 'devnet_sig', deps);
68+
69+
expect(deps.payout.sendUsdc).not.toHaveBeenCalled();
70+
expect(deps.treasury.sendSol).toHaveBeenCalledWith(tx.wallet, 5);
71+
expect(deps.db.update).toHaveBeenCalledWith('sell-001', { status: 'refunded' });
72+
});
73+
74+
it('refunds SOL when payout throws', async () => {
75+
const deps = makeDeps();
76+
deps.payout.sendUsdc.mockRejectedValue(new Error('RPC timeout'));
77+
const tx = makeTx();
78+
79+
await handleDeposit(tx, 'devnet_sig', deps);
80+
81+
expect(deps.treasury.sendSol).toHaveBeenCalledWith(tx.wallet, 5);
82+
expect(deps.db.update).toHaveBeenCalledWith('sell-001', { status: 'refunded' });
83+
});
84+
85+
it('sets status failed when both payout and refund throw', async () => {
86+
const deps = makeDeps();
87+
deps.payout.sendUsdc.mockRejectedValue(new Error('RPC timeout'));
88+
deps.treasury.sendSol.mockRejectedValue(new Error('Devnet down'));
89+
const tx = makeTx();
90+
91+
await handleDeposit(tx, 'devnet_sig', deps);
92+
93+
expect(deps.db.update).toHaveBeenCalledWith('sell-001', { status: 'failed' });
94+
});
95+
});

‎src/deposit-handler.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import type { Transaction, UpdateTransactionInput } from './db/sqlite.js';
2+
3+
interface DepositDeps {
4+
payout?: {
5+
canAffordPayout(usdcAmount: number): Promise<boolean>;
6+
sendUsdc(recipient: string, usdcAmount: number): Promise<string>;
7+
};
8+
treasury: {
9+
sendSol(recipient: string, solAmount: number): Promise<string>;
10+
};
11+
db: {
12+
update(id: string, data: UpdateTransactionInput): void;
13+
};
14+
}
15+
16+
export async function handleDeposit(
17+
tx: Transaction,
18+
devnetSig: string,
19+
deps: DepositDeps,
20+
): Promise<void> {
21+
console.log(`Deposit confirmed for sell ${tx.id}: ${devnetSig}`);
22+
23+
if (!deps.payout) {
24+
console.warn(`No payout service — sell ${tx.id} completed without USDC payout`);
25+
return;
26+
}
27+
28+
try {
29+
const canPay = await deps.payout.canAffordPayout(tx.usdc_amount);
30+
if (!canPay) {
31+
console.error(`Insufficient USDC reserves for sell ${tx.id} — refunding`);
32+
const refundSig = await deps.treasury.sendSol(tx.wallet, tx.sol_amount);
33+
deps.db.update(tx.id, { status: 'refunded' });
34+
console.log(`Refunded ${tx.sol_amount} SOL to ${tx.wallet}: ${refundSig}`);
35+
return;
36+
}
37+
38+
const mainnetSig = await deps.payout.sendUsdc(tx.wallet, tx.usdc_amount);
39+
deps.db.update(tx.id, { mainnet_payout_tx: mainnetSig });
40+
console.log(`USDC payout sent for sell ${tx.id}: ${mainnetSig}`);
41+
} catch (err) {
42+
console.error(`USDC payout failed for sell ${tx.id}:`, err);
43+
try {
44+
const refundSig = await deps.treasury.sendSol(tx.wallet, tx.sol_amount);
45+
deps.db.update(tx.id, { status: 'refunded' });
46+
console.log(`Refunded ${tx.sol_amount} SOL to ${tx.wallet}: ${refundSig} (original deposit: ${tx.devnet_tx})`);
47+
} catch (refundErr) {
48+
console.error(`CRITICAL: Refund also failed for sell ${tx.id}:`, refundErr);
49+
deps.db.update(tx.id, { status: 'failed' });
50+
}
51+
}
52+
}

‎src/index.ts‎

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { TreasuryService } from './services/treasury.js';
77
import { X402Service } from './services/x402.js';
88
import { PayoutService } from './services/payout.js';
99
import { DepositDetector } from './services/deposit.js';
10+
import { handleDeposit } from './deposit-handler.js';
1011
import { config } from './config.js';
1112

1213
async function main() {
@@ -54,37 +55,7 @@ async function main() {
5455
db,
5556
rpc: devnetRpc as any,
5657
treasuryAddress: treasury.address,
57-
onDeposit: async (tx, devnetSig) => {
58-
console.log(`Deposit confirmed for sell ${tx.id}: ${devnetSig}`);
59-
if (!payout) {
60-
console.warn(`No payout service — sell ${tx.id} completed without USDC payout`);
61-
return;
62-
}
63-
try {
64-
const canPay = await payout.canAffordPayout(tx.usdc_amount);
65-
if (!canPay) {
66-
console.error(`Insufficient USDC reserves for sell ${tx.id} — refunding`);
67-
const refundSig = await treasury.sendSol(tx.wallet, tx.sol_amount);
68-
db.update(tx.id, { status: 'refunded' });
69-
console.log(`Refunded ${tx.sol_amount} SOL to ${tx.wallet}: ${refundSig}`);
70-
return;
71-
}
72-
const mainnetSig = await payout.sendUsdc(tx.wallet, tx.usdc_amount);
73-
db.update(tx.id, { mainnet_payout_tx: mainnetSig });
74-
console.log(`USDC payout sent for sell ${tx.id}: ${mainnetSig}`);
75-
} catch (err) {
76-
console.error(`USDC payout failed for sell ${tx.id}:`, err);
77-
// Refund devnet SOL
78-
try {
79-
const refundSig = await treasury.sendSol(tx.wallet, tx.sol_amount);
80-
db.update(tx.id, { status: 'refunded' });
81-
console.log(`Refunded ${tx.sol_amount} SOL to ${tx.wallet}: ${refundSig} (original deposit: ${tx.devnet_tx})`);
82-
} catch (refundErr) {
83-
console.error(`CRITICAL: Refund also failed for sell ${tx.id}:`, refundErr);
84-
db.update(tx.id, { status: 'failed' });
85-
}
86-
}
87-
},
58+
onDeposit: (tx, sig) => handleDeposit(tx, sig, { payout, treasury, db }),
8859
});
8960
depositDetector.start();
9061

0 commit comments

Comments
 (0)