Conversation
📝 WalkthroughWalkthroughVersion bumped in Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c40dc16b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async init( | ||
| reindex: (targetHeight: Header) => Promise<void>, | ||
| supportsFinalisation?: boolean, | ||
| ): Promise<Header | undefined> { |
There was a problem hiding this comment.
Restore finalized-mode flag in init
Dropping the supportsFinalisation argument from init means this service no longer tracks whether the chain supports the finalized tag, so finalized-capable networks now always execute the non-finalized fallback logic in the overrides. In that mode, fork recovery can fall through to POI (or blockForkReindex when POI is disabled) instead of using the base finalized-chain path, which can trigger unnecessary deep rewinds and rollback indexed data on deterministic-finality chains after startup/hash-mismatch scenarios.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/node/src/indexer/unfinalizedBlocks.service.ts (1)
78-78: Clarify the fallback fork-detection comment.Line 78 says fork detection “won’t find fork,” but Lines 79–86 still detect and return a forked header as a defensive fallback. Please reword for accuracy.
Suggested comment update
- // this now won't find fork as such cases has been covered when registerUnfinalizedBlock() is called + // Most fork cases are handled in registerUnfinalizedBlock(); this remains as a defensive fallback check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/node/src/indexer/unfinalizedBlocks.service.ts` at line 78, Update the misleading comment above the fork-check in UnfinalizedBlocksService to state that primary fork detection is performed in registerUnfinalizedBlock(), but this code path (the check that can return a forked header) remains as a defensive fallback and may still detect and return a fork; reference the registerUnfinalizedBlock() function name and the local fork-detection block (the conditional that returns a forked header) so the intent is clear that both mechanisms exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/node/src/indexer/unfinalizedBlocks.service.ts`:
- Line 78: Update the misleading comment above the fork-check in
UnfinalizedBlocksService to state that primary fork detection is performed in
registerUnfinalizedBlock(), but this code path (the check that can return a
forked header) remains as a defensive fallback and may still detect and return a
fork; reference the registerUnfinalizedBlock() function name and the local
fork-detection block (the conditional that returns a forked header) so the
intent is clear that both mechanisms exist.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
packages/node/package.jsonpackages/node/src/indexer/unfinalizedBlocks.service.ts
Coverage reportCaution Test run failed
Test suite run failedFailed tests: 21/121. Failed suites: 6/16.Report generated by 🧪jest coverage report action from ba46513 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/node/package.json`:
- Line 29: The PR title and description claim bumping "@subql/node-core" to
19.1.1-0 but package.json currently shows "@subql/node-core": "^19.2.0";
reconcile by either changing the dependency string in package.json to the
intended "^19.1.1-0" (update the entry for "@subql/node-core") or update the PR
title/description to accurately state the bump to "^19.2.0" so they match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69a0a057-2395-4c61-9bc2-e9b6f8dbfc27
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
packages/node/package.json
| "@subql/common": "^5.8.2", | ||
| "@subql/common-ethereum": "workspace:*", | ||
| "@subql/node-core": "^19.1.0", | ||
| "@subql/node-core": "^19.2.0", |
There was a problem hiding this comment.
Version inconsistency between PR title and actual change.
The PR title states "bump node-core to 19.1.1-0" but the actual dependency change is to ^19.2.0. Please reconcile the PR title/description with the actual version being used to avoid confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/node/package.json` at line 29, The PR title and description claim
bumping "@subql/node-core" to 19.1.1-0 but package.json currently shows
"@subql/node-core": "^19.2.0"; reconcile by either changing the dependency
string in package.json to the intended "^19.1.1-0" (update the entry for
"@subql/node-core") or update the PR title/description to accurately state the
bump to "^19.2.0" so they match.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
Checklist
Summary by CodeRabbit
Chores
Bug Fixes / Internal Improvements