Conversation
…getDecimals fix - Add LOOKUP_SOLANA_SIGNATURE action and signatureLookupProvider for tx lookups - Add transaction fetching system with pagination, caching, retry/backoff, and task worker - Add RPC rate limiter (configurable via SOLANA_RPC_RATE_LIMIT) with retry on 429s - Fix getDecimals to return Record<string, number> instead of number[] to prevent address misalignment - Simplify ca-lookup provider to use keyed decimals directly - Add rate-limiter utility, ascii-art banner, plugin-banner utils - Enhance wallet provider with address detection and multi-wallet query support - Type-safe runtime.getSetting() usage across actions and keypairUtils - Resolve merge conflicts in build.ts, package.json, tsconfig.json - Add p-limit dependency, update tsconfig to use bun-types Co-authored-by: Cursor <cursoragent@cursor.com>
WalkthroughAdds a Solana transaction-fetching pipeline (paginated, cached, retrying) with rate limiting and background task workers, signature lookup action/provider, expanded wallet/balance APIs, new types, utilities, docs, tests, and build/config updates. Changes
Sequence DiagramsequenceDiagram
actor User as User/Client
participant Runtime as IAgentRuntime
participant Service as SolanaService
participant RateLim as RateLimiter
participant RPC as Solana RPC
participant Cache as Cache Layer
participant Worker as Background Worker
User->>Service: fetchTransactionsForAddresses(addresses, options)
activate Service
Service->>Cache: check cached results
alt cache hit (and not force refresh)
Cache-->>Service: return cached data
else
Service->>Service: paginate signatures per address
loop per signature page
Service->>RateLim: acquire token
RateLim-->>Service: granted
Service->>RPC: getSignaturesForAddress()
RPC-->>Service: signatures + cursor
Service->>Cache: store checkpoint
Service->>Runtime: onProgress callback
end
Service->>Service: dedupe signatures
Service->>Service: batch fetch transactions
loop per batch
Service->>RateLim: acquire token
RateLim-->>Service: granted
Service->>RPC: getParsedTransactions()
alt RPC transient error
Service->>Service: retryWithBackoff()
end
RPC-->>Service: parsed transactions
Service->>Cache: store results
Service->>Runtime: onProgress callback
end
Service->>Worker: enqueue onComplete tasks (optional)
Worker-->>Service: acknowledge
end
Service-->>User: FetchTransactionsResult
deactivate Service
Estimated code review effort🎯 4 (Complex) | ⏱️ ~80 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
odilitime
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds richer Solana runtime capabilities (transaction history fetching + lookup) and introduces new infra (rate limiting, task worker integration), along with TypeScript/build configuration updates to support the new functionality.
Changes:
- Add RPC-based transaction history fetching (pagination, batching, caching, retries) with background task worker support.
- Add signature lookup action/provider and expand wallet provider to query arbitrary addresses from message text.
- Introduce a token-bucket rate limiter and update build/tsconfig/deps/tests/docs accordingly.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adjust TS root/types/includes/excludes for Bun/Node + build script support. |
| build.ts | Update build flow (clean script usage, declaration generation behavior). |
| package.json | Add p-limit, swap type dependencies to Node + bun-types. |
| src/service.ts | Major: rate limiting, new transaction-fetching APIs + task worker, various refactors. |
| src/types.ts | Add transaction fetching-related types. |
| src/rate-limiter.ts | New token-bucket rate limiter used for RPC throttling. |
| src/providers/wallet.ts | Expand provider to detect addresses / “own wallet” queries and query arbitrary wallets. |
| src/providers/signature-lookup.ts | New provider to extract signatures and fetch tx info. |
| src/actions/lookup-signature.ts | New action to look up tx signatures mentioned in messages. |
| src/providers/ca-lookup.ts | Adapt to getDecimals() returning a keyed map. |
| src/keypairUtils.ts | Normalize settings to string values via helper. |
| src/index.ts | Register new action/provider; adjust plugin naming/initialization. |
| src/utils/plugin-banner.ts | New banner utility (currently not wired into plugin). |
| src/ascii-art.ts | New ASCII art constant (currently not referenced). |
| tests/service/transactions.test.ts | New tests for transaction fetching (pagination/batching/caching/retries). |
| tests/providers/ca-lookup.test.ts | Update tests for new decimals return shape + new mapping test. |
| TRANSACTION_FETCHING.md | New documentation for transaction fetching and tasks usage. |
| TASK_CHAINING_EXAMPLES.md | New task chaining examples for post-fetch workflows. |
| .gitignore | Expand ignored build/temp/IDE/cache artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const rateLimitPerSecond = parseInt( | ||
| String(rateLimitSetting ?? '10'), | ||
| 10 | ||
| ); |
There was a problem hiding this comment.
SOLANA_RPC_RATE_LIMIT parsing can produce NaN (e.g., if set to a non-numeric string), which will propagate into the token bucket math and effectively break rate limiting. After parseInt, validate with Number.isFinite and clamp to a sensible minimum (e.g., >= 1) before creating the limiter.
| const rateLimitPerSecond = parseInt( | |
| String(rateLimitSetting ?? '10'), | |
| 10 | |
| ); | |
| let rateLimitPerSecond = parseInt( | |
| String(rateLimitSetting ?? '10'), | |
| 10 | |
| ); | |
| if (!Number.isFinite(rateLimitPerSecond) || rateLimitPerSecond < 1) { | |
| rateLimitPerSecond = 10; | |
| } |
| return await this.connection.getSignaturesForAddress(publicKey, { | ||
| limit, | ||
| before, | ||
| until: options.until, |
There was a problem hiding this comment.
fetchSignaturesForAddress computes a commitment from options but never passes it into getSignaturesForAddress, so callers can't actually control the commitment level. Pass commitment in the RPC config so FetchTransactionOptions.commitment is honored.
| until: options.until, | |
| until: options.until, | |
| commitment, |
|
|
||
| export const solanaPlugin: Plugin = { | ||
| name: SOLANA_SERVICE_NAME, | ||
| name: 'solana', |
There was a problem hiding this comment.
solanaPlugin.name was changed to 'solana' while the service type / lookup key remains SOLANA_SERVICE_NAME = 'chain_solana' (used throughout routes/providers via runtime.getService('chain_solana')). If the runtime uses Plugin.name as the identifier for enabling/configuring plugins, this creates a breaking inconsistency. Consider keeping name: SOLANA_SERVICE_NAME (or updating the constant + all lookups/docs together).
| name: 'solana', | |
| name: SOLANA_SERVICE_NAME, |
| */ | ||
| export function displayPluginBanner(runtime: IAgentRuntime, config: PluginBannerConfig): void { | ||
| const border = `+${'-'.repeat(78)}+`; | ||
| const emptyLine = `|${' '.repeat(78)}|`; |
There was a problem hiding this comment.
In plugin-banner.ts, emptyLine is declared but never used. Removing unused variables keeps the banner utility easier to maintain (and avoids future noUnusedLocals issues if enabled).
| const emptyLine = `|${' '.repeat(78)}|`; |
| import bs58 from 'bs58'; | ||
| import nacl from 'tweetnacl'; | ||
| import nacl from "tweetnacl"; | ||
|
|
There was a problem hiding this comment.
This file mixes quote styles (import nacl from "tweetnacl";) while the rest of the file predominantly uses single quotes. Keeping a consistent quoting style avoids noisy diffs and aligns with typical formatter expectations.
| */ | ||
| function pad(str: string, length: number, align: 'left' | 'center' | 'right' = 'left'): string { | ||
| const cleanStr = stripAnsi(str); | ||
| const ansiExtra = str.length - cleanStr.length; |
There was a problem hiding this comment.
Unused variable ansiExtra.
| const ansiExtra = str.length - cleanStr.length; |
| // Settings table | ||
| const tableLines = generateSettingsTable(runtime, config.settings); | ||
| for (const line of tableLines) { | ||
| const cleanLine = stripAnsi(line); |
There was a problem hiding this comment.
Unused variable cleanLine.
| const cleanLine = stripAnsi(line); |
| for (let attempt = 0; attempt < maxAttempts; attempt++) { | ||
| try { | ||
| const result = await fn(); | ||
| consecutiveFailures = 0; |
There was a problem hiding this comment.
The value assigned to consecutiveFailures here is unused.
| consecutiveFailures = 0; |
| ); | ||
|
|
||
| if (signatures.length === 0) { | ||
| hasMore = false; |
There was a problem hiding this comment.
The value assigned to hasMore here is unused.
| if (options.maxTransactions) { | ||
| const remaining = options.maxTransactions - allSignatures.length; | ||
| if (remaining <= 0) { | ||
| hasMore = false; |
There was a problem hiding this comment.
The value assigned to hasMore here is unused.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI agents
In `@__tests__/service/transactions.test.ts`:
- Around line 266-300: The test is inconsistent about deduplication: update the
mocks and assertions so they match the intended behavior of
SolanaService.fetchTransactionsForAddresses — either assert deduped totals
(expect result.totalTransactions to be 9) or rename the test/comment to indicate
totalTransactions is the non-deduped aggregate and then assert accordingly; also
make the signature mock objects returned by
mockConnection.getSignaturesForAddress include full ConfirmedSignatureInfo
fields (err, memo, blockTime, confirmationStatus) and ensure
mockConnection.getParsedTransactions returns realistic parsed transactions used
by fetchTransactionsForAddresses so the service code won't access undefined
fields.
In `@build.ts`:
- Around line 37-43: The catch block around the TypeScript declaration
generation (the try invoking $`tsc --project tsconfig.build.json` and the catch
that currently only console.warns) is swallowing errors; update it to log the
actual error output (include the caught error variable) and, when running in CI
(check process.env.CI truthiness), fail the build by calling process.exit(1) so
broken/missing .d.ts files don’t get shipped, while preserving the current
non-failing behavior for local development by only warning when CI is not set;
keep the existing timing message using dtsStart but augment logs with the error
details and an explicit non-zero exit in CI.
In `@src/actions/lookup-signature.ts`:
- Around line 200-215: The catch block defines a local const named `message`
which shadows the existing handler parameter `message: Memory`; rename the local
variable to `errorMessage` (or `errMsg`) and update its usages inside the catch
(the ternary logic, the callback text template `Failed to look up transaction:
...`, and the callback `content: { error: ... }`) so the shadowing is removed
and the callback still receives the same string value.
- Around line 68-91: The current lamport arithmetic (postBalance - preBalance)
can overflow Number and lose precision; update the balance delta logic in
lookup-signature.ts (the block using tx.meta.preBalances, tx.meta.postBalances,
balanceChanges array and the change variable) to use BigInt for subtraction
(e.g., cast pre/post balances to BigInt, compute delta as BigInt) and store
change as a BigInt; then format SOL output by converting the BigInt lamport
delta into a decimal SOL string (integer division and remainder with 1e9 to
produce up to 9 fractional digits and preserve sign) instead of using a JS
Number for solChange so no precision is lost when building the info string.
In `@src/index.ts`:
- Around line 35-36: The current logic passes null into parseBooleanFromText for
non-string settings causing truthy booleans to be treated as unset; update the
handling around runtime.getSetting so parseBooleanFromText receives a proper
value: either pass the string when typeof noActionsRaw === 'string' or convert
booleans to "true"/"false" (or directly map boolean to true/false) before
calling parseBooleanFromText; modify the code referencing noActionsRaw and
noActions (and the call to parseBooleanFromText) so boolean settings are honored
rather than converted to null.
In `@src/providers/signature-lookup.ts`:
- Around line 104-108: The current handling in signature-lookup.ts for
ix.parsed.type === 'transferChecked' concatenates both tokenAmount.uiAmount and
the raw tokenAmount.amount which produces confusing output; update the block
that builds the info string for transferChecked (where transferInfo is read) to
use only the human-readable value (e.g., tokenAmount.uiAmount or uiAmountString
if available) and drop the raw integer amount, or if you need precision include
decimals via tokenAmount.decimals to format a single readable amount; ensure the
updated message remains "Token Transfer: <readableAmount> from <source> to
<destination>" and keep the same variable names (ix, transferInfo, tokenAmount)
so the change is localized.
In `@src/rate-limiter.ts`:
- Around line 86-111: The queued requests lose their requested token amount
because acquire() creates a queueEntry without the count and processQueue()
always decrements by 1; modify acquire() to include the requested count in the
queue entry (e.g., queueEntry.count = count) and update processQueue() to read
entry.count when checking availability and when deducting tokens (subtract
entry.count from available tokens) and to resolve the promise appropriately;
ensure the timeout removal/reject logic still references the same queueEntry so
queued entries with their count are cleaned up correctly.
In `@src/service.ts`:
- Around line 3865-3877: The cache is storing the full options object
(cacheData.options) which may include a non-serializable onProgress callback
from FetchTransactionOptions; update the code to strip callbacks before caching
by creating a serializable options payload (e.g., safeOptions) that omits
onProgress and any function-valued fields, use that safeOptions in cacheData
instead of the original options, and then call
this.runtime.setCache(this.getTxCacheKey(address), cacheData) with the sanitized
cacheData so serialization will not fail.
- Around line 412-418: The parsed SOLANA_RPC_RATE_LIMIT may be NaN if
runtime.getSetting returns a non-numeric string; update the initialization
around parseInt and createRateLimiterPerSecond to validate and sanitize the
value: call runtime.getSetting('SOLANA_RPC_RATE_LIMIT'), attempt to parse it
(using parseInt or Number), check isFinite/Number.isInteger and that it’s > 0,
and if validation fails fall back to a safe default (e.g., 10) before passing to
createRateLimiterPerSecond so this.rpcRateLimiter always receives a valid
positive integer (optionally clamp a maximum/minimum). Keep the checks adjacent
to the existing parse logic so runtime.getSetting, parseInt (or conversion), and
createRateLimiterPerSecond are the referenced symbols to update.
- Around line 2595-2620: Replace the two hardcoded wrapped SOL mint strings with
the canonical constant: use PROVIDER_CONFIG.TOKEN_ADDRESSES.SOL wherever the
code builds wallet balance strings (notably in walletAddressToLLMString and the
earlier balance-string assembly that prepends "Wallet Address" and token lines);
update both occurrences so the mint address and CSV row use the constant instead
of the incorrect literal, ensuring the displayed token label and address come
from PROVIDER_CONFIG.TOKEN_ADDRESSES.SOL (the code paths around
parseTokenAccounts, getBalancesByAddrs, and getTokenAccountsByKeypair will then
use the correct SOL mint).
- Around line 3636-3646: The RPC call to this.connection.getSignaturesForAddress
inside the retryWithBackoff loop is missing the commitment argument, so it uses
the connection default instead of the configured commitment; update the call in
the loop to pass the commitment variable as the third parameter (keeping the
existing options object with limit, before, until: options.until) so
getSignaturesForAddress(publicKey, { limit, before, until: options.until },
commitment) is invoked; adjust the call located in the retryWithBackoff wrapper
where getSignaturesForAddress is used (refer to
connection.getSignaturesForAddress, publicKey, commitment, options.until,
retryWithBackoff).
In `@src/utils/plugin-banner.ts`:
- Around line 94-99: truncate() currently counts bytes including ANSI escape
sequences and may cut an escape mid-sequence (used by processCustomArt), so
change truncate to operate on visible characters: strip ANSI codes (use the same
/\x1b\[[0-9;]*m/ regex) to measure visible length, and when truncation is needed
produce a result that preserves full ANSI sequences by iterating through the
original string emitting escapes unchanged while consuming only visible
characters until maxLength - suffix.length is reached, then append suffix;
update calls from processCustomArt to rely on this visible-character-aware
truncate.
- Around line 104-109: The maskValue function currently exposes 4-character
secrets because it returns value.substring(0, 4) with zero '*' when value.length
=== 4; update maskValue so it always appends at least one masking character for
lengths >= 4 (e.g., change the '*' repeat call to use Math.max(1,
Math.min(value.length - 4, 8)) or equivalent) so substring(0, 4) + '*'... never
yields the unmasked secret.
🧹 Nitpick comments (11)
src/actions/transfer.ts (1)
193-196: Consistent withswap.ts— consider a shared helper.Same RPC URL resolution pattern duplicated here. This works correctly, but if more actions adopt it, a small utility (e.g.,
getSolanaConnection(runtime)) would reduce duplication and centralize the default URL.src/utils/plugin-banner.ts (1)
239-243:cleanLineis computed but never used.Line 240 computes
cleanLineviastripAnsibut onlypaddedLineis referenced afterward.Suggested fix
for (const line of tableLines) { - const cleanLine = stripAnsi(line); const paddedLine = pad(line, 78); output.push(`${ANSI.gray}|${ANSI.reset}${paddedLine}${ANSI.gray}|${ANSI.reset}`); }src/types.ts (1)
249-249: Consider narrowingerrtype fromany.
TransactionRecord.erris typed asany. The Solana SDK usesTransactionError | nullfor transaction errors. A narrower type would improve downstream type safety.Suggested type narrowing
- err?: any; + err?: Record<string, any> | null;Or import and use the SDK's
TransactionErrortype if available.TASK_CHAINING_EXAMPLES.md (1)
1-374: Documentation looks comprehensive and well-structured.The examples cover a good range of patterns for task chaining workflows. One minor note: in Pattern 6 (line 269),
error.messageis accessed without a type guard in the catch block, which would fail in strict TypeScript. Since these are illustrative examples, consider adding a type annotation or guard (e.g.,error instanceof Error ? error.message : String(error)) to keep the examples copy-pasteable.src/actions/lookup-signature.ts (1)
128-216: Handler mixesruntime.logger(line 162) and importedlogger(line 201).Minor inconsistency — pick one for uniformity within the same function.
src/rate-limiter.ts (1)
59-68:tryAcquireis markedasyncbut contains noawait.This wraps the return value in an unnecessary
Promise. Consider making it synchronous or at minimum noting that theasyncis intentional for interface consistency.src/providers/wallet.ts (1)
39-49: Broad regex may match non-address base58 strings, but PublicKey validation mitigates this.The regex on line 40 (
/\b[1-9A-HJ-NP-Za-km-z]{32,44}\b/g) will match any 32–44 char base58 string in the message, potentially including fragments of transaction signatures or other data. ThePublicKeyconstructor validation on line 96 is a good safety net. Also, thetoLowerCase()call on line 72 is redundant sinceownWalletKeywordsvalues are already lowercase.src/service.ts (4)
3621-3708: Signature fetching doesn't go through the RPC rate limiter.
fetchSignaturesForAddresscallsgetSignaturesForAddressviaretryWithBackoff, but unlikebatchGetMultipleAccountsInfo, it doesn't callthis.rpcRateLimiter.acquire()before each RPC call. For wallets with many pages of signatures, this could overwhelm the RPC endpoint. Consider adding rate limiter acquisition inside theretryWithBackoffcallback.Proposed fix
let signatures = await this.retryWithBackoff( async () => { + await this.rpcRateLimiter.acquire(1, 30000); return await this.connection.getSignaturesForAddress(publicKey, { limit, before, until: options.until, }); }, `getSignaturesForAddress(${address}, page ${pageCount + 1})` );
3716-3781: Same missing rate limiter forgetParsedTransactions+ static analysis note.Similar to signature fetching,
fetchTransactionsForSignaturesdoesn't acquire rate limiter tokens beforegetParsedTransactionscalls. Also, thecommitmentparameter (line 3718) is unused — consider either using it or removing it.Regarding the static analysis hint on line 3774:
batchResults.forEach((batch) => transactions.push(...batch))— thepush()return value is implicitly returned from the arrow. Use a block body orfor...ofto satisfy the linter.Proposed fix for rate limiting and linter
const batchPromises = batches.map((batch, batchIndex) => limit(async () => { const batchResults = await this.retryWithBackoff( async () => { + await this.rpcRateLimiter.acquire(1, 30000); return await this.connection.getParsedTransactions(batch, { maxSupportedTransactionVersion: 0, }); },- batchResults.forEach((batch) => transactions.push(...batch)); + for (const batch of batchResults) { + transactions.push(...batch); + }
3835-3841: Health check failure is not a gate — execution proceeds regardless.If
checkRpcHealth()fails, the code waits 5 seconds and then continues anyway. This essentially makes the health check a "soft delay" rather than a meaningful guard. Consider retrying the health check or returning an error if the RPC is confirmed unhealthy.
2411-2431:secretKey.fill(0)provides limited security benefit.Line 2421 zeroes the
Uint8Arrayafterbs58.encodehas already created the base58 string inprivateKey. The string remains in memory until GC. Thefill(0)is a reasonable best-effort, but don't rely on it as a security guarantee — document this limitation or add a note.
| it('should handle multi-wallet fetching with deduplication', async () => { | ||
| const service = new SolanaService(mockRuntime); | ||
| (service as any).connection = mockConnection; | ||
|
|
||
| const address1 = Keypair.generate().publicKey.toBase58(); | ||
| const address2 = Keypair.generate().publicKey.toBase58(); | ||
|
|
||
| // Both wallets share some transactions | ||
| const sharedSig = 'shared-sig'.padEnd(88, '0'); | ||
| const sigs1 = [sharedSig, ...generateMockSignatures(5).map((s) => s.signature)]; | ||
| const sigs2 = [sharedSig, ...generateMockSignatures(3).map((s) => s.signature)]; | ||
|
|
||
| mockConnection.getSignaturesForAddress = mock(async (pubkey: PublicKey) => { | ||
| const addr = pubkey.toBase58(); | ||
| if (addr === address1) { | ||
| return sigs1.map((sig) => ({ signature: sig, slot: 1000 })); | ||
| } else if (addr === address2) { | ||
| return sigs2.map((sig) => ({ signature: sig, slot: 1000 })); | ||
| } | ||
| return []; | ||
| }); | ||
|
|
||
| mockConnection.getParsedTransactions = mock(async (sigs: string[]) => | ||
| sigs.map((sig) => generateMockTransaction(sig)) | ||
| ); | ||
|
|
||
| const result = await service.fetchTransactionsForAddresses([address1, address2]); | ||
|
|
||
| // Wallet 1 has 6 sigs, Wallet 2 has 4 sigs = 10 total transactions | ||
| // Unique signatures: 6 from wallet1 + 4 from wallet2 - 1 shared = 9 unique, but the Set tracks all 10 since we add per-wallet | ||
| // The deduplication Set counts unique signatures across all results | ||
| expect(result.totalTransactions).toBe(10); | ||
| expect(result.totalSignatures).toBeGreaterThanOrEqual(6); // At least 6 unique (from wallet 1) | ||
| expect(Object.keys(result.results)).toHaveLength(2); | ||
| }); |
There was a problem hiding this comment.
Multi-wallet deduplication test comment contradicts itself — verify expected behavior.
The comment on lines 294–296 acknowledges that the shared signature should yield 9 unique entries but then asserts totalTransactions is 10, stating "the Set tracks all 10 since we add per-wallet." If the service is supposed to deduplicate across wallets, the expected value should be 9. If totalTransactions is intentionally the sum of per-wallet counts (not deduped), the test name and comment should clarify that.
Also, the mock return values on lines 281/283 ({ signature: sig, slot: 1000 }) are missing ConfirmedSignatureInfo fields like err, memo, blockTime, and confirmationStatus, which could cause issues if the service accesses them.
Suggested clarification for the assertion
Either fix the expected value to reflect true deduplication:
- // Wallet 1 has 6 sigs, Wallet 2 has 4 sigs = 10 total transactions
- // Unique signatures: 6 from wallet1 + 4 from wallet2 - 1 shared = 9 unique, but the Set tracks all 10 since we add per-wallet
- // The deduplication Set counts unique signatures across all results
- expect(result.totalTransactions).toBe(10);
+ // Wallet 1 has 6 sigs, Wallet 2 has 4 sigs, 1 shared = 9 unique
+ expect(result.totalTransactions).toBe(9);Or rename/reword to clarify that totalTransactions is the aggregate (non-deduped) count and that deduplication is reflected elsewhere (e.g., in totalSignatures).
🤖 Prompt for AI Agents
In `@__tests__/service/transactions.test.ts` around lines 266 - 300, The test is
inconsistent about deduplication: update the mocks and assertions so they match
the intended behavior of SolanaService.fetchTransactionsForAddresses — either
assert deduped totals (expect result.totalTransactions to be 9) or rename the
test/comment to indicate totalTransactions is the non-deduped aggregate and then
assert accordingly; also make the signature mock objects returned by
mockConnection.getSignaturesForAddress include full ConfirmedSignatureInfo
fields (err, memo, blockTime, confirmationStatus) and ensure
mockConnection.getParsedTransactions returns realistic parsed transactions used
by fetchTransactionsForAddresses so the service code won't access undefined
fields.
| try { | ||
| await $`tsc --project tsconfig.build.json`; | ||
| console.log(`✅ Declarations generated in ${((Date.now() - dtsStart) / 1000).toFixed(2)}s`); | ||
| } catch (error) { | ||
| console.warn(`⚠️ TypeScript declaration generation had errors (${((Date.now() - dtsStart) / 1000).toFixed(2)}s)`); | ||
| console.warn(" Build will continue - fix type errors when possible"); | ||
| } |
There was a problem hiding this comment.
Swallowing declaration-generation errors risks shipping broken types.
If tsc fails, the dist/ directory may contain partial or missing .d.ts files. Any downstream consumer of this package (e.g., import type { SolanaService }) would get confusing errors at compile time.
Consider at minimum logging the actual error output, and failing the build in CI (e.g., check an env var like CI) while allowing local dev to continue:
Proposed improvement
try {
await $`tsc --project tsconfig.build.json`;
console.log(`✅ Declarations generated in ${((Date.now() - dtsStart) / 1000).toFixed(2)}s`);
} catch (error) {
- console.warn(`⚠️ TypeScript declaration generation had errors (${((Date.now() - dtsStart) / 1000).toFixed(2)}s)`);
- console.warn(" Build will continue - fix type errors when possible");
+ console.warn(`⚠️ TypeScript declaration generation had errors (${((Date.now() - dtsStart) / 1000).toFixed(2)}s)`);
+ if (process.env.CI) {
+ throw new Error("Declaration generation failed in CI — fix type errors before merging");
+ }
+ console.warn(" Build will continue locally — fix type errors before publishing");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| await $`tsc --project tsconfig.build.json`; | |
| console.log(`✅ Declarations generated in ${((Date.now() - dtsStart) / 1000).toFixed(2)}s`); | |
| } catch (error) { | |
| console.warn(`⚠️ TypeScript declaration generation had errors (${((Date.now() - dtsStart) / 1000).toFixed(2)}s)`); | |
| console.warn(" Build will continue - fix type errors when possible"); | |
| } | |
| try { | |
| await $`tsc --project tsconfig.build.json`; | |
| console.log(`✅ Declarations generated in ${((Date.now() - dtsStart) / 1000).toFixed(2)}s`); | |
| } catch (error) { | |
| console.warn(`⚠️ TypeScript declaration generation had errors (${((Date.now() - dtsStart) / 1000).toFixed(2)}s)`); | |
| if (process.env.CI) { | |
| throw new Error("Declaration generation failed in CI — fix type errors before merging"); | |
| } | |
| console.warn(" Build will continue locally — fix type errors before publishing"); | |
| } |
🤖 Prompt for AI Agents
In `@build.ts` around lines 37 - 43, The catch block around the TypeScript
declaration generation (the try invoking $`tsc --project tsconfig.build.json`
and the catch that currently only console.warns) is swallowing errors; update it
to log the actual error output (include the caught error variable) and, when
running in CI (check process.env.CI truthiness), fail the build by calling
process.exit(1) so broken/missing .d.ts files don’t get shipped, while
preserving the current non-failing behavior for local development by only
warning when CI is not set; keep the existing timing message using dtsStart but
augment logs with the error details and an explicit non-zero exit in CI.
| if (tx.meta.preBalances && tx.meta.postBalances) { | ||
| const balanceChanges: Array<{ account: string; change: number }> = []; | ||
| if (tx.transaction && tx.transaction.message && tx.transaction.message.accountKeys) { | ||
| const accountKeys = tx.transaction.message.accountKeys; | ||
| for (let i = 0; i < Math.min(accountKeys.length, tx.meta.preBalances.length); i++) { | ||
| const preBalance = tx.meta.preBalances[i]; | ||
| const postBalance = tx.meta.postBalances[i]; | ||
| const change = postBalance - preBalance; | ||
| if (change !== 0) { | ||
| const account = accountKeys[i]; | ||
| const accountStr = typeof account === 'string' ? account : account.pubkey?.toString() || 'Unknown'; | ||
| balanceChanges.push({ account: accountStr, change }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (balanceChanges.length > 0) { | ||
| info += `\nBalance Changes:\n`; | ||
| balanceChanges.forEach(({ account, change }) => { | ||
| const solChange = change / 1e9; | ||
| info += ` ${account}: ${solChange >= 0 ? '+' : ''}${solChange} SOL\n`; | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential precision loss with large lamport values.
On line 75, postBalance - preBalance operates on raw lamport values. Solana lamport values can exceed Number.MAX_SAFE_INTEGER (~9×10¹⁵) for very large balances, causing silent precision loss. Consider using BigInt or BigNumber for the arithmetic.
Proposed fix
- if (tx.meta.preBalances && tx.meta.postBalances) {
- const balanceChanges: Array<{ account: string; change: number }> = [];
+ if (tx.meta.preBalances && tx.meta.postBalances) {
+ const balanceChanges: Array<{ account: string; change: bigint }> = [];
if (tx.transaction && tx.transaction.message && tx.transaction.message.accountKeys) {
const accountKeys = tx.transaction.message.accountKeys;
for (let i = 0; i < Math.min(accountKeys.length, tx.meta.preBalances.length); i++) {
- const preBalance = tx.meta.preBalances[i];
- const postBalance = tx.meta.postBalances[i];
- const change = postBalance - preBalance;
- if (change !== 0) {
+ const preBalance = BigInt(tx.meta.preBalances[i]);
+ const postBalance = BigInt(tx.meta.postBalances[i]);
+ const change = postBalance - preBalance;
+ if (change !== 0n) {
const account = accountKeys[i];
const accountStr = typeof account === 'string' ? account : account.pubkey?.toString() || 'Unknown';
balanceChanges.push({ account: accountStr, change });And update the formatting:
balanceChanges.forEach(({ account, change }) => {
- const solChange = change / 1e9;
- info += ` ${account}: ${solChange >= 0 ? '+' : ''}${solChange} SOL\n`;
+ const solChange = Number(change) / 1e9;
+ info += ` ${account}: ${solChange >= 0 ? '+' : ''}${solChange} SOL\n`;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (tx.meta.preBalances && tx.meta.postBalances) { | |
| const balanceChanges: Array<{ account: string; change: number }> = []; | |
| if (tx.transaction && tx.transaction.message && tx.transaction.message.accountKeys) { | |
| const accountKeys = tx.transaction.message.accountKeys; | |
| for (let i = 0; i < Math.min(accountKeys.length, tx.meta.preBalances.length); i++) { | |
| const preBalance = tx.meta.preBalances[i]; | |
| const postBalance = tx.meta.postBalances[i]; | |
| const change = postBalance - preBalance; | |
| if (change !== 0) { | |
| const account = accountKeys[i]; | |
| const accountStr = typeof account === 'string' ? account : account.pubkey?.toString() || 'Unknown'; | |
| balanceChanges.push({ account: accountStr, change }); | |
| } | |
| } | |
| } | |
| if (balanceChanges.length > 0) { | |
| info += `\nBalance Changes:\n`; | |
| balanceChanges.forEach(({ account, change }) => { | |
| const solChange = change / 1e9; | |
| info += ` ${account}: ${solChange >= 0 ? '+' : ''}${solChange} SOL\n`; | |
| }); | |
| } | |
| } | |
| if (tx.meta.preBalances && tx.meta.postBalances) { | |
| const balanceChanges: Array<{ account: string; change: bigint }> = []; | |
| if (tx.transaction && tx.transaction.message && tx.transaction.message.accountKeys) { | |
| const accountKeys = tx.transaction.message.accountKeys; | |
| for (let i = 0; i < Math.min(accountKeys.length, tx.meta.preBalances.length); i++) { | |
| const preBalance = BigInt(tx.meta.preBalances[i]); | |
| const postBalance = BigInt(tx.meta.postBalances[i]); | |
| const change = postBalance - preBalance; | |
| if (change !== 0n) { | |
| const account = accountKeys[i]; | |
| const accountStr = typeof account === 'string' ? account : account.pubkey?.toString() || 'Unknown'; | |
| balanceChanges.push({ account: accountStr, change }); | |
| } | |
| } | |
| } | |
| if (balanceChanges.length > 0) { | |
| info += `\nBalance Changes:\n`; | |
| balanceChanges.forEach(({ account, change }) => { | |
| const solChange = Number(change) / 1e9; | |
| info += ` ${account}: ${solChange >= 0 ? '+' : ''}${solChange} SOL\n`; | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/actions/lookup-signature.ts` around lines 68 - 91, The current lamport
arithmetic (postBalance - preBalance) can overflow Number and lose precision;
update the balance delta logic in lookup-signature.ts (the block using
tx.meta.preBalances, tx.meta.postBalances, balanceChanges array and the change
variable) to use BigInt for subtraction (e.g., cast pre/post balances to BigInt,
compute delta as BigInt) and store change as a BigInt; then format SOL output by
converting the BigInt lamport delta into a decimal SOL string (integer division
and remainder with 1e9 to produce up to 9 fractional digits and preserve sign)
instead of using a JS Number for solChange so no precision is lost when building
the info string.
| } catch (error) { | ||
| logger.error({ error }, 'Error looking up Solana signature'); | ||
| if (callback) { | ||
| const message = | ||
| error instanceof Error | ||
| ? error.message | ||
| : typeof error === 'string' | ||
| ? error | ||
| : JSON.stringify(error); | ||
| callback({ | ||
| text: `Failed to look up transaction: ${message}`, | ||
| content: { error: message }, | ||
| }); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
Variable message shadows the handler parameter.
On line 203, const message shadows the message: Memory parameter from line 130. While it doesn't cause a bug here, it's confusing and error-prone. Rename to errorMessage or errMsg.
Proposed fix
- const message =
+ const errorMessage =
error instanceof Error
? error.message
: typeof error === 'string'
? error
: JSON.stringify(error);
callback({
- text: `Failed to look up transaction: ${message}`,
- content: { error: message },
+ text: `Failed to look up transaction: ${errorMessage}`,
+ content: { error: errorMessage },
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| logger.error({ error }, 'Error looking up Solana signature'); | |
| if (callback) { | |
| const message = | |
| error instanceof Error | |
| ? error.message | |
| : typeof error === 'string' | |
| ? error | |
| : JSON.stringify(error); | |
| callback({ | |
| text: `Failed to look up transaction: ${message}`, | |
| content: { error: message }, | |
| }); | |
| } | |
| return; | |
| } | |
| } catch (error) { | |
| logger.error({ error }, 'Error looking up Solana signature'); | |
| if (callback) { | |
| const errorMessage = | |
| error instanceof Error | |
| ? error.message | |
| : typeof error === 'string' | |
| ? error | |
| : JSON.stringify(error); | |
| callback({ | |
| text: `Failed to look up transaction: ${errorMessage}`, | |
| content: { error: errorMessage }, | |
| }); | |
| } | |
| return; | |
| } |
🤖 Prompt for AI Agents
In `@src/actions/lookup-signature.ts` around lines 200 - 215, The catch block
defines a local const named `message` which shadows the existing handler
parameter `message: Memory`; rename the local variable to `errorMessage` (or
`errMsg`) and update its usages inside the catch (the ternary logic, the
callback text template `Failed to look up transaction: ...`, and the callback
`content: { error: ... }`) so the shadowing is removed and the callback still
receives the same string value.
| const noActionsRaw = runtime.getSetting("SOLANA_NO_ACTIONS"); | ||
| const noActions = parseBooleanFromText(typeof noActionsRaw === 'string' ? noActionsRaw : null); |
There was a problem hiding this comment.
parseBooleanFromText receives null for non-string settings — verify intent.
If SOLANA_NO_ACTIONS is set to a non-string truthy value (e.g., true as a boolean), it will be converted to null before parsing, causing parseBooleanFromText to return null. Since !null === true, actions will be registered despite the setting being truthy. This is likely fine given getSetting typically returns strings, but worth confirming the behavior is intentional.
🤖 Prompt for AI Agents
In `@src/index.ts` around lines 35 - 36, The current logic passes null into
parseBooleanFromText for non-string settings causing truthy booleans to be
treated as unset; update the handling around runtime.getSetting so
parseBooleanFromText receives a proper value: either pass the string when typeof
noActionsRaw === 'string' or convert booleans to "true"/"false" (or directly map
boolean to true/false) before calling parseBooleanFromText; modify the code
referencing noActionsRaw and noActions (and the call to parseBooleanFromText) so
boolean settings are honored rather than converted to null.
| balanceStr += 'Wallet Address: ' + pubKey + '\n' | ||
| balanceStr += ' Token Address (Symbol)\n' | ||
| balanceStr += ' So11111111111111111111111111111111111111111 ($sol) balance: ' + (solBal ?? 'unknown') + '\n' | ||
| const tokens = await this.parseTokenAccounts(heldTokens) // options | ||
| for (const ca in tokens) { | ||
| const t = tokens[ca] | ||
| balanceStr += ' ' + ca + ' ($' + t.symbol + ') balance: ' + t.balanceUi + '\n' | ||
| } | ||
| balanceStr += '\n' | ||
| return balanceStr | ||
| } | ||
|
|
||
| async walletAddressToLLMString(pubKey: string): Promise<string> { | ||
| let balanceStr = '' | ||
| // get wallet contents | ||
| const pubKeyObj = new PublicKey(pubKey) | ||
| const [balances, heldTokens] = await Promise.all([ | ||
| this.getBalancesByAddrs([pubKey]), | ||
| this.getTokenAccountsByKeypair(pubKeyObj), | ||
| ]); | ||
| //console.log('balances', balances) | ||
| const solBal = balances[pubKey] | ||
| balanceStr += 'Wallet Address: ' + pubKey + '\n' | ||
| balanceStr += 'Current wallet contents in csv format:\n' | ||
| balanceStr += 'Token Address,Symbol,Balance\n' | ||
| balanceStr += 'So11111111111111111111111111111111111111111,sol,' + (solBal ?? 'unknown') + '\n' |
There was a problem hiding this comment.
Incorrect wrapped SOL mint address — will display wrong token label.
Lines 2597 and 2620 use So11111111111111111111111111111111111111111 (43 chars, ending in 1), but the canonical wrapped SOL mint address is So11111111111111111111111111111111111111112 (44 chars, ending in 2) — as correctly defined in PROVIDER_CONFIG.TOKEN_ADDRESSES.SOL on line 58. Use the constant instead of a hardcoded (incorrect) string.
Proposed fix
- balanceStr += ' So11111111111111111111111111111111111111111 ($sol) balance: ' + (solBal ?? 'unknown') + '\n'
+ balanceStr += ' ' + PROVIDER_CONFIG.TOKEN_ADDRESSES.SOL + ' ($sol) balance: ' + (solBal ?? 'unknown') + '\n'- balanceStr += 'So11111111111111111111111111111111111111111,sol,' + (solBal ?? 'unknown') + '\n'
+ balanceStr += PROVIDER_CONFIG.TOKEN_ADDRESSES.SOL + ',sol,' + (solBal ?? 'unknown') + '\n'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| balanceStr += 'Wallet Address: ' + pubKey + '\n' | |
| balanceStr += ' Token Address (Symbol)\n' | |
| balanceStr += ' So11111111111111111111111111111111111111111 ($sol) balance: ' + (solBal ?? 'unknown') + '\n' | |
| const tokens = await this.parseTokenAccounts(heldTokens) // options | |
| for (const ca in tokens) { | |
| const t = tokens[ca] | |
| balanceStr += ' ' + ca + ' ($' + t.symbol + ') balance: ' + t.balanceUi + '\n' | |
| } | |
| balanceStr += '\n' | |
| return balanceStr | |
| } | |
| async walletAddressToLLMString(pubKey: string): Promise<string> { | |
| let balanceStr = '' | |
| // get wallet contents | |
| const pubKeyObj = new PublicKey(pubKey) | |
| const [balances, heldTokens] = await Promise.all([ | |
| this.getBalancesByAddrs([pubKey]), | |
| this.getTokenAccountsByKeypair(pubKeyObj), | |
| ]); | |
| //console.log('balances', balances) | |
| const solBal = balances[pubKey] | |
| balanceStr += 'Wallet Address: ' + pubKey + '\n' | |
| balanceStr += 'Current wallet contents in csv format:\n' | |
| balanceStr += 'Token Address,Symbol,Balance\n' | |
| balanceStr += 'So11111111111111111111111111111111111111111,sol,' + (solBal ?? 'unknown') + '\n' | |
| balanceStr += 'Wallet Address: ' + pubKey + '\n' | |
| balanceStr += ' Token Address (Symbol)\n' | |
| balanceStr += ' ' + PROVIDER_CONFIG.TOKEN_ADDRESSES.SOL + ' ($sol) balance: ' + (solBal ?? 'unknown') + '\n' | |
| const tokens = await this.parseTokenAccounts(heldTokens) // options | |
| for (const ca in tokens) { | |
| const t = tokens[ca] | |
| balanceStr += ' ' + ca + ' ($' + t.symbol + ') balance: ' + t.balanceUi + '\n' | |
| } | |
| balanceStr += '\n' | |
| return balanceStr | |
| } | |
| async walletAddressToLLMString(pubKey: string): Promise<string> { | |
| let balanceStr = '' | |
| // get wallet contents | |
| const pubKeyObj = new PublicKey(pubKey) | |
| const [balances, heldTokens] = await Promise.all([ | |
| this.getBalancesByAddrs([pubKey]), | |
| this.getTokenAccountsByKeypair(pubKeyObj), | |
| ]); | |
| //console.log('balances', balances) | |
| const solBal = balances[pubKey] | |
| balanceStr += 'Wallet Address: ' + pubKey + '\n' | |
| balanceStr += 'Current wallet contents in csv format:\n' | |
| balanceStr += 'Token Address,Symbol,Balance\n' | |
| balanceStr += PROVIDER_CONFIG.TOKEN_ADDRESSES.SOL + ',sol,' + (solBal ?? 'unknown') + '\n' |
🤖 Prompt for AI Agents
In `@src/service.ts` around lines 2595 - 2620, Replace the two hardcoded wrapped
SOL mint strings with the canonical constant: use
PROVIDER_CONFIG.TOKEN_ADDRESSES.SOL wherever the code builds wallet balance
strings (notably in walletAddressToLLMString and the earlier balance-string
assembly that prepends "Wallet Address" and token lines); update both
occurrences so the mint address and CSV row use the constant instead of the
incorrect literal, ensuring the displayed token label and address come from
PROVIDER_CONFIG.TOKEN_ADDRESSES.SOL (the code paths around parseTokenAccounts,
getBalancesByAddrs, and getTokenAccountsByKeypair will then use the correct SOL
mint).
| while (hasMore) { | ||
| let signatures = await this.retryWithBackoff( | ||
| async () => { | ||
| return await this.connection.getSignaturesForAddress(publicKey, { | ||
| limit, | ||
| before, | ||
| until: options.until, | ||
| }); | ||
| }, | ||
| `getSignaturesForAddress(${address}, page ${pageCount + 1})` | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the code context around line 3628-3650
cat -n src/service.ts | sed -n '3620,3660p'Repository: elizaos-plugins/plugin-solana
Length of output: 1659
🏁 Script executed:
# Also search for how getSignaturesForAddress is used elsewhere in the codebase
rg "getSignaturesForAddress" -B 5 -A 5 --type tsRepository: elizaos-plugins/plugin-solana
Length of output: 11823
🌐 Web query:
@solana/web3.js getSignaturesForAddress method signature commitment parameter
💡 Result:
In @solana/web3.js, the method is:
connection.getSignaturesForAddress(
address: PublicKey,
options?: SignaturesForAddressOptions,
commitment?: Finality,
): Promise<ConfirmedSignatureInfo[]>[1]
optionsdoes not includecommitment; it only supportsbefore,until,limit, andminContextSlot. [2]commitmentis the 3rd argument (Finality). [1]
Example:
const sigs = await connection.getSignaturesForAddress(
address,
{ limit: 100, before },
'finalized',
);Note: the underlying JSON-RPC method takes commitment inside its config object, but that’s at the RPC layer (not the web3.js options type). [3]
Sources:
[1] Connection.getSignaturesForAddress typedoc signature (solana-foundation.github.io)
[2] SignaturesForAddressOptions fields (solana-foundation.github.io)
[3] Solana JSON-RPC getSignaturesForAddress params (config includes commitment) (solana.com)
Citations:
- 1: https://solana-foundation.github.io/solana-web3.js/v1.x/classes/Connection.html
- 2: https://solana-foundation.github.io/solana-web3.js/v1.x/types/SignaturesForAddressOptions.html
- 3: https://solana.com/docs/rpc/http/getsignaturesforaddress?utm_source=openai
Add commitment as the third argument to getSignaturesForAddress call.
The commitment variable is set on line 3628 but never passed to the RPC call on line 3639. According to the @solana/web3.js library, getSignaturesForAddress accepts commitment as the third argument (separate from the options object), not as part of the options. Signatures will currently be fetched at the connection's default commitment level instead of the configured one.
Fix location (lines 3639-3643)
Change:
return await this.connection.getSignaturesForAddress(publicKey, {
limit,
before,
until: options.until,
});To:
return await this.connection.getSignaturesForAddress(publicKey, {
limit,
before,
until: options.until,
}, commitment);🤖 Prompt for AI Agents
In `@src/service.ts` around lines 3636 - 3646, The RPC call to
this.connection.getSignaturesForAddress inside the retryWithBackoff loop is
missing the commitment argument, so it uses the connection default instead of
the configured commitment; update the call in the loop to pass the commitment
variable as the third parameter (keeping the existing options object with limit,
before, until: options.until) so getSignaturesForAddress(publicKey, { limit,
before, until: options.until }, commitment) is invoked; adjust the call located
in the retryWithBackoff wrapper where getSignaturesForAddress is used (refer to
connection.getSignaturesForAddress, publicKey, commitment, options.until,
retryWithBackoff).
| // Cache result | ||
| const cacheTtl = options.cacheTtl || TX_FETCH_CONFIG.DEFAULT_CACHE_TTL; | ||
| const cacheData: CachedTransactionData = { | ||
| address, | ||
| signatures, | ||
| transactions, | ||
| lastSignature: signatures[signatures.length - 1], | ||
| fetchedAt: Date.now(), | ||
| ttl: cacheTtl, | ||
| options, | ||
| }; | ||
|
|
||
| await this.runtime.setCache(this.getTxCacheKey(address), cacheData); |
There was a problem hiding this comment.
Caching options object that may contain non-serializable onProgress callback.
Line 3874 stores the original options in cacheData, but options can contain an onProgress function (per the FetchTransactionOptions type). Depending on the cache implementation, this will either throw a serialization error, silently drop the function, or store null. Consider stripping callbacks before caching.
Proposed fix
+ // Strip non-serializable fields before caching
+ const { onProgress, ...serializableOptions } = options;
const cacheData: CachedTransactionData = {
address,
signatures,
transactions,
lastSignature: signatures[signatures.length - 1],
fetchedAt: Date.now(),
ttl: cacheTtl,
- options,
+ options: serializableOptions,
};🤖 Prompt for AI Agents
In `@src/service.ts` around lines 3865 - 3877, The cache is storing the full
options object (cacheData.options) which may include a non-serializable
onProgress callback from FetchTransactionOptions; update the code to strip
callbacks before caching by creating a serializable options payload (e.g.,
safeOptions) that omits onProgress and any function-valued fields, use that
safeOptions in cacheData instead of the original options, and then call
this.runtime.setCache(this.getTxCacheKey(address), cacheData) with the sanitized
cacheData so serialization will not fail.
| function truncate(str: string, maxLength: number, suffix: string = '...'): string { | ||
| if (str.length <= maxLength) { | ||
| return str; | ||
| } | ||
| return str.substring(0, maxLength - suffix.length) + suffix; | ||
| } |
There was a problem hiding this comment.
truncate() can break ANSI escape sequences mid-stream.
When called on strings containing ANSI codes (e.g., in processCustomArt at line 145), substring may cut in the middle of an escape sequence like \x1b[35m, producing garbled terminal output. Consider stripping ANSI before measuring length, or truncating based on visible character count.
🤖 Prompt for AI Agents
In `@src/utils/plugin-banner.ts` around lines 94 - 99, truncate() currently counts
bytes including ANSI escape sequences and may cut an escape mid-sequence (used
by processCustomArt), so change truncate to operate on visible characters: strip
ANSI codes (use the same /\x1b\[[0-9;]*m/ regex) to measure visible length, and
when truncation is needed produce a result that preserves full ANSI sequences by
iterating through the original string emitting escapes unchanged while consuming
only visible characters until maxLength - suffix.length is reached, then append
suffix; update calls from processCustomArt to rely on this
visible-character-aware truncate.
| function maskValue(value: string): string { | ||
| if (!value || value.length < 4) { | ||
| return '***'; | ||
| } | ||
| return value.substring(0, 4) + '*'.repeat(Math.min(value.length - 4, 8)); | ||
| } |
There was a problem hiding this comment.
4-character secrets are fully exposed by maskValue.
When value.length === 4, the function returns value.substring(0, 4) + zero * repeats, i.e., the entire value unmasked.
Suggested fix
function maskValue(value: string): string {
- if (!value || value.length < 4) {
+ if (!value || value.length <= 8) {
return '***';
}
return value.substring(0, 4) + '*'.repeat(Math.min(value.length - 4, 8));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function maskValue(value: string): string { | |
| if (!value || value.length < 4) { | |
| return '***'; | |
| } | |
| return value.substring(0, 4) + '*'.repeat(Math.min(value.length - 4, 8)); | |
| } | |
| function maskValue(value: string): string { | |
| if (!value || value.length <= 8) { | |
| return '***'; | |
| } | |
| return value.substring(0, 4) + '*'.repeat(Math.min(value.length - 4, 8)); | |
| } |
🤖 Prompt for AI Agents
In `@src/utils/plugin-banner.ts` around lines 104 - 109, The maskValue function
currently exposes 4-character secrets because it returns value.substring(0, 4)
with zero '*' when value.length === 4; update maskValue so it always appends at
least one masking character for lengths >= 4 (e.g., change the '*' repeat call
to use Math.max(1, Math.min(value.length - 4, 8)) or equivalent) so substring(0,
4) + '*'... never yields the unmasked secret.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| resolve: () => void; | ||
| reject: (error: Error) => void; | ||
| timestamp: number; | ||
| }> = []; |
There was a problem hiding this comment.
Rate limiter queue ignores requested token count
Medium Severity
The acquire(count) method accepts a count parameter, but when the request is queued, the count is not stored in the queue entry. When processQueue later resolves queued entries, it always deducts exactly 1 token (this.tokens -= 1) regardless of the originally requested count. This means acquire(5) would only consume 1 token when dequeued, violating the rate limiting contract.
Additional Locations (1)
| async () => { | ||
| return await this.connection.getParsedTransactions(batch, { | ||
| maxSupportedTransactionVersion: 0, | ||
| }); |
There was a problem hiding this comment.
Transaction fetch ignores commitment level parameter
Medium Severity
fetchTransactionsForSignatures accepts a commitment parameter but never passes it to getParsedTransactions. The caller in getTransactionsForAddress explicitly passes options.commitment || TX_FETCH_CONFIG.TRANSACTION_COMMITMENT (defaulting to 'finalized'), but it's silently ignored. This means the Connection's default commitment is always used, which may differ from what was requested.
| } | ||
|
|
||
| return Array.from(results); | ||
| } |
There was a problem hiding this comment.
Duplicated signature extraction function across two files
Low Severity
extractSignaturesFromText is identically implemented in both src/actions/lookup-signature.ts and src/providers/signature-lookup.ts. Similarly, formatTransactionInfo appears in both files with slight variations. Extracting these into a shared utility would reduce maintenance burden and avoid the risk of divergent bug fixes.
Additional Locations (1)
|
|
||
| // Output banner | ||
| runtime.logger.info('\n' + output.join('\n')); | ||
| } |
There was a problem hiding this comment.
Unused exported functions and modules never imported
Low Severity
Several new exported symbols are never imported anywhere in the codebase: displayPluginBanner and createSetting from plugin-banner.ts, SOLANA_ART from ascii-art.ts, and createRateLimiterPerMinute from rate-limiter.ts. These are dead code that adds to bundle size and maintenance burden.
Additional Locations (2)
| "types": [ | ||
| "bun" | ||
| "bun-types", | ||
| "node" |
There was a problem hiding this comment.
tsconfig changes contradict existing type safety tests
Medium Severity
The types array changed from ["bun"] to ["bun-types", "node"], which directly contradicts assertions in __tests__/tsconfig.test.ts. That test expects types to contain 'bun' (not 'bun-types') and explicitly asserts types must NOT contain 'node' because Node types conflict with Bun types. These tests will now fail, and the build script change that swallows TypeScript errors masks this breakage.
| before, | ||
| until: options.until, | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Signature fetching also ignores computed commitment parameter
Medium Severity
In fetchSignaturesForAddress, the commitment variable is computed on line 3628 from options (defaulting to TX_FETCH_CONFIG.SIGNATURE_COMMITMENT = 'confirmed') but is never passed to getSignaturesForAddress. That RPC call defaults to 'finalized', meaning recently confirmed-but-not-finalized transactions will be silently excluded when the caller expected 'confirmed' level results.
| } catch (error) { | ||
| console.warn(`⚠️ TypeScript declaration generation had errors (${((Date.now() - dtsStart) / 1000).toFixed(2)}s)`); | ||
| console.warn(" Build will continue - fix type errors when possible"); | ||
| } |
There was a problem hiding this comment.
Build script swallows TypeScript errors silently
Medium Severity
The build script now wraps tsc in a try/catch and continues on failure, and the pre-build typecheck step (typecheck()) has been removed entirely. TypeScript type errors no longer fail the build, meaning broken types and logic errors that the type system would catch can silently ship to production.
…lback - Resolve stash merge conflicts in package.json, src/index.ts, src/actions/swap.ts, src/service.ts - Move @elizaos/core to workspace:* dependency (monorepo resolution) - Remove @elizaos/service-interfaces; import IWalletService/WalletPortfolio from @elizaos/core - Add getTransactionWithCostUnitsFallback() for Solana RPC edge case where meta.costUnits is missing - Use costUnits fallback in executeSwap transaction detail fetch Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/service.ts`:
- Around line 930-962: The catch block in batchGetMultipleAccountsInfo currently
only applies exponential backoff when isRateLimit is true, causing immediate
retries for other transient errors; modify the catch handling so that when
isRateLimit is false but retryCount < maxRetries - 1 you still wait using the
same/backoff logic (use baseDelay, retryCount and Math.pow(2, retryCount - 1)
bounded by 32000) before continuing, and keep the existing logging pattern (use
this.runtime.logger.warn) to indicate non-rate-limit retry and delay; ensure
retryCount is incremented consistently and the final throw logic (when
retryCount >= maxRetries - 1) remains unchanged.
- Around line 182-205: The function getTransactionWithCostUnitsFallback
currently calls (connection as any).rpcRequest which relies on an undocumented
internal API; replace that call with the documented private method _rpcRequest
(or perform a direct JSON-RPC HTTP call) to avoid fragile any-casting: locate
the rpcRequest usage inside getTransactionWithCostUnitsFallback, change to using
connection._rpcRequest (adding a single-line `@ts-ignore` above the call to
suppress TS error) or implement a direct JSON-RPC POST using the same params
(signature and { ...GET_TRANSACTION_OPTIONS, encoding: 'json' }), and preserve
the existing handling of result.meta.costUnits and error branching for
COST_UNITS_ERROR_FRAGMENT while keeping GET_TRANSACTION_OPTIONS intact.
🧹 Nitpick comments (6)
package.json (1)
76-76: Inconsistent use of smart apostrophe (\u2019) in description strings.Lines 76 and 94 use
\u2019(curly/smart apostrophe) while the rest of the file uses plain ASCII. This is likely a copy-paste artifact. Consider using a standard ASCII apostrophe (') for consistency.Proposed fix
- "description": "Salt used to derive or encrypt the Solana wallet\u2019s secret key; required if the direct secret key is not provided.", + "description": "Salt used to derive or encrypt the Solana wallet's secret key; required if the direct secret key is not provided.",- "description": "Alternative name accepted by runtime for the wallet\u2019s public key.", + "description": "Alternative name accepted by runtime for the wallet's public key.",Also applies to: 94-94
src/service.ts (5)
3808-3812: Static analysis:forEachcallback implicitly returns a value.
transactions.push(...batch)returns the new array length, which becomes the implicit return of theforEachcallback. Use a block body or a simplefor...ofloop.Proposed fix
- batchResults.forEach((batch) => transactions.push(...batch)); + for (const batch of batchResults) { + transactions.push(...batch); + }
4043-4046: Static analysis:forEachcallback implicitly returns a value fromSet.add().Same pattern as above —
globalSignatureSet.add(sig)returns theSet, creating an implicit return in theforEachcallback.Proposed fix
- result.signatures.forEach((sig) => globalSignatureSet.add(sig)); + for (const sig of result.signatures) { + globalSignatureSet.add(sig); + }
476-561: Task worker stores potentially large transaction results in cache and task metadata.Line 511-516 stores the full
FetchTransactionsResult(which can contain thousands of parsed transactions) into cache, and line 530 embeds it into the chained task's metadata. This could cause memory pressure or exceed cache/storage size limits.Consider storing only a summary or reference (e.g., the cache keys for the per-address results) rather than the full transaction data in task metadata.
3984-4013:waitForTransactionFetchcannot distinguish "task not found" from "task pending".When
statusisnull(line 3994), the method assumes the task hasn't started yet and keeps polling. If the caller passes an invalidtaskId, this will silently poll until the 30-minute timeout. Consider adding an early exit or returning a distinct "not found" status after a reasonable initial wait.
2448-2468:secretKey.fill(0)provides limited memory-clearing benefit in JavaScript.Line 2458 zeroes the
Uint8Arraybacking the keypair, which is good practice. However,bs58.encode(newKeypair.secretKey)on line 2455 has already created a string copy that cannot be cleared from JS memory (strings are immutable and GC'd). Thefill(0)gives a false sense of security — just noting for awareness.
| export async function getTransactionWithCostUnitsFallback( | ||
| connection: Connection, | ||
| signature: string | ||
| ): Promise<VersionedTransactionResponse | null> { | ||
| try { | ||
| return await connection.getTransaction(signature, GET_TRANSACTION_OPTIONS); | ||
| } catch (error) { | ||
| if (error instanceof Error && error.message.includes(COST_UNITS_ERROR_FRAGMENT)) { | ||
| logger.warn( | ||
| 'solana::getTransaction - missing meta.costUnits. Retrying via raw RPC request with patch.' | ||
| ); | ||
| const rawResponse = await (connection as any).rpcRequest('getTransaction', [ | ||
| signature, | ||
| { ...GET_TRANSACTION_OPTIONS, encoding: 'json' }, | ||
| ]); | ||
| const result = rawResponse?.result ?? null; | ||
| if (result?.meta && result.meta.costUnits == null) { | ||
| result.meta.costUnits = 0; | ||
| } | ||
| return (result as VersionedTransactionResponse) ?? null; | ||
| } | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Verify the file and code location exists
if [ -f "src/service.ts" ]; then
echo "=== File exists, checking lines 182-205 ==="
sed -n '182,205p' src/service.ts
else
echo "File not found, searching for similar functions"
find . -name "*.ts" -type f | head -20
fiRepository: elizaos-plugins/plugin-solana
Length of output: 1008
🌐 Web query:
@solana/web3.js Connection rpcRequest method documentation private API
💡 Result:
Connection.rpcRequest is not a public/documented API in @solana/web3.js v1.x.
- The generated TypeDoc for
Connectiondoes not list anyrpcRequestmethod (only public/protected members like_buildArgs,getBalance, etc.), which indicatesrpcRequestisn’t part of the supported surface. [1] - In practice, people who need “raw RPC” access use the internal (private/TS-hidden) method
connection._rpcRequest(note the leading underscore). A common workaround is/*@ts-ignore*/ const rpcRequest = connection._rpcRequest;and then callingrpcRequest("someMethod", params). [2] - If you want a supported approach, call Solana RPC “directly” via JSON-RPC HTTP from your app (POST
{ jsonrpc: "2.0", id, method, params }), using the official RPC method docs as reference. [3]
Bottom line: there is no official “rpcRequest” documentation because it’s a private/internal API; prefer direct JSON-RPC HTTP calls (or a newer supported client stack) unless you accept the risk of breakage when web3.js changes. [1][3]
References: [1] [2] [3]
🌐 Web query:
@solana/web3.js Connection class public methods TypeScript definition
💡 Result:
The canonical TypeScript surface for @solana/web3.js’s Connection class is the generated API docs (TypeDoc), which are built directly from the library’s TS source (src/connection.ts). The public API is:
export class Connection {
constructor(endpoint: string, commitmentOrConfig?: Commitment | ConnectionConfig);
// accessors / properties
get commitment(): Commitment | undefined;
get rpcEndpoint(): string;
getBlockHeight: (commitmentOrConfig?: Commitment | GetBlockHeightConfig) => Promise<number>;
// methods (public)
_buildArgs(...): any[];
confirmTransaction(...): Promise<...>;
getAccountInfo(...): Promise<...>;
getAccountInfoAndContext(...): Promise<...>;
getAddressLookupTable(...): Promise<...>;
getBalance(...): Promise<...>;
getBalanceAndContext(...): Promise<...>;
getBlock(...): Promise<...>;
getBlockProduction(...): Promise<...>;
getBlocks(...): Promise<...>;
getBlockSignatures(...): Promise<...>;
getBlockTime(...): Promise<...>;
getClusterNodes(...): Promise<...>;
getConfirmedBlock(...): Promise<...>;
getConfirmedBlockSignatures(...): Promise<...>;
getConfirmedSignaturesForAddress(...): Promise<...>;
getConfirmedSignaturesForAddress2(...): Promise<...>;
getConfirmedTransaction(...): Promise<...>;
getEpochInfo(...): Promise<...>;
getEpochSchedule(...): Promise<...>;
getFeeCalculatorForBlockhash(...): Promise<...>;
getFeeForMessage(...): Promise<...>;
getFirstAvailableBlock(...): Promise<...>;
getGenesisHash(...): Promise<...>;
getInflationGovernor(...): Promise<...>;
getInflationRate(...): Promise<...>;
getInflationReward(...): Promise<...>;
getLargestAccounts(...): Promise<...>;
getLatestBlockhash(...): Promise<...>;
getLatestBlockhashAndContext(...): Promise<...>;
getLeaderSchedule(...): Promise<...>;
getMinimumBalanceForRentExemption(...): Promise<...>;
getMinimumLedgerSlot(...): Promise<...>;
getMultipleAccountsInfo(...): Promise<...>;
getMultipleAccountsInfoAndContext(...): Promise<...>;
getMultipleParsedAccounts(...): Promise<...>;
getNonce(...): Promise<...>;
getNonceAndContext(...): Promise<...>;
getParsedAccountInfo(...): Promise<...>;
getParsedBlock(...): Promise<...>;
getParsedConfirmedTransaction(...): Promise<...>;
getParsedConfirmedTransactions(...): Promise<...>;
getParsedProgramAccounts(...): Promise<...>;
getParsedTokenAccountsByOwner(...): Promise<...>;
getParsedTransaction(...): Promise<...>;
getParsedTransactions(...): Promise<...>;
getProgramAccounts(...): Promise<...>;
getRecentBlockhash(...): Promise<...>;
getRecentBlockhashAndContext(...): Promise<...>;
getRecentPerformanceSamples(...): Promise<...>;
getRecentPrioritizationFees(...): Promise<...>;
getSignaturesForAddress(...): Promise<...>;
getSignatureStatus(...): Promise<...>;
getSignatureStatuses(...): Promise<...>;
getSlot(...): Promise<...>;
getSlotLeader(...): Promise<...>;
getSlotLeaders(...): Promise<...>;
getStakeActivation(...): Promise<...>;
getStakeMinimumDelegation(...): Promise<...>;
getSupply(...): Promise<...>;
getTokenAccountBalance(...): Promise<...>;
getTokenAccountsByOwner(...): Promise<...>;
getTokenLargestAccounts(...): Promise<...>;
getTokenSupply(...): Promise<...>;
getTotalSupply(...): Promise<...>;
getTransaction(...): Promise<...>;
getTransactionCount(...): Promise<...>;
getTransactions(...): Promise<...>;
getVersion(...): Promise<...>;
getVoteAccounts(...): Promise<...>;
isBlockhashValid(...): Promise<...>;
onAccountChange(...): number;
onLogs(...): number;
onProgramAccountChange(...): number;
onRootChange(...): number;
onSignature(...): number;
onSignatureWithOptions(...): number;
onSlotChange(...): number;
onSlotUpdate(...): number;
removeAccountChangeListener(...): Promise<void>;
removeOnLogsListener(...): Promise<void>;
removeProgramAccountChangeListener(...): Promise<void>;
removeRootChangeListener(...): Promise<void>;
removeSignatureListener(...): Promise<void>;
removeSlotChangeListener(...): Promise<void>;
removeSlotUpdateListener(...): Promise<void>;
requestAirdrop(...): Promise<TransactionSignature>;
sendEncodedTransaction(...): Promise<TransactionSignature>;
sendRawTransaction(...): Promise<TransactionSignature>;
sendTransaction(...): Promise<TransactionSignature>;
simulateTransaction(...): Promise<...>;
}For the exact parameter/return types for each method (the full .d.ts-equivalent signatures), use the per-method entries in the official Connection class docs. [1]
Source: Solana Foundation @solana/web3.js TypeDoc for Connection (methods index + signatures). [1]
[1] https://solana-foundation.github.io/solana-web3.js/classes/Connection.html
getTransactionWithCostUnitsFallback uses an undocumented internal RPC API.
(connection as any).rpcRequest(...) (line 193) relies on a private/undocumented method of the Connection class. This method is not exposed in the official TypeScript definitions and lacks any documented contract. It may break silently on @solana/web3.js upgrades. If direct RPC access is required, use JSON-RPC HTTP calls directly or the documented _rpcRequest private method with proper @ts-ignore handling instead of casting to any.
🤖 Prompt for AI Agents
In `@src/service.ts` around lines 182 - 205, The function
getTransactionWithCostUnitsFallback currently calls (connection as
any).rpcRequest which relies on an undocumented internal API; replace that call
with the documented private method _rpcRequest (or perform a direct JSON-RPC
HTTP call) to avoid fragile any-casting: locate the rpcRequest usage inside
getTransactionWithCostUnitsFallback, change to using connection._rpcRequest
(adding a single-line `@ts-ignore` above the call to suppress TS error) or
implement a direct JSON-RPC POST using the same params (signature and {
...GET_TRANSACTION_OPTIONS, encoding: 'json' }), and preserve the existing
handling of result.meta.costUnits and error branching for
COST_UNITS_ERROR_FRAGMENT while keeping GET_TRANSACTION_OPTIONS intact.
| } catch (error: any) { | ||
| lastError = error; | ||
|
|
||
| // Check if it's a rate limit error (429) | ||
| const isRateLimit = | ||
| error?.message?.includes('429') || | ||
| error?.message?.includes('Too Many Requests') || | ||
| error?.message?.includes('max usage reached') || | ||
| error?.code === -32429 || | ||
| error?.status === 429; | ||
|
|
||
| if (isRateLimit && retryCount < maxRetries - 1) { | ||
| retryCount++; | ||
| // Exponential backoff: 2s, 4s, 8s, 16s, 32s | ||
| const delay = Math.min(baseDelay * Math.pow(2, retryCount - 1), 32000); | ||
| this.runtime.logger.warn( | ||
| `[SolanaService] Rate limit hit in batchGetMultipleAccountsInfo(${label}), retrying in ${delay}ms (attempt ${retryCount}/${maxRetries})` | ||
| ); | ||
| await new Promise(resolve => setTimeout(resolve, delay)); | ||
| continue; | ||
| } | ||
|
|
||
| // For non-rate-limit errors or final retry, throw | ||
| if (retryCount >= maxRetries - 1) { | ||
| this.runtime.logger.error( | ||
| `[SolanaService] Failed to fetch accounts after ${maxRetries} attempts:`, | ||
| error | ||
| ); | ||
| throw error; | ||
| } | ||
|
|
||
| retryCount++; | ||
| } |
There was a problem hiding this comment.
Non-rate-limit errors are retried without any backoff delay.
When isRateLimit is false and retryCount < maxRetries - 1, the loop retries immediately without sleeping. This can hammer a failing RPC node with rapid-fire requests for transient (non-429) errors like network timeouts.
Proposed fix — add a delay for non-rate-limit retries too
- // For non-rate-limit errors or final retry, throw
- if (retryCount >= maxRetries - 1) {
+ // For non-rate-limit errors, still apply backoff
+ if (retryCount >= maxRetries - 1) {
this.runtime.logger.error(
`[SolanaService] Failed to fetch accounts after ${maxRetries} attempts:`,
error
);
throw error;
}
retryCount++;
+ const delay = Math.min(baseDelay * Math.pow(2, retryCount - 1), 32000);
+ this.runtime.logger.warn(
+ `[SolanaService] Non-rate-limit error in batchGetMultipleAccountsInfo(${label}), retrying in ${delay}ms (attempt ${retryCount}/${maxRetries})`
+ );
+ await new Promise(resolve => setTimeout(resolve, delay));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error: any) { | |
| lastError = error; | |
| // Check if it's a rate limit error (429) | |
| const isRateLimit = | |
| error?.message?.includes('429') || | |
| error?.message?.includes('Too Many Requests') || | |
| error?.message?.includes('max usage reached') || | |
| error?.code === -32429 || | |
| error?.status === 429; | |
| if (isRateLimit && retryCount < maxRetries - 1) { | |
| retryCount++; | |
| // Exponential backoff: 2s, 4s, 8s, 16s, 32s | |
| const delay = Math.min(baseDelay * Math.pow(2, retryCount - 1), 32000); | |
| this.runtime.logger.warn( | |
| `[SolanaService] Rate limit hit in batchGetMultipleAccountsInfo(${label}), retrying in ${delay}ms (attempt ${retryCount}/${maxRetries})` | |
| ); | |
| await new Promise(resolve => setTimeout(resolve, delay)); | |
| continue; | |
| } | |
| // For non-rate-limit errors or final retry, throw | |
| if (retryCount >= maxRetries - 1) { | |
| this.runtime.logger.error( | |
| `[SolanaService] Failed to fetch accounts after ${maxRetries} attempts:`, | |
| error | |
| ); | |
| throw error; | |
| } | |
| retryCount++; | |
| } | |
| } catch (error: any) { | |
| lastError = error; | |
| // Check if it's a rate limit error (429) | |
| const isRateLimit = | |
| error?.message?.includes('429') || | |
| error?.message?.includes('Too Many Requests') || | |
| error?.message?.includes('max usage reached') || | |
| error?.code === -32429 || | |
| error?.status === 429; | |
| if (isRateLimit && retryCount < maxRetries - 1) { | |
| retryCount++; | |
| // Exponential backoff: 2s, 4s, 8s, 16s, 32s | |
| const delay = Math.min(baseDelay * Math.pow(2, retryCount - 1), 32000); | |
| this.runtime.logger.warn( | |
| `[SolanaService] Rate limit hit in batchGetMultipleAccountsInfo(${label}), retrying in ${delay}ms (attempt ${retryCount}/${maxRetries})` | |
| ); | |
| await new Promise(resolve => setTimeout(resolve, delay)); | |
| continue; | |
| } | |
| // For non-rate-limit errors, still apply backoff | |
| if (retryCount >= maxRetries - 1) { | |
| this.runtime.logger.error( | |
| `[SolanaService] Failed to fetch accounts after ${maxRetries} attempts:`, | |
| error | |
| ); | |
| throw error; | |
| } | |
| retryCount++; | |
| const delay = Math.min(baseDelay * Math.pow(2, retryCount - 1), 32000); | |
| this.runtime.logger.warn( | |
| `[SolanaService] Non-rate-limit error in batchGetMultipleAccountsInfo(${label}), retrying in ${delay}ms (attempt ${retryCount}/${maxRetries})` | |
| ); | |
| await new Promise(resolve => setTimeout(resolve, delay)); | |
| } |
🤖 Prompt for AI Agents
In `@src/service.ts` around lines 930 - 962, The catch block in
batchGetMultipleAccountsInfo currently only applies exponential backoff when
isRateLimit is true, causing immediate retries for other transient errors;
modify the catch handling so that when isRateLimit is false but retryCount <
maxRetries - 1 you still wait using the same/backoff logic (use baseDelay,
retryCount and Math.pow(2, retryCount - 1) bounded by 32000) before continuing,
and keep the existing logging pattern (use this.runtime.logger.warn) to indicate
non-rate-limit retry and delay; ensure retryCount is incremented consistently
and the final throw logic (when retryCount >= maxRetries - 1) remains unchanged.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| this.tokens -= 1; | ||
| entry.resolve(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Rate limiter queue ignores requested token count
Low Severity
The acquire method accepts a count parameter but does not store it in the queue entry. When processQueue drains the queue, it always deducts exactly 1 token per entry (line 131: this.tokens -= 1), regardless of the original count requested. If acquire(N) is called with N > 1 and falls through to the queue, only 1 token is consumed instead of N, violating the rate-limiting contract.
| throw error; | ||
| } | ||
|
|
||
| retryCount++; |
There was a problem hiding this comment.
Non-rate-limit errors retry in tight loop without delay
Medium Severity
In batchGetMultipleAccountsInfo, the comment says "For non-rate-limit errors or final retry, throw" but the code only throws when retryCount >= maxRetries - 1. For non-rate-limit errors (e.g., network failures, deserialization errors) with retryCount below that threshold, execution falls through to retryCount++ and retries immediately with no delay. This creates up to 4 rapid-fire retry attempts against a potentially failing endpoint.
| } | ||
|
|
||
| return info; | ||
| } |
There was a problem hiding this comment.
Duplicated signature extraction and formatting functions across files
Low Severity
extractSignaturesFromText is identically implemented in both lookup-signature.ts (action) and signature-lookup.ts (provider). formatTransactionInfo is also nearly identical across both files. These could be extracted into a shared utility to avoid divergent bug fixes.
Additional Locations (1)
1 participant
Note
Medium Risk
Touches core Solana RPC/service logic and adds long-running background task workflows with caching/retry behavior, which can impact performance and correctness under real RPC rate limits despite good test coverage.
Overview
Adds transaction signature lookup via a new
LOOKUP_SOLANA_SIGNATUREaction andSOLANA_SIGNATURE_LOOKUPprovider, extracting signatures from messages and returning parsed transaction details (status, fees, balance changes, instructions).Introduces RPC-only historical transaction fetching in
SolanaServicewith pagination, batching + concurrency control (p-limit), caching/checkpointing, progress callbacks, and exponential-backoff retry/circuit breaker logic; this is also exposed as a backgroundFETCH_SOLANA_TRANSACTIONStask worker with status polling (getTransactionFetchStatus/waitForTransactionFetch) and optional task chaining.Improves reliability by adding a token-bucket
RateLimiterfor RPC calls (configurable viaSOLANA_RPC_RATE_LIMIT) and agetTransactionWithCostUnitsFallbackto handle Solana RPC responses missingmeta.costUnits. Updates CA lookup decimals to return a keyed map, expands wallet provider to query arbitrary addresses from messages, and tightens runtime setting handling/types; adds docs and test coverage for the new behaviors.Written by Cursor Bugbot for commit 74c5940. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Documentation
Tests