Feature/endorsement chain boe - #164
Closed
manishdex25 wants to merge 13 commits into
Closed
Conversation
… dense block ranges
…c into feature/endorsement-chain-boe
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesEscrow retrieval and endorsement-chain processing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant useEndorsementChain
participant fetchEscrowTransfersV5
participant fetchLogsChunked
participant Infura
participant retrieveEndorsementChain
useEndorsementChain->>fetchEscrowTransfersV5: request obligation transfers with status mode
fetchEscrowTransfersV5->>fetchLogsChunked: start provider-aware scan
fetchLogsChunked->>Infura: query bounded block ranges
Infura-->>fetchLogsChunked: return logs or range errors
fetchLogsChunked-->>fetchEscrowTransfersV5: return ordered logs and scan status
fetchEscrowTransfersV5-->>useEndorsementChain: return parsed transfers
retrieveEndorsementChain->>Infura: fetch unique block timestamps
Infura-->>retrieveEndorsementChain: return timestamps
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/endorsement-chain/obligation.ts (1)
15-15: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRestore the removed public API before merging.
fetchObligationEndorsementChainwas exported and documented at the package root. Keep a deprecated wrapper that delegates tofetchEndorsementChain, or document this as a breaking release with migration notes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/endorsement-chain/obligation.ts` at line 15, Restore the public fetchObligationEndorsementChain API as a deprecated wrapper that delegates to fetchEndorsementChain, and re-export it from the package root with its documentation. Preserve existing behavior while directing callers toward fetchEndorsementChain.
🧹 Nitpick comments (6)
src/core/endorsement-chain/fetchLogsChunked.ts (3)
14-14: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMatch the URL host, not the whole URL string.
INFURA_HOST_REtests the complete RPC URL. A URL that containsinfura.ioin the path or query, but has a different host, selects the Infura scan path. Parse the URL and test the hostname.♻️ Proposed refactor
-const INFURA_HOST_RE = /infura\.io/i; +const INFURA_HOST_RE = /(^|\.)infura\.io$/i; @@ export function isInfuraProvider(provider: Provider | ethersV6.Provider): boolean { - return INFURA_HOST_RE.test(getProviderRpcUrl(provider)); + const url = getProviderRpcUrl(provider); + if (!url) return false; + try { + return INFURA_HOST_RE.test(new URL(url).hostname); + } catch { + return false; + } }Also applies to: 40-58
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/endorsement-chain/fetchLogsChunked.ts` at line 14, Update INFURA_HOST_RE usage in the fetchLogsChunked flow to parse the RPC URL and test only its hostname, rather than matching the complete URL string. Preserve Infura path selection only when the parsed URL’s host matches infura.io, including the existing logic around the affected lines.
202-255: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce the cognitive complexity of
scanLogsBackward.SonarCloud reports a failure for this function (complexity 23, limit 15). The mint-index search and the error branch are separable. Extract the mint handling into a helper, for example
sliceFromMint(chunkLogs, isMintLog), and reuse it. This also clears the pipeline gate.Sonar additionally flags the in-place
reverse()at line 254. Use[...chunkGroups].reverse().flat()to avoid mutating the accumulator.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/endorsement-chain/fetchLogsChunked.ts` around lines 202 - 255, Reduce the cognitive complexity of scanLogsBackward by extracting the mint-index search and slicing logic into a separate sliceFromMint helper, then use that helper in the chunk-processing loop while preserving the existing stop and log-collection behavior. Keep the error handling behavior unchanged, and replace the mutating chunkGroups.reverse().flat() call with a copied-array reverse before flattening.Source: Linters/SAST tools
115-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated bounded-concurrency worker pool. Both files implement the same pattern: a shared index cursor,
Math.min(limit, items.length)workers, and awhileloop that claims the next index. Extract one helper, for examplemapWithConcurrency(items, limit, fn), and call it from both sites.
src/core/endorsement-chain/fetchLogsChunked.ts#L115-L127: replace thenext++worker loop inscanLogsParallelFixedwith the shared helper overwindows.src/core/endorsement-chain/retrieveEndorsementChain.ts#L19-L35: replace thenextBlockIndex++worker loop with the shared helper overuniqueBlockNumbers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/endorsement-chain/fetchLogsChunked.ts` around lines 115 - 127, Extract the duplicated bounded-concurrency worker-loop logic into a shared mapWithConcurrency(items, limit, fn) helper. In src/core/endorsement-chain/fetchLogsChunked.ts lines 115-127, update scanLogsParallelFixed to map over windows through the helper; in src/core/endorsement-chain/retrieveEndorsementChain.ts lines 19-35, update the corresponding block in the endorsement-chain retrieval flow to map over uniqueBlockNumbers through the same helper, preserving result ordering and the existing concurrency limits.src/core/endorsement-chain/fetchEscrowTransfer.ts (2)
58-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
includeObligationStatusto reflect its real meaning.The flag no longer only adds status events. It selects the contract ABI (
ObligationEscrow__factory.abi), the filter set, and the Infura range strategy. A name such asisObligationEscrowdescribes that behavior and prevents a caller from passingtruefor a classic V5 escrow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/endorsement-chain/fetchEscrowTransfer.ts` around lines 58 - 78, Rename the includeObligationStatus parameter to isObligationEscrow throughout fetchEscrowTransfersV5 and its related call chain, including the ABI selection, filter selection, and Infura range strategy, so callers clearly indicate the escrow contract type rather than merely requesting status events.
271-285: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDo not discard a valid
mintBlockwhenshredBlock()fails.One
tryblock wraps both calls. IfmintBlock()succeeds andshredBlock()reverts or is absent from the ABI, the function returnsnull. The caller then falls back to the unbounded backward scan even though a valid lower bound is known. Scope thetryto each call.♻️ Proposed fix
- const shredBlock = Number(await contract.shredBlock()); - const toBlock = - Number.isFinite(shredBlock) && shredBlock > 0 ? shredBlock : await provider.getBlockNumber(); + let shredBlock = Number.NaN; + try { + shredBlock = Number(await contract.shredBlock()); + } catch { + // shredBlock is optional; fall back to the chain tip. + } + const toBlock = + Number.isFinite(shredBlock) && shredBlock > 0 ? shredBlock : await provider.getBlockNumber();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/endorsement-chain/fetchEscrowTransfer.ts` around lines 271 - 285, Update the block-range lookup around mintBlock and shredBlock so failures are handled per call rather than by one shared try block. Preserve a valid mintBlock lower bound when shredBlock() fails or is unavailable by falling back to provider.getBlockNumber(), while still returning null when mintBlock() cannot be read or is invalid; keep the existing range validation.src/core/endorsement-chain/helpers.ts (1)
93-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake
TYPE_PRECEDENCEexhaustiveness compiler-checked.The list is typed as
TransferEventType[]. If a new member is added toTransferEventType, the compiler accepts the incomplete list, andidentifyEventTypeFromLogsthrowsUnable to identify event typeat runtime. Type the precedence data so a missing member failsnpm run type-check.♻️ Proposed refactor
-const TYPE_PRECEDENCE: TransferEventType[] = [ +// Lower number wins. Every TransferEventType must appear, so a new member fails type-check. +const TYPE_RANK: Record<TransferEventType, number> = { + INITIAL: 0, + RETURN_TO_ISSUER_REJECTED: 1, + // ...remaining members, including TRANSFER_HOLDER / TRANSFER_BENEFICIARY with a low rank +};Then order the present types by rank, and keep the existing
TRANSFER_OWNERScombination check.As per coding guidelines: "Before completing work, run
npm run type-checkandnpm run lint".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/endorsement-chain/helpers.ts` around lines 93 - 105, Make TYPE_PRECEDENCE exhaustively compiler-checked against every TransferEventType member, using a typed mapped structure or equivalent compile-time enforcement so newly added members fail npm run type-check until ranked. Preserve the existing precedence ordering, ensure identifyEventTypeFromLogs still ranks the present types correctly, and retain the TRANSFER_OWNERS combination check; run npm run type-check and npm run lint afterward.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 905-910: Update the README endorsement-chain example to pass the
encryption key ID as the fourth argument to fetchEndorsementChain, reusing the
options.id value established when encryption is enabled. Keep the existing
obligationRegistry, tokenId, and provider arguments unchanged.
In `@src/core/endorsement-chain/fetchEscrowTransfer.ts`:
- Around line 242-257: Add a bounded deadline or iteration budget to the
backward scan initiated by the visible scanLogsBackward call, and pass it
through so the search cannot continue indefinitely toward block 0 when isMintLog
never matches or decoding fails. Preserve the existing mint/status matching
behavior while ensuring the scan terminates within the configured budget.
In `@src/core/endorsement-chain/fetchLogsChunked.ts`:
- Around line 223-236: The mint-marker search in the chunk grouping logic must
select the earliest matching log so preceding mint events are preserved. Update
the loop around mintIndex in fetchLogsChunked to scan chunkLogs from the
beginning and stop at the first isMintLog match, while keeping the existing
slice and break behavior.
- Around line 159-171: Replace the spread-based logs.push call in the
scanLogsChunked loop with an aggregation approach that does not expand remaining
into function arguments, such as concatenation or reassignment. Preserve the
existing ordering and continue behavior after incorporating the complete result
from scanLogsParallelFixed.
- Around line 103-130: Update scanLogsParallelFixed to add per-window retry
handling around getLogsRange: when a window fails due to a range, response-size,
or transient RPC error, split that window into smaller sub-ranges and retry each
sub-range, recursively or iteratively, before propagating unrecoverable errors.
Preserve successful results from other windows and return logs from all
completed sub-ranges rather than allowing one failed window to discard them
through Promise.all.
- Around line 43-49: Update the provider URL resolution used by
fetchAllTransfers to handle ethers v6 FallbackProvider and BrowserProvider
wrappers, rather than relying only on _getConnection(). Use explicit provider
metadata or inspect supported underlying providers so an embedded Infura
JsonRpcProvider is detected and the ranged path is selected.
---
Outside diff comments:
In `@src/core/endorsement-chain/obligation.ts`:
- Line 15: Restore the public fetchObligationEndorsementChain API as a
deprecated wrapper that delegates to fetchEndorsementChain, and re-export it
from the package root with its documentation. Preserve existing behavior while
directing callers toward fetchEndorsementChain.
---
Nitpick comments:
In `@src/core/endorsement-chain/fetchEscrowTransfer.ts`:
- Around line 58-78: Rename the includeObligationStatus parameter to
isObligationEscrow throughout fetchEscrowTransfersV5 and its related call chain,
including the ABI selection, filter selection, and Infura range strategy, so
callers clearly indicate the escrow contract type rather than merely requesting
status events.
- Around line 271-285: Update the block-range lookup around mintBlock and
shredBlock so failures are handled per call rather than by one shared try block.
Preserve a valid mintBlock lower bound when shredBlock() fails or is unavailable
by falling back to provider.getBlockNumber(), while still returning null when
mintBlock() cannot be read or is invalid; keep the existing range validation.
In `@src/core/endorsement-chain/fetchLogsChunked.ts`:
- Line 14: Update INFURA_HOST_RE usage in the fetchLogsChunked flow to parse the
RPC URL and test only its hostname, rather than matching the complete URL
string. Preserve Infura path selection only when the parsed URL’s host matches
infura.io, including the existing logic around the affected lines.
- Around line 202-255: Reduce the cognitive complexity of scanLogsBackward by
extracting the mint-index search and slicing logic into a separate sliceFromMint
helper, then use that helper in the chunk-processing loop while preserving the
existing stop and log-collection behavior. Keep the error handling behavior
unchanged, and replace the mutating chunkGroups.reverse().flat() call with a
copied-array reverse before flattening.
- Around line 115-127: Extract the duplicated bounded-concurrency worker-loop
logic into a shared mapWithConcurrency(items, limit, fn) helper. In
src/core/endorsement-chain/fetchLogsChunked.ts lines 115-127, update
scanLogsParallelFixed to map over windows through the helper; in
src/core/endorsement-chain/retrieveEndorsementChain.ts lines 19-35, update the
corresponding block in the endorsement-chain retrieval flow to map over
uniqueBlockNumbers through the same helper, preserving result ordering and the
existing concurrency limits.
In `@src/core/endorsement-chain/helpers.ts`:
- Around line 93-105: Make TYPE_PRECEDENCE exhaustively compiler-checked against
every TransferEventType member, using a typed mapped structure or equivalent
compile-time enforcement so newly added members fail npm run type-check until
ranked. Preserve the existing precedence ordering, ensure
identifyEventTypeFromLogs still ranks the present types correctly, and retain
the TRANSFER_OWNERS combination check; run npm run type-check and npm run lint
afterward.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 82e24ca7-4a8a-4d59-bed1-458d7b371625
📒 Files selected for processing (7)
README.mdsrc/core/endorsement-chain/fetchEscrowTransfer.tssrc/core/endorsement-chain/fetchLogsChunked.tssrc/core/endorsement-chain/helpers.tssrc/core/endorsement-chain/obligation.tssrc/core/endorsement-chain/retrieveEndorsementChain.tssrc/core/endorsement-chain/useEndorsementChain.ts
…r escrow transfers
…urate log retention
…size and error handling
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/endorsement-chain/fetchLogsChunked.ts`:
- Around line 173-250: Reduce cognitive complexity in
src/core/endorsement-chain/fetchLogsChunked.ts:173-250 by extracting window
construction, request-budget validation, and batch result processing from
scanLogsBackwardParallel into focused helpers while preserving its retry,
truncation, and mint-detection behavior. Also refactor
src/core/endorsement-chain/fetchLogsChunked.ts:264-332 by extracting the
paid-tier iteration and Free-tier handoff from scanLogsBackward into focused
helpers; both sites require direct changes, and each top-level scan function
must remain at or below the SonarCloud complexity threshold.
- Around line 190-225: Update the chunk-fetch loop around getLogsRange so each
RPC attempt is counted before invocation, including failed and retried requests,
while preserving FREE_TIER_MAX_REQUESTS enforcement. Replace immediate
Promise.all rejection handling with batch settlement that waits for every active
request before retrying or throwing, preventing overlapping batches beyond
FREE_TIER_CONCURRENCY. Enforce FREE_TIER_MAX_DURATION_MS with a deadline applied
to the active batch so slow or hanging calls cannot continue past the scan
budget.
In `@src/core/endorsement-chain/retrieveEndorsementChain.ts`:
- Around line 21-27: Bound concurrency in the timestamp-fetching logic within
retrieveEndorsementChain by processing uniqueBlockNumbers through a fixed-size
limit or bounded batches instead of launching every fetchEventTime call via one
unrestricted Promise.all. Preserve the existing block-number-to-timestamp
pairing and timestampByBlock reconstruction after all requests complete.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 97ddaa88-6c7d-4b02-901b-9dfa8e1f6d79
📒 Files selected for processing (6)
README.mdsrc/constants.tssrc/core/endorsement-chain/fetchEscrowTransfer.tssrc/core/endorsement-chain/fetchLogsChunked.tssrc/core/endorsement-chain/helpers.tssrc/core/endorsement-chain/retrieveEndorsementChain.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- src/core/endorsement-chain/fetchEscrowTransfer.ts
Comment on lines
+173
to
+250
| const scanLogsBackwardParallel = async ( | ||
| provider: Provider | ethersV6.Provider, | ||
| address: string, | ||
| fromBlock: number, | ||
| toBlockFloor: number, | ||
| chunkSize: number, | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| isMintLog?: (log: any) => boolean, | ||
| ): Promise<ScanLogsBackwardResult> => { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const chunkGroups: any[][] = []; | ||
| let cursor = fromBlock; | ||
| let windowSize = Math.max(Math.min(chunkSize, FREE_TIER_MAX_CHUNK_SIZE), MIN_CHUNK_SIZE); | ||
| let requestsUsed = 0; | ||
| const startedAt = Date.now(); | ||
| let foundMint = false; | ||
|
|
||
| while (cursor >= toBlockFloor) { | ||
| if (Date.now() - startedAt > FREE_TIER_MAX_DURATION_MS) { | ||
| throw new Error( | ||
| `Infura Free-tier scan time budget exhausted after ${FREE_TIER_MAX_DURATION_MS}ms`, | ||
| ); | ||
| } | ||
|
|
||
| const windows: Array<{ start: number; end: number }> = []; | ||
| let winCursor = cursor; | ||
| for (let i = 0; i < FREE_TIER_CONCURRENCY && winCursor >= toBlockFloor; i++) { | ||
| const start = Math.max(winCursor - windowSize + 1, toBlockFloor); | ||
| windows.push({ start, end: winCursor }); | ||
| if (start <= toBlockFloor) break; | ||
| winCursor = start - 1; | ||
| } | ||
|
|
||
| if (requestsUsed + windows.length > FREE_TIER_MAX_REQUESTS) { | ||
| throw new Error( | ||
| `Infura Free-tier scan request budget exhausted (${FREE_TIER_MAX_REQUESTS} eth_getLogs calls)`, | ||
| ); | ||
| } | ||
|
|
||
| let results: Awaited<ReturnType<typeof getLogsRange>>[]; | ||
| try { | ||
| results = await Promise.all( | ||
| windows.map(({ start, end }) => getLogsRange(provider, address, start, end)), | ||
| ); | ||
| requestsUsed += windows.length; | ||
| } catch (err) { | ||
| const message = errorMessage(err); | ||
| if (RANGE_TOO_LARGE_ERROR_RE.test(message) && windowSize > MIN_CHUNK_SIZE) { | ||
| // Keep already-collected chunkGroups; only shrink and retry this batch. | ||
| windowSize = Math.max(Math.floor(windowSize / 4), MIN_CHUNK_SIZE); | ||
| continue; | ||
| } | ||
| throw err; | ||
| } | ||
|
|
||
| for (const chunkLogs of results) { | ||
| if (pushMintSliceIfFound(chunkLogs, isMintLog, chunkGroups)) { | ||
| foundMint = true; | ||
| break; | ||
| } | ||
| chunkGroups.push(chunkLogs); | ||
| } | ||
|
|
||
| if (foundMint) { | ||
| return { logs: flattenOldestFirst(chunkGroups), foundMint: true, truncated: false }; | ||
| } | ||
|
|
||
| const oldest = windows[windows.length - 1]; | ||
| if (oldest.start <= toBlockFloor) break; | ||
| cursor = oldest.start - 1; | ||
| } | ||
|
|
||
| return { | ||
| logs: flattenOldestFirst(chunkGroups), | ||
| foundMint: false, | ||
| truncated: Boolean(isMintLog), | ||
| }; | ||
| }; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Refactor the scan orchestration to satisfy the SonarCloud complexity gate.
SonarCloud reports cognitive complexity of 26 for scanLogsBackwardParallel and 17 for scanLogsBackward. Both exceed the allowed value of 15.
src/core/endorsement-chain/fetchLogsChunked.ts#L173-L250: Extract window construction, budget validation, and batch processing into focused helpers.src/core/endorsement-chain/fetchLogsChunked.ts#L264-L332: Extract the paid-tier iteration and Free-tier handoff into focused helpers.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 181-181: Refactor this function to reduce its Cognitive Complexity from 26 to the 15 allowed.
[warning] 240-240: Prefer .at(…) over [….length - index].
📍 Affects 1 file
src/core/endorsement-chain/fetchLogsChunked.ts#L173-L250(this comment)src/core/endorsement-chain/fetchLogsChunked.ts#L264-L332
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/endorsement-chain/fetchLogsChunked.ts` around lines 173 - 250,
Reduce cognitive complexity in
src/core/endorsement-chain/fetchLogsChunked.ts:173-250 by extracting window
construction, request-budget validation, and batch result processing from
scanLogsBackwardParallel into focused helpers while preserving its retry,
truncation, and mint-detection behavior. Also refactor
src/core/endorsement-chain/fetchLogsChunked.ts:264-332 by extracting the
paid-tier iteration and Free-tier handoff from scanLogsBackward into focused
helpers; both sites require direct changes, and each top-level scan function
must remain at or below the SonarCloud complexity threshold.
Source: Linters/SAST tools
Comment on lines
+190
to
+225
| while (cursor >= toBlockFloor) { | ||
| if (Date.now() - startedAt > FREE_TIER_MAX_DURATION_MS) { | ||
| throw new Error( | ||
| `Infura Free-tier scan time budget exhausted after ${FREE_TIER_MAX_DURATION_MS}ms`, | ||
| ); | ||
| } | ||
|
|
||
| const windows: Array<{ start: number; end: number }> = []; | ||
| let winCursor = cursor; | ||
| for (let i = 0; i < FREE_TIER_CONCURRENCY && winCursor >= toBlockFloor; i++) { | ||
| const start = Math.max(winCursor - windowSize + 1, toBlockFloor); | ||
| windows.push({ start, end: winCursor }); | ||
| if (start <= toBlockFloor) break; | ||
| winCursor = start - 1; | ||
| } | ||
|
|
||
| if (requestsUsed + windows.length > FREE_TIER_MAX_REQUESTS) { | ||
| throw new Error( | ||
| `Infura Free-tier scan request budget exhausted (${FREE_TIER_MAX_REQUESTS} eth_getLogs calls)`, | ||
| ); | ||
| } | ||
|
|
||
| let results: Awaited<ReturnType<typeof getLogsRange>>[]; | ||
| try { | ||
| results = await Promise.all( | ||
| windows.map(({ start, end }) => getLogsRange(provider, address, start, end)), | ||
| ); | ||
| requestsUsed += windows.length; | ||
| } catch (err) { | ||
| const message = errorMessage(err); | ||
| if (RANGE_TOO_LARGE_ERROR_RE.test(message) && windowSize > MIN_CHUNK_SIZE) { | ||
| // Keep already-collected chunkGroups; only shrink and retry this batch. | ||
| windowSize = Math.max(Math.floor(windowSize / 4), MIN_CHUNK_SIZE); | ||
| continue; | ||
| } | ||
| throw err; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Enforce the request and duration limits for actual RPC calls.
requestsUsed increases only after a full batch succeeds. Retries and failed range batches do not count against FREE_TIER_MAX_REQUESTS.
If one request rejects, Promise.all rejects immediately. The remaining requests continue in the background, and the next loop starts another batch. This can exceed FREE_TIER_CONCURRENCY.
The duration check runs before Promise.all. A slow or hanging RPC batch can run past FREE_TIER_MAX_DURATION_MS.
Count each request before provider.getLogs. Apply a deadline to the active batch. Wait for the batch to settle before retrying a smaller range.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/endorsement-chain/fetchLogsChunked.ts` around lines 190 - 225,
Update the chunk-fetch loop around getLogsRange so each RPC attempt is counted
before invocation, including failed and retried requests, while preserving
FREE_TIER_MAX_REQUESTS enforcement. Replace immediate Promise.all rejection
handling with batch settlement that waits for every active request before
retrying or throwing, preventing overlapping batches beyond
FREE_TIER_CONCURRENCY. Enforce FREE_TIER_MAX_DURATION_MS with a deadline applied
to the active batch so slow or hanging calls cannot continue past the scan
budget.
Comment on lines
+21
to
+27
| const timestampByBlock = new Map( | ||
| await Promise.all( | ||
| uniqueBlockNumbers.map( | ||
| async (blockNumber) => [blockNumber, await fetchEventTime(blockNumber, provider)] as const, | ||
| ), | ||
| ), | ||
| ); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound concurrent block timestamp requests.
Promise.all starts a getBlock request for every unique block at once. A large endorsement chain can create thousands of concurrent RPC requests.
Process unique block numbers with a fixed concurrency limit or bounded batches. Preserve the timestampByBlock reconstruction after the requests complete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/endorsement-chain/retrieveEndorsementChain.ts` around lines 21 - 27,
Bound concurrency in the timestamp-fetching logic within
retrieveEndorsementChain by processing uniqueBlockNumbers through a fixed-size
limit or bounded batches instead of launching every fetchEventTime call via one
unrestricted Promise.all. Preserve the existing block-number-to-timestamp
pairing and timestampByBlock reconstruction after all requests complete.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What is the background of this pull request?
Changes
Issues
What are the related issues or stories?
Summary by CodeRabbit
Improvements
Documentation