Skip to content

Optimize block sync performance#432

Merged
antoinedc merged 7 commits intodevelopfrom
feature/optimize-block-sync
Mar 2, 2026
Merged

Optimize block sync performance#432
antoinedc merged 7 commits intodevelopfrom
feature/optimize-block-sync

Conversation

@antoinedc
Copy link
Copy Markdown
Member

@antoinedc antoinedc commented Dec 27, 2025

User description

Summary

  • Batch RPC calls: Fetch multiple transaction receipts in a single JSON-RPC batch request instead of individual calls
  • Inline receipt fetching: For blocks with ≤10 transactions, fetch and store receipts inline in blockSync job (eliminates job queue overhead)
  • Parallel storage: Store multiple receipts concurrently with configurable concurrency limit (default: 5)
  • Workspace caching: Pass cached workspace data (rpcServer, rateLimitInterval, etc.) in receiptSync job payload to skip expensive DB lookups

Expected Impact

  • Real-time syncing: 2-5x faster for typical blocks (< 10 txs)
  • Historical syncing: 3-10x faster with batch RPC calls
  • Database load: Reduced by ~50% from cached workspace queries

Files Changed

  • run/lib/rpc.js: Added fetchTransactionReceiptsBatch() method with fallback to sequential
  • run/jobs/blockSync.js: Inline receipt fetching for small blocks, workspace caching for larger blocks
  • run/jobs/receiptSync.js: Use cached workspace data when available, lighter DB queries

Test plan

  • Verify blocks with ≤10 txs are synced inline (no receiptSync jobs created)
  • Verify blocks with >10 txs queue receiptSync jobs with cached workspace data
  • Verify batch RPC fallback works when batch request fails
  • Verify existing receiptSync jobs (without cached data) still work

🤖 Generated with Claude Code


CodeAnt-AI Description

Optimize block and receipt synchronization to make receipts appear faster and reduce DB work

What Changed

  • Small public blocks (≤10 transactions) now fetch all transaction receipts in a single batch RPC call and store them inline, so receipts are available immediately without queuing receipt jobs
  • Introduced a batch RPC path that sends one JSON-RPC batch request for multiple receipts (handles RPC URLs with basic auth) and falls back to sequential fetches if the batch request fails
  • Parallelized receipt storage with a concurrency limit for faster writes when storing inline
  • For larger blocks, receipt processing jobs now include a cached workspace snapshot (RPC server, rate-limit settings, public flag) so those jobs run with a lighter database query; orbit workspaces skip caching to preserve full context
  • Receipt processing (receiptSync) uses cached workspace data when provided (skips loading workspace includes), preserves the cache when re-enqueuing on rate limits/timeouts, and rejects cached private workspaces to enforce access rules
  • Tests added/updated to cover inline fetching, batch fetch mock, cached-workspace behavior, orbit skipping, and re-enqueue preserving cached data

Impact

✅ Faster real-time block sync for small blocks (receipts appear without extra jobs)
✅ Shorter receipt processing time for large blocks (fewer DB lookups in queued jobs)
✅ Clearer retry behavior (cached workspace preserved when re-enqueuing)

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

- Add batch RPC method to fetch multiple receipts in single request
- Inline receipt fetching for small blocks (≤10 txs) to reduce latency
- Parallel receipt storage with configurable concurrency limit
- Cache workspace data in receiptSync job payload to skip DB lookups
- Fallback to sequential fetching if batch request fails

