Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion solana/solana-dump/src/dumper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface Options extends DumperOptions {
maxConfirmationAttempts: number
assertLogMessagesNotNull: boolean
validateChainContinuity: boolean
txThreshold?: number
}


Expand All @@ -31,6 +32,7 @@ export class SolanaDumper extends Dumper<Block, Options> {
program.option('--max-confirmation-attempts <N>', 'Maximum number of confirmation attempts', positiveInt, 10)
program.option('--assert-log-messages-not-null', 'Check if tx.meta.logMessages is not null', false)
program.option('--validate-chain-continuity', 'Check if block parent hash matches previous block hash', false)
program.option('--tx-threshold <N>', 'Retry getBlock call if transactions count is less than threshold')
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CLI option for tx-threshold does not specify a parser function (like positiveInt) to validate the input. This means the value will be parsed as a string rather than a number, which will cause issues when passed to the Rpc constructor. Add positiveInt as the parser function to ensure proper validation and type conversion.

Suggested change
program.option('--tx-threshold <N>', 'Retry getBlock call if transactions count is less than threshold')
program.option('--tx-threshold <N>', 'Retry getBlock call if transactions count is less than threshold', positiveInt)

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description for the tx-threshold option mentions 'transactions count' which is grammatically awkward (similar to the error message in rpc.ts). Consider changing it to 'transaction count' for consistency and better readability.

Copilot uses AI. Check for mistakes.
}

protected fixUnsafeIntegers(): boolean {
Expand Down Expand Up @@ -69,7 +71,8 @@ export class SolanaDumper extends Dumper<Block, Options> {
url: options.endpoint,
capacity: Number.MAX_SAFE_INTEGER,
retryAttempts: Number.MAX_SAFE_INTEGER,
requestTimeout: 30_000
requestTimeout: 30_000,
txThreshold: options.txThreshold,
})

return new SolanaRpcDataSource({
Expand Down
1 change: 1 addition & 0 deletions solana/solana-rpc/src/rpc-remote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export type RemoteRpcOptions = Pick<
* Remove vote transactions from all relevant responses
*/
noVotes?: boolean
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The txThreshold field in RemoteRpcOptions lacks documentation. Consider adding a JSDoc comment explaining what this parameter controls, similar to how the noVotes field is documented. This helps users of the API understand the purpose and usage of this option.

Suggested change
noVotes?: boolean
noVotes?: boolean
/**
* Limit the number of transactions returned for a block or response.
* If set, blocks with more transactions than this threshold may be truncated.
*/

Copilot uses AI. Check for mistakes.
txThreshold?: number
}


Expand Down
4 changes: 2 additions & 2 deletions solana/solana-rpc/src/rpc-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import {Commitment, GetBlockOptions, Rpc} from './rpc'
import type {RemoteRpcOptions} from './rpc-remote'


const {noVotes, ...rpcOptions} = getServerArguments<RemoteRpcOptions>()
const {noVotes, txThreshold, ...rpcOptions} = getServerArguments<RemoteRpcOptions>()

const rpc = new Rpc(new RpcClient({
...rpcOptions,
fixUnsafeIntegers: true
}))
}), txThreshold)


