Skip to content

Commit 6d98df1

Browse files
Copilot0xrinegade
andcommitted
Fix critical security issues and implement core functionality improvements
Co-authored-by: 0xrinegade <[email protected]>
1 parent b4f2882 commit 6d98df1

File tree

5 files changed

+317
-124
lines changed

5 files changed

+317
-124
lines changed

src/cli/utils/solana.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import { Connection, Keypair, LAMPORTS_PER_SOL, PublicKey, SystemProgram, Transaction } from '@solana/web3.js';
6+
import { createMemoInstruction } from '@solana/spl-memo';
67
import bs58 from 'bs58';
78
import { isTestMode } from './config';
89

@@ -98,9 +99,15 @@ export async function sendPayment(
9899

99100
// Add memo if provided
100101
if (memo) {
101-
// Note: This would require importing @solana/spl-memo
102-
// For now, we'll skip the memo functionality
103-
console.log(`Note: Memo "${memo}" would be added to transaction`);
102+
// Truncate memo to 566 character limit for Solana
103+
const truncatedMemo = memo.length > 566 ? memo.substring(0, 563) + '...' : memo;
104+
const memoInstruction = createMemoInstruction(truncatedMemo, [fromKeypair.publicKey]);
105+
transaction.add(memoInstruction);
106+
console.log(`✓ Added memo to transaction: "${truncatedMemo}"`);
107+
108+
if (memo.length > 566) {
109+
console.warn(`⚠ Memo was truncated from ${memo.length} to 566 characters due to Solana limit`);
110+
}
104111
}
105112

106113
// Get recent blockhash

src/core/metadata.ts

Lines changed: 73 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ export function createNetworkMemoInstruction(
189189
}
190190

191191
/**
192-
* Optimize memo content for size and readability
192+
* Optimize memo content for size and readability with improved heuristics
193193
*
194194
* @param metadata The metadata to optimize
195195
* @returns Optimized metadata that fits within limits
@@ -200,6 +200,7 @@ export function optimizeMemoContent(metadata: {
200200
memo?: string;
201201
}): { label?: string; message?: string; memo?: string } {
202202
const maxLength = 566; // Solana memo limit
203+
let wasOptimized = false;
203204

204205
// Calculate current length
205206
const memoParts: string[] = [];
@@ -213,24 +214,83 @@ export function optimizeMemoContent(metadata: {
213214
return metadata; // No optimization needed
214215
}
215216

216-
// Optimize by truncating in order of priority: memo > message > label
217+
console.warn(`⚠ Memo content exceeds ${maxLength} character limit (current: ${currentLength} chars)`);
218+
console.warn(' Applying intelligent truncation to fit within Solana memo constraints...');
219+
220+
// Prioritize message over label, and preserve custom memo content when possible
217221
const optimized = { ...metadata };
218-
const overhead = ' | '.length * (memoParts.length - 1);
219-
let availableLength = maxLength - overhead;
222+
const separator = ' | ';
223+
const separatorOverhead = separator.length * (memoParts.length - 1);
224+
let availableLength = maxLength - separatorOverhead;
225+
226+
// Strategy: Allocate space dynamically based on content priority and length
227+
// Priority: 1) Custom memo (most important), 2) Message (context), 3) Label (identification)
228+
229+
// Reserve space for custom memo first (it's usually the most important)
230+
let memoSpace = 0;
231+
if (optimized.memo) {
232+
memoSpace = Math.min(optimized.memo.length, Math.floor(availableLength * 0.6)); // 60% for memo
233+
availableLength -= memoSpace;
234+
}
235+
236+
// Allocate remaining space between label and message (favor message for context)
237+
const remainingSpace = availableLength;
238+
const messageSpace = Math.min(
239+
optimized.message ? optimized.message.length : 0,
240+
Math.floor(remainingSpace * 0.7) // 70% of remaining for message
241+
);
242+
const labelSpace = remainingSpace - messageSpace;
243+
244+
// Apply truncation with ellipsis
245+
if (optimized.label && `Label: ${optimized.label}`.length > labelSpace) {
246+
const availableForLabel = Math.max(0, labelSpace - 10); // Reserve space for "Label: ..."
247+
if (availableForLabel > 3) { // Only truncate if we have meaningful space
248+
optimized.label = optimized.label.substring(0, availableForLabel - 3) + '...';
249+
wasOptimized = true;
250+
} else {
251+
// If no meaningful space, remove label entirely
252+
optimized.label = undefined;
253+
wasOptimized = true;
254+
console.warn(' ⚠ Label removed due to space constraints');
255+
}
256+
}
220257

221-
// Allocate space: label (50), message (150), memo (rest)
222-
if (optimized.label && optimized.label.length > 50) {
223-
optimized.label = optimized.label.substring(0, 47) + '...';
258+
if (optimized.message && `Message: ${optimized.message}`.length > messageSpace) {
259+
const availableForMessage = Math.max(0, messageSpace - 12); // Reserve space for "Message: ..."
260+
if (availableForMessage > 3) {
261+
optimized.message = optimized.message.substring(0, availableForMessage - 3) + '...';
262+
wasOptimized = true;
263+
} else {
264+
optimized.message = undefined;
265+
wasOptimized = true;
266+
console.warn(' ⚠ Message removed due to space constraints');
267+
}
224268
}
225-
availableLength -= optimized.label ? `Label: ${optimized.label}`.length : 0;
226269

227-
if (optimized.message && optimized.message.length > 150) {
228-
optimized.message = optimized.message.substring(0, 147) + '...';
270+
if (optimized.memo && optimized.memo.length > memoSpace) {
271+
if (memoSpace > 3) {
272+
optimized.memo = optimized.memo.substring(0, memoSpace - 3) + '...';
273+
wasOptimized = true;
274+
} else {
275+
optimized.memo = undefined;
276+
wasOptimized = true;
277+
console.warn(' ⚠ Custom memo removed due to space constraints');
278+
}
229279
}
230-
availableLength -= optimized.message ? `Message: ${optimized.message}`.length : 0;
231280

232-
if (optimized.memo && optimized.memo.length > availableLength) {
233-
optimized.memo = optimized.memo.substring(0, availableLength - 3) + '...';
281+
// Final verification
282+
const finalParts: string[] = [];
283+
if (optimized.label) finalParts.push(`Label: ${optimized.label}`);
284+
if (optimized.message) finalParts.push(`Message: ${optimized.message}`);
285+
if (optimized.memo) finalParts.push(optimized.memo);
286+
287+
const finalLength = finalParts.join(' | ').length;
288+
289+
if (wasOptimized) {
290+
console.warn(` ✓ Memo optimized: ${currentLength}${finalLength} characters`);
291+
if (finalLength > maxLength) {
292+
console.error(` ❌ ERROR: Optimization failed, still exceeds limit (${finalLength}/${maxLength})`);
293+
}
234294
}
235295

236296
return optimized;

src/core/transaction-handler.ts

Lines changed: 117 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,51 @@ interface PaymentStore {
1818
delete(id: string): Promise<boolean>;
1919
}
2020

21+
/**
22+
* Simple async mutex implementation for concurrency safety
23+
*/
24+
class AsyncMutex {
25+
private locked = false;
26+
private waitQueue: Array<() => void> = [];
27+
28+
async acquire(): Promise<void> {
29+
return new Promise<void>((resolve) => {
30+
if (!this.locked) {
31+
this.locked = true;
32+
resolve();
33+
} else {
34+
this.waitQueue.push(resolve);
35+
}
36+
});
37+
}
38+
39+
release(): void {
40+
if (this.waitQueue.length > 0) {
41+
const next = this.waitQueue.shift();
42+
if (next) next();
43+
} else {
44+
this.locked = false;
45+
}
46+
}
47+
48+
async withLock<T>(fn: () => Promise<T>): Promise<T> {
49+
await this.acquire();
50+
try {
51+
return await fn();
52+
} finally {
53+
this.release();
54+
}
55+
}
56+
}
57+
2158
/**
2259
* In-memory payment store implementation
2360
*
2461
* ⚠️ CONCURRENCY & SCALING LIMITATIONS:
25-
* This in-memory store is NOT suitable for production use beyond demos due to:
62+
* This in-memory store includes basic concurrency safety via async mutex but is still
63+
* NOT suitable for production use beyond demos due to:
2664
*
27-
* 1. NO CONCURRENCY SAFETY: Multiple concurrent operations can cause data corruption
65+
* 1. LIMITED CONCURRENCY: Basic mutex prevents corruption but reduces throughput
2866
* 2. NO PERSISTENCE: Data is lost when the process restarts
2967
* 3. NO SCALING: Limited by single-process memory constraints
3068
* 4. NO DISTRIBUTION: Cannot share data across multiple instances
@@ -37,43 +75,31 @@ interface PaymentStore {
3775
*/
3876
class MemoryPaymentStore implements PaymentStore {
3977
private payments = new Map<string, PaymentRecord>();
40-
41-
// Track concurrent operations to warn about potential issues
42-
private activeOperations = 0;
78+
private mutex = new AsyncMutex();
4379

4480
async save(record: PaymentRecord): Promise<void> {
45-
this.activeOperations++;
46-
if (this.activeOperations > 1) {
47-
console.warn('⚠️ CONCURRENCY WARNING: Multiple concurrent operations detected in MemoryPaymentStore');
48-
console.warn(' This can lead to data corruption. Consider upgrading to a database-backed store.');
49-
}
50-
51-
try {
81+
await this.mutex.withLock(async () => {
5282
this.payments.set(record.id, { ...record });
53-
} finally {
54-
this.activeOperations--;
55-
}
83+
});
5684
}
5785

5886
async load(id: string): Promise<PaymentRecord | null> {
59-
const record = this.payments.get(id);
60-
return record ? { ...record } : null;
87+
return this.mutex.withLock(async () => {
88+
const record = this.payments.get(id);
89+
return record ? { ...record } : null;
90+
});
6191
}
6292

6393
async loadByType(type: RequestType): Promise<PaymentRecord[]> {
64-
return Array.from(this.payments.values())
65-
.filter(record => record.request.type === type)
66-
.map(record => ({ ...record }));
94+
return this.mutex.withLock(async () => {
95+
return Array.from(this.payments.values())
96+
.filter(record => record.request.type === type)
97+
.map(record => ({ ...record }));
98+
});
6799
}
68100

69101
async update(id: string, updates: Partial<PaymentRecord>): Promise<PaymentRecord> {
70-
this.activeOperations++;
71-
if (this.activeOperations > 1) {
72-
console.warn('⚠️ CONCURRENCY WARNING: Multiple concurrent operations detected in MemoryPaymentStore');
73-
console.warn(' This can lead to race conditions. Consider upgrading to a database with transactions.');
74-
}
75-
76-
try {
102+
return this.mutex.withLock(async () => {
77103
const existing = this.payments.get(id);
78104
if (!existing) {
79105
throw new Error(`Payment record not found: ${id}`);
@@ -87,55 +113,106 @@ class MemoryPaymentStore implements PaymentStore {
87113

88114
this.payments.set(id, updated);
89115
return { ...updated };
90-
} finally {
91-
this.activeOperations--;
92-
}
116+
});
93117
}
94118

95119
async delete(id: string): Promise<boolean> {
96-
return this.payments.delete(id);
120+
return this.mutex.withLock(async () => {
121+
return this.payments.delete(id);
122+
});
97123
}
98124
}
99125

100126
/**
101127
* Payment ID parser for extracting type and network information
102128
*/
103129
class PaymentIdParser {
130+
// Regex pattern for valid payment ID format
131+
private static readonly ID_PATTERN = /^([a-zA-Z]+)_([a-zA-Z0-9]+)_(\d+)_([a-zA-Z0-9]+)$/;
132+
104133
/**
105-
* Parse payment ID to extract metadata
134+
* Parse payment ID to extract metadata with robust validation
106135
*/
107136
static parse(paymentId: string): {
108137
id: string;
109138
type?: RequestType;
110139
network?: SVMNetwork;
111140
timestamp?: number;
112141
} {
113-
// Expected format: {type}_{network}_{timestamp}_{random}
114-
const parts = paymentId.split('_');
142+
if (!paymentId || typeof paymentId !== 'string') {
143+
throw new Error('Invalid payment ID: must be a non-empty string');
144+
}
115145

116-
if (parts.length >= 4) {
117-
const [typeStr, networkStr, timestampStr] = parts;
146+
// Test against expected format: {type}_{network}_{timestamp}_{random}
147+
const match = this.ID_PATTERN.exec(paymentId);
148+
149+
if (match) {
150+
const [, typeStr, networkStr, timestampStr] = match;
151+
152+
// Validate type
153+
const type = typeStr.toUpperCase() as RequestType;
154+
if (!Object.values(RequestType).includes(type)) {
155+
console.warn(`Warning: Unknown request type '${typeStr}' in payment ID ${paymentId}`);
156+
}
157+
158+
// Validate network
159+
const network = networkStr.toUpperCase() as SVMNetwork;
160+
if (!Object.values(SVMNetwork).includes(network)) {
161+
console.warn(`Warning: Unknown network '${networkStr}' in payment ID ${paymentId}`);
162+
}
163+
164+
// Validate timestamp
165+
const timestamp = parseInt(timestampStr, 10);
166+
if (isNaN(timestamp) || timestamp <= 0) {
167+
throw new Error(`Invalid timestamp in payment ID: ${paymentId}`);
168+
}
118169

119170
return {
120171
id: paymentId,
121-
type: typeStr.toUpperCase() as RequestType,
122-
network: networkStr.toUpperCase() as SVMNetwork,
123-
timestamp: parseInt(timestampStr, 10)
172+
type,
173+
network,
174+
timestamp
124175
};
125176
}
126177

178+
// Log warning for malformed IDs but don't throw to maintain backward compatibility
179+
console.warn(`Warning: Payment ID '${paymentId}' does not match expected format. Using as simple ID.`);
180+
console.warn('Expected format: {type}_{network}_{timestamp}_{random}');
181+
127182
// Fallback for simple IDs
128183
return { id: paymentId };
129184
}
130185

131186
/**
132-
* Generate a structured payment ID
187+
* Generate a structured payment ID with validation
133188
*/
134189
static generate(type: RequestType, network: SVMNetwork): string {
190+
if (!type || !network) {
191+
throw new Error('Type and network are required to generate payment ID');
192+
}
193+
194+
if (!Object.values(RequestType).includes(type)) {
195+
throw new Error(`Invalid request type: ${type}`);
196+
}
197+
198+
if (!Object.values(SVMNetwork).includes(network)) {
199+
throw new Error(`Invalid network: ${network}`);
200+
}
201+
135202
const timestamp = Date.now();
136203
const random = Math.random().toString(36).substring(2, 8);
137204
return `${type.toLowerCase()}_${network.toLowerCase()}_${timestamp}_${random}`;
138205
}
206+
207+
/**
208+
* Validate payment ID format without parsing
209+
*/
210+
static isValid(paymentId: string): boolean {
211+
if (!paymentId || typeof paymentId !== 'string') {
212+
return false;
213+
}
214+
return this.ID_PATTERN.test(paymentId);
215+
}
139216
}
140217

141218
/**

0 commit comments

Comments
 (0)