Repository navigation
Conversation
…n every synced block A syncing node used to apply whatever bodies and statuses the serving peer sent. It now requires the served transaction set to equal the block's ordered hashes, re-validates every body and signature at the block's own height (with the previous height accepted on a fork's activation block, since the forger admitted those transactions under the earlier rule), derives CONFIRMED and FAILED by applying the transactions locally, and compares the native table hashes the forger committed before applying the block. Any disagreement undoes the applied edits, drops the block's assignments and rejects the block; the serving peer is cooled down and sync moves to the next one. verifyBlock runs for every block after genesis regardless of the nonce-enforcement fork. Transactions restored from the database are rebuilt from the stored signed content rather than from columns, which never hashed to the committed hash (key order, numeric fields, dropped keys). The TLSN table is left out of the hash comparison because its stored creation time is per node. Balances and nonces are not covered by any signed commitment and a peer-attested reference block cannot be checked. Refs kynesyslabs/mainnet-launch#21
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
 [Critical impact] The changes reach block acceptance and transaction-driven state across syncing nodes. The PR still needs the final deep-window block covered before it is safe to merge.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (isActivationBlock(block.number)) { | ||
| const retry = userTxs.filter( | ||
| tx => verdicts.get(tx.hash)?.valid !== true, | ||
| ) | ||
| if (retry.length > 0) { | ||
| for (const result of await validateAtHeight( | ||
| retry, | ||
| block.number - 1, |
There was a problem hiding this comment.
The previous-rule retry only covers the activation block, but the forger can include older transactions later. A pre-fork transaction with a high nonce stays in the mempool while its predecessor is missing; TX_NONCE_INVALID_HIGH is explicitly kept for later rounds. Once the predecessor arrives, the forger can include that transaction after activation without checking its signature again.
This check then rejects the honest block under the new rules, and changing peers cannot help. Match the forger's retained-transaction rules rather than limiting compatibility to one block.
Knowledge Base Used: Blockchain state and transaction execution
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/libs/blockchain/routines/syncVerification.ts
Line: 92-99
Comment:
**Retained transactions stop sync**
The previous-rule retry only covers the activation block, but the forger can include older transactions later. A pre-fork transaction with a high nonce stays in the mempool while its predecessor is missing; `TX_NONCE_INVALID_HIGH` is explicitly kept for later rounds. Once the predecessor arrives, the forger can include that transaction after activation without checking its signature again.
This check then rejects the honest block under the new rules, and changing peers cannot help. Match the forger's retained-transaction rules rather than limiting compatibility to one block.
**Knowledge Base Used:** [Blockchain state and transaction execution](https://app.greptile.com/kynesyslabs/-/custom-context/knowledge-base/kynesyslabs/node/-/docs/blockchain-state-and-transactions.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
Agreed. The forger admits a transaction under the rules of the tip it saw and never revalidates its mempool, so a kept nonce-high transaction can land after activation. Sync now also accepts the rules in force before each fork for as long as such a transaction can still be included, which is the deep reference-block window (5 x room); after that the forger drops it. This only widens which genuine signature domain is accepted; hash binding and signature verification are unchanged. Tests cover the window edges and forks activating at different heights.
…fork transactions for the retention window Assignments are written inside the database transaction that saves the GCR edits, so a failed write leaves neither and undo can rely on them. Block transaction verification also accepts the rules in force before a fork for as long as a transaction admitted before it can still reach a block.
A syncing node re-verifies what it applies. Refs kynesyslabs/mainnet-launch#21.
Before. A node syncing from a peer applied whatever transaction bodies and statuses the peer served: the block hash covers only the ordered hashes, bodies were never re-hashed or signature-checked, the peer's CONFIRMED/FAILED decided what was applied,
native_tables_hasheswas never compared, andverifyBlockran only while the nonce-enforcement fork was active.Now. For every synced block: the served set must equal
ordered_transactions; every body is re-hashed and its signature checked at the block's own height (on a fork's activation block the previous height's rule is also accepted, because the forger admitted those transactions under it); the node's batch relay transactions are verified by the form they are signed in; CONFIRMED and FAILED are derived by applying the transactions locally and must match the served statuses; the native table hashes the forger committed are compared before the block is applied. Any disagreement undoes the applied edits, drops the block's assignments and rejects the block; the serving peer is cooled down and sync rotates.verifyBlockruns for every block after genesis.Also fixed. Transactions restored from the database are rebuilt from the stored signed content: the column-based rebuild never hashed to the committed hash (key order, numeric fields, dropped keys), so the new check would have rejected every honest block.
Limits, stated plainly. The TLSN table is excluded from the hash comparison because its stored creation time is per node (that also breaks validator agreement on
native_tables_hashesonce a TLSN row exists and needs a consensus-wide fix). Balances and nonces are not covered by any signed commitment, and a peer-attested reference block cannot be checked, so an expired transaction served as confirmed with an in-window reference block is still applied; closing that needs a fork that commits block attributes or an applied-set hash. L2PS pending-proof effects never reach sync-only nodes (existing divergence, DEM-816).Tests. syncVerification (28), Sync.verification (15, real syncBlock and fastSync with mocked chain and apply, real signing and validation), transaction.roundtrip (2); existing verifyBlock and txCoherenceFork jest files green. No devnet run.