getServer()
Expand Down
63 changes: 57 additions & 6 deletions solana/solana-rpc/src/rpc.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {createLogger} from '@subsquid/logger'
import {CallOptions, RpcClient, RpcError, RpcProtocolError} from '@subsquid/rpc-client'
import {RpcCall, RpcErrorInfo} from '@subsquid/rpc-client/lib/interfaces'
import {GetBlock} from '@subsquid/solana-rpc-data'
import {CallOptions, RetryError, RpcClient, RpcError, RpcProtocolError} from '@subsquid/rpc-client'
import {RpcCall, RpcErrorInfo, RpcRequest} from '@subsquid/rpc-client/lib/interfaces'
import {GetBlock, isVoteTransaction} from '@subsquid/solana-rpc-data'
import {assertNotNull} from '@subsquid/util-internal'
import {
array,
B58,
Expand Down Expand Up @@ -49,10 +50,18 @@ export interface RpcApi {


export class Rpc implements RpcApi {
private requests: ThresholdRequests

constructor(
public readonly client: RpcClient,
public readonly log = createLogger('sqd:solana-rpc')
) {}
public readonly txThreshold?: number,
public readonly log = createLogger('sqd:solana-rpc'),
) {
Comment on lines 55 to +59
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The txThreshold parameter in the constructor lacks documentation. Consider adding a JSDoc comment or inline documentation explaining what this parameter does, its expected values, and its impact on behavior. This is especially important since this is a public API parameter that affects retry behavior.

Copilot uses AI. Check for mistakes.
if (this.txThreshold != null) {
assert(this.txThreshold > 0)
}
Comment on lines +60 to +62
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion checks that txThreshold > 0, but there's no check for what happens when txThreshold is 0. While the condition 'if (this.txThreshold && ...)' on line 148 would treat 0 as falsy and skip the validation, it would be clearer to either: (1) explicitly document that 0 disables the feature, or (2) include 0 in the assertion if it's not a valid value. This improves code clarity and prevents confusion about the intended behavior.

Copilot uses AI. Check for mistakes.
this.requests = new ThresholdRequests()
}

call<T=any>(method: string, params?: any[], options?: CallOptions<T>): Promise<T> {
return this.client.call(method, params, options)
Expand Down Expand Up @@ -107,7 +116,7 @@ export class Rpc implements RpcApi {
call[i] = {method: 'getBlock', params}
}
return this.reduceBatchOnRetry<GetBlock | 'skipped' | null | undefined>(call, {
validateResult: getResultValidator(nullable(GetBlock)),
validateResult: (result, req) => this.validateGetBlockResult(result, req),
validateError: captureNoBlockAtSlot
})
}
Expand All @@ -132,6 +141,23 @@ export class Rpc implements RpcApi {

return pack.flat()
}

validateGetBlockResult(result: unknown, req: RpcRequest) {
let validator = getResultValidator(nullable(GetBlock))
let block = validator(result)
if (this.txThreshold && block != null && block.transactions != null) {
let transactions = block.transactions.filter(tx => !isVoteTransaction(tx))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my bad i didn't understand my measurements at first. it doesn't take ~30ms to filter it's just ~0.030ms in average

if (transactions.length < this.txThreshold) {
let slot = req.params![0] as any as number
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type cast 'as any as number' is used to extract the slot from req.params. While this works, it's fragile and bypasses TypeScript's type safety. Consider using a more type-safe approach, such as checking that req.params is defined and that the first element is a number before casting, or defining a proper type for the getBlock request parameters.

Suggested change
let slot = req.params![0] as any as number
if (!Array.isArray(req.params) || typeof req.params[0] !== 'number') {
throw new DataValidationError('invalid getBlock request parameters: expected numeric slot as first parameter')
}
let slot = req.params[0]

Copilot uses AI. Check for mistakes.
let retries = this.requests.get(slot)
if (retries < 3) {
this.requests.inc(slot)
throw new RetryError(`transactions count is less than threshold: ${transactions.length} < ${this.txThreshold}`)
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message uses 'transactions count' which is grammatically awkward. Consider changing it to 'transaction count' (singular) for better readability, as 'count' is already a measure of quantity.

Copilot uses AI. Check for mistakes.
}
Comment on lines +153 to +156
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The retry counting logic increments the counter before throwing the RetryError. This creates an off-by-one issue where the actual number of retries will be one less than expected. When retries reaches 2, it increments to 3, but then the next attempt (which should be the 4th total attempt) won't retry because retries < 3 will be false. The condition should check retries < 2 if you want to allow 3 total attempts (1 initial + 2 retries), or increment after throwing if you want the current behavior to match the intended semantics.

Copilot uses AI. Check for mistakes.
}
}
return block
}
Comment on lines +145 to +160
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validateGetBlockResult method lacks documentation explaining the retry logic and the threshold behavior. Consider adding a JSDoc comment that explains: (1) what txThreshold represents, (2) why vote transactions are filtered out before checking the threshold, (3) the maximum number of retries (currently 3), and (4) what happens when retries are exhausted (the block is returned as-is).

Copilot uses AI. Check for mistakes.
}


Expand All @@ -152,3 +178,28 @@ function getResultValidator<V extends Validator>(validator: V): (result: unknown
}
}
}


class ThresholdRequests {
inner: Map<number, number>
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'inner' field of ThresholdRequests is not marked as private, making it accessible from outside the class. This breaks encapsulation and allows external code to directly modify the internal state. Consider making this field private to maintain proper encapsulation.

Suggested change
inner: Map<number, number>
private inner: Map<number, number>

Copilot uses AI. Check for mistakes.

constructor() {
this.inner = new Map()
}

inc(slot: number) {
if (this.inner.size > 100) {
let keys = this.inner.keys()
for (let i = 0; i < 20; i++) {
let res = keys.next()
this.inner.delete(assertNotNull(res.value))
}
}
Comment on lines +190 to +197
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ThresholdRequests.inc method uses a non-deterministic cleanup strategy that deletes the first 20 entries from the Map when size exceeds 100. Since Map iteration order is insertion order, this means older slots get removed. However, if batch requests are processed out-of-order or slots are retried non-sequentially, this could incorrectly delete entries for slots that are still being retried. Consider using a more robust cleanup strategy, such as removing entries for slots that are outside the current processing window, or using a timestamp-based eviction policy.

Copilot uses AI. Check for mistakes.
let val = this.inner.get(slot) ?? 0
this.inner.set(slot, val + 1)
}

get(slot: number) {
return this.inner.get(slot) ?? 0
}
}
Loading