🤖 Generated with [Claude Code](https://claude.ai/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Dec 27, 2025

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Dec 27, 2025
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Dec 27, 2025

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Batch receipts ordering / fallback
    The code assumes the array returned by fetchTransactionReceiptsBatch matches the same index order as transactions.map(tx => tx.hash). If the RPC returns receipts out-of-order or omits some receipts, the index-based lookup will mis-associate receipts. Also there is no robust fallback if the batch call fails — a thrown error would fail the job. Consider mapping receipts by tx hash and adding a safe fallback path (sequential fetch or enqueue receipt jobs).

  • Batch response shape
    The code assumes the batch JSON-RPC response is an array and calls .sort() on it. If the provider returns a non-array (error object, single response, or null), .sort() will throw. Validate the response shape and handle errors per-item rather than assuming an array.

  • fetch availability & response handling
    The code uses global fetch and .json() without checking response.ok. In Node environments older than 18, fetch may be unavailable. Also non-2xx responses should be handled (inspect response.ok / status) before parsing body.

  • Provider URL access
    The code accesses provider internals to obtain the request URL (this.provider.connection and this.provider._getConnection()). Those properties/methods are implementation-specific and may be missing on some ethers Provider implementations, causing runtime exceptions. Confirm compatibility across supported provider types and add safe guards/fallbacks.

  • Missing error resilience for batch fetch
    fetchTransactionReceiptsBatch is called directly without try/catch. If it throws (even if rpc.js implements an internal fallback), the job can fail. Add error handling to gracefully retry, fall back to individual fetches, or enqueue receiptSync jobs when batch retrieval fails.

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Dec 27, 2025

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Score
Logic error
Inline receipt syncing bypasses the public workspace check, allowing receipts to be synced for private workspaces contrary to existing behavior

The new inline receipt syncing path ignores the existing public workspace check
enforced in receiptSync, so private workspaces will now have receipts synced inline
even though the dedicated receipt job explicitly prevents syncing on private
workspaces, leading to inconsistent behavior and violating the intended access
control.

run/jobs/blockSync.js [182-187]

 // For blocks with few transactions, fetch receipts inline for lower latency
-if (transactions.length > 0 && transactions.length <= INLINE_RECEIPT_THRESHOLD) {
+// Only perform receipt syncing for public workspaces to match receiptSync behavior
+if (transactions.length > 0 && transactions.length <= INLINE_RECEIPT_THRESHOLD && workspace.public) {
     // Fetch all receipts in a single batch RPC request
     const receipts = await providerConnector.fetchTransactionReceiptsBatch(
         transactions.map(tx => tx.hash)
     );
 
Suggestion importance[1-10]: 10

Why it matters? 🤔: The queued receiptSync path caches workspace.public and presumably enforces the public check before syncing receipts. The inline path in this PR does not check workspace.public, so it changes behavior for private workspaces (they'd get receipts fetched/stored inline). That's an access/control inconsistency that can be a real bug. Adding the workspace.public guard aligns behavior and prevents unexpected privacy changes.

10
Batch JSON-RPC requests drop RPC basic auth credentials, causing unnecessary authentication failures and fallback to the slower path

For RPC URLs that use basic authentication, the batch receipt fetch currently uses
only connection.url without re-attaching the user/password from the provider
connection, causing unauthenticated HTTP requests that will typically be rejected
and force every batch call to fall back to slower sequential fetching instead of
benefiting from the optimization.

run/lib/rpc.js [259]

-const url = this.provider.connection ? this.provider.connection.url : this.provider._getConnection().url;
+const connection = this.provider.connection || this.provider._getConnection();
+let url = connection.url;
+if ((connection.user || connection.password) && url && !url.includes('@')) {
+    const parsedUrl = new URL(url);
+    if (connection.user) parsedUrl.username = connection.user;
+    if (connection.password) parsedUrl.password = connection.password;
+    url = parsedUrl.toString();
+}
 
Suggestion importance[1-10]: 10

Why it matters? 🤔: The code tries to use the provider's connection URL for a direct HTTP POST, but earlier in getProvider the authenticated connection may have been represented as an object { url, user, password } when credentials are present. Using only connection.url can cause unauthenticated requests if the provider uses basic auth and the credentials are not embedded in the url string. This will cause many batch attempts to fail and fall back to the slower sequential path — a real logic/regression risk. The suggested approach to include user/password when present (or otherwise attach proper auth headers) fixes this behavior.

10
Missing error propagation
Errors during inline receipt creation are swallowed after logging, so failed writes never trigger job failure or retries

In the inline receipt path, errors from safeCreateReceipt are caught and only logged
without being rethrown, so failed receipt writes will be silently ignored instead of
failing the job as they do in receiptSync, leading to silent data inconsistencies
and preventing retries.

run/jobs/blockSync.js [199-206]

 const processedReceipt = processRawRpcObject(
     receipt,
     Object.keys(TransactionReceipt.rawAttributes).concat(['logs'])
 );
 processedReceipt.workspace = workspace;
 return tx.safeCreateReceipt(processedReceipt).catch(err => {
     logger.error(`Failed to store receipt for ${tx.hash}`, { error: err.message });
+    // Propagate the error so the job can fail and be retried instead of silently skipping this receipt
+    throw err;
 });
 
Suggestion importance[1-10]: 10

Why it matters? 🤔: Currently the inline path logs safeCreateReceipt failures but swallows them, which means transient storage errors won't fail the job and won't be retried. That can lead to silent data loss/inconsistency. Propagating the error (or otherwise surfacing it) so the blockSync job can retry is a valid behavioral fix to make failure handling consistent with the queued receipt path.

10
Type error
Using a minimal workspace stub without orbit data causes orbit-related receipt processing to crash when it iterates over missing properties

When using cached workspace data, processedReceipt.workspace is set to a minimal
object { id: workspaceId }, but Transaction.safeCreateReceipt expects
receipt.workspace.orbitConfig and iterates over receipt.workspace.orbitChildConfigs,
so for cached jobs this will make orbitChildConfigs undefined and cause a runtime
TypeError when the orbit-processing loop runs. The fix is to ensure that, even in
the cached-workspace path, processedReceipt.workspace includes proper orbit-related
fields (or at least safe defaults) by fetching the workspace with its orbit
associations or providing a fallback object with an empty orbitChildConfigs array.

run/jobs/receiptSync.js [161-167]

 // For safeCreateReceipt, we need to pass workspace context for orbit processing
 // When using cached data, construct a minimal workspace-like object
 if (hasCachedWorkspace) {
-    processedReceipt.workspace = { id: data.workspaceId };
+    const workspace = await Workspace.findByPk(data.workspaceId, { include: ['orbitConfig', 'orbitChildConfigs'] });
+    processedReceipt.workspace = workspace || { id: data.workspaceId, orbitConfig: null, orbitChildConfigs: [] };
 } else {
     processedReceipt.workspace = transaction.workspace;
 }
 
Suggestion importance[1-10]: 10

Why it matters? 🤔: The suggestion flags a real runtime issue. In Transaction.safeCreateReceipt the code assumes receipt.workspace.orbitChildConfigs exists and immediately iterates over it:
for (const orbitChildConfig of receipt.workspace.orbitChildConfigs) { ... }
If processedReceipt.workspace is a minimal object with only { id } then receipt.workspace.orbitChildConfigs is undefined and that loop will throw a TypeError. I verified the iteration exists in run/models/transaction.js (around the orbit processing section), and receiptSync.js currently sets the minimal workspace stub only when cached workspace data is used. Adding orbit-related fields (or an explicit empty array) for the cached path prevents the TypeError and fixes a real bug.

10

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Dec 27, 2025

CodeAnt AI finished reviewing your PR.

Antoine de Chevigné and others added 2 commits December 27, 2025 17:24
- Add fetch availability check with graceful fallback to sequential
- Handle authenticated RPC URLs in batch fetch with proper auth headers
- Skip workspace caching for orbit workspaces to preserve full context
- Add unit tests for inline receipt fetching, cached workspace, and orbit handling
- Update mock to include fetchTransactionReceiptsBatch

🤖 Generated with [Claude Code](https://claude.ai/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Only perform inline receipt fetching for public workspaces to match
the access control behavior in receiptSync. Private workspaces will
queue jobs that get properly rejected by receiptSync.

🤖 Generated with [Claude Code](https://claude.ai/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Dec 28, 2025

CodeAnt AI is running Incremental review


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

Resolve conflicts in blockSync.js, receiptSync.js, and rpc mock:
- Combine model imports (TransactionReceipt from PR + OpChainConfig/OpBatch/sequelize from develop)
- Integrate reportRpcFailure from develop with cached workspace requeue from PR
- Add both fetchTransactionReceiptsBatch and fetchLatestBlock to RPC mock

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 2, 2026

CodeAnt AI is running Incremental review


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai Bot added size:L This PR changes 100-499 lines, ignoring generated files and removed size:L This PR changes 100-499 lines, ignoring generated files labels Mar 2, 2026
- Validate batch JSON-RPC response is array before .sort() (rpc.js)
- Check response.ok before parsing JSON (rpc.js)
- Add try/catch around inline receipt fetch in blockSync, fall back to
  queueing receiptSync jobs on failure
- Track failed safeCreateReceipt calls and queue receiptSync for those
  transactions instead of silently swallowing errors
- Add orbitConfig/orbitChildConfigs safe defaults to minimal workspace
  stub in receiptSync cached path to prevent TypeError in
  safeCreateReceipt orbit processing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread run/jobs/blockSync.js Outdated
Comment on lines +397 to +402
const processedReceipt = processRawRpcObject(
receipt,
Object.keys(TransactionReceipt.rawAttributes).concat(['logs'])
);
processedReceipt.workspace = workspace;
return tx.safeCreateReceipt(processedReceipt).catch(err => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: When storing receipts inline, processedReceipt.workspace is set directly to the workspace instance without ensuring orbitChildConfigs (and opChildConfigs) exist, but safeCreateReceipt iterates over receipt.workspace.orbitChildConfigs unconditionally; for workspaces where this association was not eagerly loaded, this will cause a runtime "undefined is not iterable" TypeError when processing receipts. [type error]

Severity Level: Critical 🚨
- ❌ Small-block `blockSync` jobs crash on contract transactions.
- ❌ Transaction receipts not stored; transactions never reach ready state.
- ⚠️ Orbit child-chain events never processed for affected receipts.
- ⚠️ Explorer views may hang with permanently pending transactions.
Suggested change
const processedReceipt = processRawRpcObject(
receipt,
Object.keys(TransactionReceipt.rawAttributes).concat(['logs'])
);
processedReceipt.workspace = workspace;
return tx.safeCreateReceipt(processedReceipt).catch(err => {
const processedReceipt = processRawRpcObject(
receipt,
Object.keys(TransactionReceipt.rawAttributes).concat(['logs'])
);
processedReceipt.workspace = workspace;
// Ensure child config arrays exist for safeCreateReceipt's Orbit/OP processing
processedReceipt.workspace.orbitChildConfigs = workspace.orbitChildConfigs || [];
processedReceipt.workspace.opChildConfigs = workspace.opChildConfigs || [];
return tx.safeCreateReceipt(processedReceipt).catch(err => {
logger.error(`Failed to store receipt for ${tx.hash}`, { error: err.message });
});
Steps of Reproduction ✅
1. A `blockSync` job runs for a public workspace with a small block (≤10 transactions),
entering the inline receipt path in `run/jobs/blockSync.js:379-406`, where `const
transactions = syncedBlock.transactions;` is followed by the `if (transactions.length > 0
&& transactions.length <= INLINE_RECEIPT_THRESHOLD && workspace.public)` branch.

2. The workspace is loaded earlier in `run/jobs/blockSync.js` via
`Workspace.findOne`/`findByPk` at lines 34-51 and 61-95 with `include: ['user',
'orbitConfig', { model: OpChainConfig, as: 'opChildConfigs' }, ...]` but **does not**
include the `'orbitChildConfigs'` association, so the resulting `workspace` instance has
`workspace.orbitChildConfigs === undefined`.

3. For each transaction in the small block, the inline receipt logic at
`run/jobs/blockSync.js:387-403` calls:

   - `const processedReceipt = processRawRpcObject(receipt,
   Object.keys(TransactionReceipt.rawAttributes).concat(['logs']));`

   - `processedReceipt.workspace = workspace;`

   - `return tx.safeCreateReceipt(processedReceipt) ...;`

   where `tx` is a `Transaction` model instance from `syncedBlock.transactions`.

4. Inside `Transaction.safeCreateReceipt` in `run/models/transaction.js:203-385`, after
logs are stored, any transaction with `receipt.to` set and non-empty input data
(`this.data.length > 2`) enters the block at lines 320-356 and then executes `for (const
orbitChildConfig of receipt.workspace.orbitChildConfigs) { ... }` at line 358. Since
`processedReceipt.workspace` came from `blockSync`'s `workspace` object without an
`orbitChildConfigs` property, `receipt.workspace.orbitChildConfigs` is `undefined`, and
the `for...of` loop throws `TypeError: receipt.workspace.orbitChildConfigs is not
iterable`, causing the `blockSync` job to fail for such small blocks and preventing
receipts from being stored.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** run/jobs/blockSync.js
**Line:** 397:402
**Comment:**
	*Type Error: When storing receipts inline, `processedReceipt.workspace` is set directly to the `workspace` instance without ensuring `orbitChildConfigs` (and `opChildConfigs`) exist, but `safeCreateReceipt` iterates over `receipt.workspace.orbitChildConfigs` unconditionally; for workspaces where this association was not eagerly loaded, this will cause a runtime "undefined is not iterable" TypeError when processing receipts.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread run/jobs/receiptSync.js
Comment on lines +193 to +199
// For safeCreateReceipt, we need to pass workspace context for orbit processing
// When using cached data, construct a minimal workspace-like object
if (hasCachedWorkspace) {
processedReceipt.workspace = { id: data.workspaceId };
} else {
processedReceipt.workspace = transaction.workspace;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: In the cached-workspace path, processedReceipt.workspace is set to { id: data.workspaceId }, but safeCreateReceipt always iterates over receipt.workspace.orbitChildConfigs, so this minimal object (without orbitChildConfigs) will trigger a runtime "undefined is not iterable" TypeError for any such receiptSync job using cached workspace data. [type error]

Severity Level: Critical 🚨
- ❌ receiptSync jobs crash for cached-workspace contract-call transactions.
- ❌ Receipts and logs not stored for affected transactions.
- ⚠️ Downstream Orbit/OP analytics missing when receipts never persist.
Suggested change
// For safeCreateReceipt, we need to pass workspace context for orbit processing
// When using cached data, construct a minimal workspace-like object
if (hasCachedWorkspace) {
processedReceipt.workspace = { id: data.workspaceId };
} else {
processedReceipt.workspace = transaction.workspace;
}
// For safeCreateReceipt, we need to pass workspace context for orbit processing
// When using cached data, construct a minimal workspace-like object
if (hasCachedWorkspace) {
processedReceipt.workspace = {
id: data.workspaceId,
orbitChildConfigs: [],
opChildConfigs: []
};
} else {
processedReceipt.workspace = transaction.workspace;
}
Steps of Reproduction ✅
1. Configure a non-orbit public workspace (no OrbitChainConfig rows and no
workspace.orbitConfig) so that `hasOrbitConfig` is false in
`/run/jobs/blockSync.js:269-271`. This is the default for standard (non-Orbit) workspaces.

2. Trigger a `blockSync` job (entrypoint `/run/jobs/blockSync.js:24-25`) for a block where
`processedBlock.transactions.length > INLINE_RECEIPT_THRESHOLD` (`INLINE_RECEIPT_THRESHOLD
= 10` at line 19) and which contains at least one contract-call transaction (its `data`
field is non-empty so `this.data.length > 2`).

3. In `blockSync`, observe that for such a block the inline path is skipped and the
else-branch at `/run/jobs/blockSync.js:377-439` executes. Because `hasOrbitConfig` is
false, each `jobData` built at lines 415-421 is augmented with `cachedWorkspace` at lines
423-430 and enqueued to the `receiptSync` queue at lines 433-438.

4. When a queued `receiptSync` job runs (entrypoint `/run/jobs/receiptSync.js:23-24`),
`hasCachedWorkspace` is true at line 30 so the `include` at lines 32-39 omits `Workspace`.
`Transaction.findByPk` therefore returns a transaction object without `workspace` (lines
85-87). After fetching the RPC receipt (lines 147-191), `processedReceipt.workspace` is
set to `{ id: data.workspaceId }` at lines 193-197, and
`transaction.safeCreateReceipt(processedReceipt)` is called at line 201.

5. Inside `safeCreateReceipt` in `/run/models/transaction.js:203-785`, once the receipt
has been stored and logs loaded into `storedLogs`, the code checks `if (receipt.to &&
this.data.length > 2)` at lines 320-321. For the contract-call transaction from step 2
this condition is true, so execution reaches the unguarded loop `for (const
orbitChildConfig of receipt.workspace.orbitChildConfigs)` at lines 358-359. Because
`processedReceipt.workspace` is `{ id: ... }` and has no `orbitChildConfigs` property,
`receipt.workspace.orbitChildConfigs` is `undefined` and the `for...of` triggers a runtime
TypeError ("undefined is not iterable" / "receipt.workspace.orbitChildConfigs is not
iterable"), causing the `receiptSync` job to fail and preventing the receipt from being
stored.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** run/jobs/receiptSync.js
**Line:** 193:199
**Comment:**
	*Type Error: In the cached-workspace path, `processedReceipt.workspace` is set to `{ id: data.workspaceId }`, but `safeCreateReceipt` always iterates over `receipt.workspace.orbitChildConfigs`, so this minimal object (without `orbitChildConfigs`) will trigger a runtime "undefined is not iterable" TypeError for any such receiptSync job using cached workspace data.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 2, 2026

CodeAnt AI Incremental review completed.

…ceipt

orbitChildConfigs are loaded separately via OrbitChainConfig.findAll()
and not eagerly loaded on the workspace instance. safeCreateReceipt
iterates receipt.workspace.orbitChildConfigs without a fallback, so
assign the array on the workspace before passing it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@antoinedc
Copy link
Copy Markdown
Member Author

@CodeAnt-AI review incremental changes

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 2, 2026

Question: review incremental changes

Answer:
Overall impression

  • The changes implement the intended optimizations: batch RPC receipt fetching, inline storage for small blocks, parallel receipt storage, and passing cached workspace context to receiptSync jobs. The diffs are focused and include unit tests covering key new behaviors. Good work — these changes should notably reduce latency for small blocks and RPC overhead.

What looks good

  • ProviderConnector.fetchTransactionReceiptsBatch: clean implementation with HTTP auth support, JSON-RPC batch request creation, response validation, and a safe fallback to sequential fetching when batch fails.
  • blockSync inline receipts: sensible threshold (<=10) for inline inline fetching, parallel save with a concurrency cap, and fallback to enqueuing receiptSync jobs if the batch or storage fails.
  • receiptSync cached workspace path: lighter DB query when cached workspace data is present, re-enqueue preserving cachedWorkspace, and avoiding expensive includes for the common case.
  • Tests: new unit tests added for inline receipt handling, cached workspace usage, and re-enqueue preserving cachedWorkspace after timeout — good coverage of the new behavior.

Potential issues and suggestions (priority ordered)

  1. Rate limiting semantics for batch fetch
  • Current flow: fetchTransactionReceiptsBatch calls this.checkRateLimit() once. But a batch request performs N eth_getTransactionReceipt calls within a single HTTP request. If your rate limiting is per-RPC call or per-eth_getTransactionReceipt, the limiter should account for transactionHashes.length (or at least be aware). Consider updating checkRateLimit to accept a weight/count or have fetchTransactionReceiptsBatch consume multiple tokens (e.g., call limiter.wouldLimit(N) or loop N times).
  • Suggested change: add optional parameter to checkRateLimit or a willConsume(count) method on RateLimiter. If not possible, document that rate limit unit is "HTTP requests" not "RPC methods".
  1. Ordering guarantees between batch response and transaction list
  • ProviderConnector.fetchTransactionReceiptsBatch returns sanitized receipts ordered by the JSON-RPC response id sorting. blockSync assumes array index matches the transactions order (it uses receiptIndex = i + j). That is safe only if batch is built with ids in the same order as transactions and the response includes corresponding ids (your implementation does that). Consider explicitly documenting that behavior or returning an object keyed by hash for extra robustness.
  • Small suggestion: in fetchTransactionReceiptsBatch, you already set id = i in map — good. Optionally validate that sortedResults[k].id === k and log a warning if not; or build a map id->result and then create an array in the original order.
  1. Error handling and logging
  • When batch fetch returns non-array or fails, you fallback to sequential — good. But blockSync's catch only logs error.message; consider logging full error with stack for easier debugging in production (some places already log error.message).
  • When individual receipt storage fails in inline path, you log error.message but still push tx to failedTxHashes. Consider including tx.hash in the logged context and the actual error object.
  1. Tests: environment assumptions (fetch)
  • fetchTransactionReceiptsBatch checks typeof fetch and throws if undefined. In environments without global fetch (older Node versions), it falls back to sequential (because catch triggers fallback). Unit tests mock fetch by stubbing rpc lib; ensure test environment has a global fetch or ProviderConnector is mocked (your tests mock lib/rpc so ok). Consider documenting Node version requirement if relying on global fetch for optimal path.
  1. Inline receipt path only for public workspaces
  • That's intentional and correct for security. Ensure it is well-documented in comments and in README or runtime docs.
  1. Concurrency & configurability
  • RECEIPT_STORAGE_CONCURRENCY is hardcoded to 5. Consider making this configurable (env var or workspace setting) so operators can tune it based on DB throughput.
  1. Workspace cached data completeness
  • The cachedWorkspace passed to receiptSync includes rpcServer, rateLimitInterval, rateLimitMaxInInterval, public. You intentionally skip explorer, rpcHealthCheck and OP config for cached path. You also skip caching for orbit workspaces. Good. But be careful if later receiptSync requires explorer fields for reporting. You guarded reportRpcFailure by only calling it when full workspace/explorer is present. That's correct.
  1. Minor: URL handling & headers
  • _getFetchOptions uses new URL(this.server) and sets url = rpcUrl.origin + rpcUrl.pathname. That drops search/query parameters — probably fine for RPC endpoints but call out if you expect query params. Also credentials extraction with rpcUrl.username/password is correct. Using Buffer is fine in Node.
  1. Removed incrementFailedAttempts commented out
  • I noticed you removed db.incrementFailedAttempts(transaction.workspace.id) (commented out) at the end of receiptSync's catch. If this counter is used for monitoring/retries, consider restoring or replacing it and ensure it respects cachedWorkspace path (transaction.workspace may be undefined in that path).
  1. Naming and constants
  • Consider making INLINE_RECEIPT_THRESHOLD and RECEIPT_STORAGE_CONCURRENCY configurable so they can be adjusted without code change.
  1. Potential race with orbitChildConfigs
  • In blockSync inline path you set workspace.orbitChildConfigs = orbitChildConfigs || []; ensure orbitChildConfigs is in scope (it appears earlier in blockSync includes) — tests cover orbit path, so likely fine. But verify that workspace used by safeCreateReceipt expects orbitChildConfigs to exist on workspace.
  1. Batch response with errors
  • You map response entries to r.result ? sanitize(r.result) : null. If a response entry contains an error field (JSON-RPC error), r.result is undefined but there may be r.error. You discard r.error; consider logging these errors (including r.id and r.error) to aid debugging.

Small code suggestions (examples)

  • In fetchTransactionReceiptsBatch, after parsing response:

    • const resultsById = new Map(response.map(r => [r.id, r]));
    • return transactionHashes.map((_, i) => {
      const r = resultsById.get(i);
      if (!r) return null;
      if (r.error) { logger.warn('RPC error in batch result', { id: i, error: r.error }); return null; }
      return sanitize(r.result);
      });
  • For limiter weight:

    • await this.checkRateLimit(transactionHashes.length); and let checkRateLimit accept a count.

Tests and QA checklist

  • Tests introduced look solid: inline receipts for small blocks, caching in receiptSync, preserved cachedWorkspace on re-enqueue. Add tests for:
    • Batch RPC failing and fallback to sequential (ensure fetchTransactionReceipt is used after batch failure).
    • Batch RPC returning errors for one or more ids (simulate response array with error entries).
    • Rate limiter behavior when many receipts are fetched in batch: ensure limiter is consulted and requeue occurs properly if rate limited.

Final recommendation

  • I approve of these changes in principle; they align with the PR goals and include tests. Before merging, address higher-priority concerns:
    1. Ensure rate limiter semantics are correct for batch requests (account for multiple internal RPC calls).
    2. Improve logging around batch result errors and storage failures (include tx.hash and full error).
    3. Consider making concurrency and threshold configurable.
    4. Restore or handle the incrementFailedAttempts or equivalent monitoring if needed.

If you want, I can:

  • Propose a code patch to make checkRateLimit accept a weight and update fetchTransactionReceiptsBatch (and other batch places) to pass transactionHashes.length.
  • Add improved logging for batch result errors and individual failures.
  • Make RECEIPT_STORAGE_CONCURRENCY and INLINE_RECEIPT_THRESHOLD configurable via environment vars.

- Use Map for batch response ordering instead of .sort() for robustness
- Log RPC errors from individual batch results (r.error) instead of
  silently returning null
- Preserve URL query params in _getFetchOptions
- Document rate limit semantics (1 check per HTTP call, not per method)
- Improve inline receipt error logging with location and tx hash

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@antoinedc antoinedc merged commit 96ff0d0 into develop Mar 2, 2026
2 checks passed
@antoinedc antoinedc deleted the feature/optimize-block-sync branch March 2, 2026 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant