Conversation
Openapi-ai.yaml removed
…-ids Prevent duplicate IDs in OrderedTxPool
Fix indentation in openapi.yaml for CommitmentWithSecret
Block candidate generation improvements
Preserve extra index consistency across chain switches
PoPowHeader's encoder emits interlinksProof and its decoder requires it, but the PopowHeader schema listed only header and interlinks, so the documented shape did not match the one the node serves. Add the field, and a BatchMerkleProof schema matching what batchMerkleProofEncoder produces. Noticed while reading #1384.
…ests Close state storage in the UTXO and digest state property tests
…-proof-openapi Document interlinksProof in the PopowHeader OpenAPI schema
jozanek
left a comment
jozanek
left a comment
There was a problem hiding this comment.
Reviewed the full 6.0.4 candidate diff. No protocol/consensus concerns: no serializer or validation-rule changes; the new txIds on LocalBlockApplied/RemoteBlockApplied is internal event-stream only, never on the wire; appVersion/openapi version stamps are consistent.
Findings: 1 MAJOR, 3 MINOR, 2 NIT — inline. The same findings are posted on #2416, which builds on this branch, so fixing them once on v6.0.4 and syncing forward resolves both.
| context.become(receive.orElse(loaded(newState))) | ||
| self ! Index() | ||
| } else { | ||
| log.info("Deferring catch-up because the next header does not extend the indexed tip") |
There was a problem hiding this comment.
MAJOR After "Deferring catch-up" no retry is scheduled, and while caughtUp = false the actor has no FullBlockApplied handler — only a Rollback event can resume indexing. If a headers-only fork briefly becomes the best header chain and then loses without the full-block chain ever switching (the losing side of a miner race), no Rollback is published and the indexer stays stalled until restart, dropping all further FullBlockApplied events.
Suggestion: case _: FullBlockApplied if !state.caughtUp && !state.rollbackInProgress => self ! Index(), or re-schedule Index() with a delay instead of only logging.
| if (modCount >= saveLimit) saveProgress(newState) | ||
| context.become(receive.orElse(loaded(newState))) | ||
| self ! Index() | ||
| val nextHeaderOpt = history.bestHeaderAtHeight(state.indexedHeight + 1) |
There was a problem hiding this comment.
MINOR This adds a bestHeaderAtHeight(h + 1) read per block during catch-up, and index() re-reads bestHeaderIdAtHeight(height) at line 381 (headerOpt is None on this path) — two redundant storage reads per block on the reindex hot path. Passing the already-fetched nextHeaderOpt into index() would reuse a single read.
| )) | ||
| }) | ||
| } ~ | ||
| (path(".well-known" / "ai-plugin.json") & get) { |
There was a problem hiding this comment.
MINOR Removing /openapi.yaml and /.well-known/ai-plugin.json is clearly intentional (ChatGPT-plugin retirement), but it is a breaking removal of public endpoints — anything still fetching them gets a 404 after upgrade. Worth an explicit line in the 6.0.4 release notes.
| // `lazy` so the container is only started when a test actually touches `node`. | ||
| // The single test below is currently `ignore`d (the openapi-checker image is gone), | ||
| // so without `lazy` we would start and tear down a node for nothing. | ||
| lazy val node: Node = docker.startDevNetNode(offlineGeneratingPeer).get |
There was a problem hiding this comment.
MINOR The spec's only test remains ignored (the openapi-checker image is gone), so this suite passes CI while providing zero signal — the new comment documents the situation but keeps the dead spec. Consider deleting the spec or reviving the check with a maintained validator image.
| private val mempoolCapacity = settings.nodeSettings.mempoolCapacity | ||
|
|
||
| private def withoutTransaction(id: ModifierId): TreeMap[WeightedTxId, UnconfirmedTransaction] = { | ||
| // Keep healthy mutations logarithmic; scan by ID only after cardinality diverges. |
There was a problem hiding this comment.
NIT The size-equality heuristic takes the fast path on compensating corruption — one duplicate key plus one orphaned entry leaves the sizes equal, so a duplicate would survive withoutTransaction. Fine as best-effort self-healing, but worth extending the comment to note that limitation.
| done.await() | ||
| awaitCondition(done) | ||
| indexer ! GenerateBetterChainTip() | ||
| lock.lock() |
There was a problem hiding this comment.
NIT awaitCondition fixes the lock discipline (the old pattern never unlocks after await()), but this test still uses bare lock.lock(); created.await() in two places. Worth finishing the migration to awaitCondition(created) here too.
Fix for lastHeaders in upcoming state context , miner not stuck on divergence in block pre-validation and validation
Caught-up fix
6 participants
Uh oh!
There was an error while loading. Please reload this page.