Skip to content

Candidate for 6.0.6 release - #2473

Merged
kushti merged 46 commits into
masterfrom
v6.0.6
Sep 21, 2026
Merged

kushti merged 46 commits into
masterfrom
v6.0.6

Conversation

kushti and others added 21 commits August 18, 2026 21:41
…pends still validate

ErgoNodeViewHolder's Declined branch discarded the pool returned by
ErgoMemPool.process, so the prefilter's rejected-id cache never reached a
running node. Install the returned pool when it differs by reference, which
is a no-op for every pre-existing declining path.

Also: single-pass input resolution at the prefilter anchor, a clearer
rejection message, and an end-to-end case covering a conformant owner spend
with checkReemissionRules enabled.
Invariant (h) drives the token-preserving rejection through a real
ErgoNodeViewHolder, using a test-only subclass whose InjectState message
reuses the existing protected updateNodeView to install a synthetic
token-bearing state. It asserts the id is cached both in the published
ChangedMempool reader and in a freshly queried GetDataFromCurrentView. A
companion control pins the reference-inequality guard as a no-op on a
declining path that returns the same pool. Removing only the guarded
install makes (h) fail while the control still passes.
Send InjectState and the transaction through the same probe to avoid an
Akka per-pair FIFO race that could let the tx overtake the state injection
onto the missing-UTXO path. Assert the intended decline reason in both
cases, and assert minimalFeeAmount == 0 as an explicit precondition, since
the fee gate precedes the prefilter. Behaviour unchanged; all four mutation
checks (predicate-false, delete-install, unconditional-install, d3 burn)
confirmed against these tests.
Test that proofsForTransactions() does not modify the UTXO set tree along a growing chain
…overy

Preserve prepared UTXO snapshot state on restart
Replay the three reviewed commits from PR #2512 on the release branch. Preserve the seven approved file contents and the checkpoint, startup and synchronization observation tests without importing unrelated master changes.
Block assembly is bounded by maxBlockSize and maxBlockCost, both enforced
by CandidateGenerator.correctLimits. The mempool API returned `size` but
not `cost`, so external block builders could enforce only the size limit
and had to recompute or guess per-transaction cost, even though the node
already measured it on mempool admission.

Add a `cost` field to the mempool transaction JSON, sourced from
UnconfirmedTransaction.lastCost. It is null when the node never measured
the cost: digest state, or a transaction returned to the pool by a
rollback and not yet re-validated by CleanupWorker.

Affected endpoints:
  GET  /transactions/unconfirmed
  GET  /transactions/unconfirmed/byTransactionId/{txId}
  POST /transactions/unconfirmed/byErgoTree

byTransactionId used ErgoMemPoolReader.modifierById, which returns
Option[ErgoTransaction] and drops the cost. Add unconfirmedById to the
reader, returning the UnconfirmedTransaction wrapper; modifierById is now
defined in terms of it. byErgoTree kept the wrapper out of its collect
blocks, so those become filters. Deduplication is unchanged:
UnconfirmedTransaction.equals is defined on the transaction id.

The digest-state branch of byErgoTree previously serialized a bare
ErgoTransaction, which did not match the ErgoTransactionWithInputBoxes
schema the spec declares for that response. It now goes through the same
JSON builder, so `cost` is never silently absent.

The field is additive and optional, so clients ignoring unknown keys are
unaffected. Confirmed-transaction schemas are untouched, as lastCost only
exists for unconfirmed transactions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@odiseusme odiseusme left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the candidate at 2046cfc. Two blockers, both in candidate generation; I found no additional blockers in the production diff, subject to the assessment limits below.

Blocker 1 - the candidate does not compile. CandidateGenerator.scala:762:93: EliminateTransactions(retryEliminate) where retryEliminate is already an EliminateTransactions (found EliminateTransactions, required Seq[ModifierId]). Reproduced locally with sbt compile. The retry-success path at :751 passes it correctly.

Blocker 2 - the change contradicts its own comment. The emission-only fallback is reached only when both proof attempts failed, so the second collection's classifications were never validated against a state that produced a usable candidate. At f002151d2 this branch read EliminateTransactions(Seq.empty) under the comment "Both collections produced failed proofs; their rejections may be stale." At the head the comment is unchanged but the code attempts to propagate retryEliminate; removing only the extra wrapper would retain potentially stale rejections. I'd keep the empty list on the second-failure fallback, and keep the retry's classifications on retry success, as in #2559. @a-shannon reports a rollback regression failing here once the type error is fixed locally; that matches, though I have not run it myself.

Non-blocking, from reading the rest of the diff:

  1. Mempool JSON (TransactionsApiRoute.scala:133,321): cost is new, and the digest-state ErgoTree query now serializes with resolved inputs so digest mode matches the declared ErgoTransactionWithInputBoxes schema. Both are visible to API consumers and deserve a release note.
  2. Wallet change addresses (ErgoWalletService.scala:402, ErgoWalletState.scala:85-98): a change address must now be tracked and signable, and updates require an unlocked wallet. Previously persisted external change addresses stop being used and change goes to the root address. That is a behaviour change for existing operators and needs a release note beyond the OpenAPI text.
  3. NiPoPoW (ErgoNodeViewHolder.scala:320-330): applying a proof now calls setHeadersChainSynced() when utxoBootstrap is on. This starts snapshot discovery earlier; it does not itself establish a change in the proof-validation trust boundary.
  4. Inbound accounting (NetworkController.scala:185-198,245-267) looks correct to me: the pending slot covers only Connected to ConfirmConnection, that decision is local, dead sockets are caught by Terminated, and post-confirmation handlers are bounded by handshakeTimeout.

Not assessed: crash-consistency ordering between snapshot state and history writes, digest replay from the recovered checkpoint, and whether cached or in-flight body sections remain correctly gated during snapshot bootstrap.

@odiseusme

Copy link
Copy Markdown
Contributor

Both blockers are resolved at ac6f9d943. The emission-only fallback reads EliminateTransactions(Seq.empty) again, which fixes the compile error and the stale-rejection propagation together, and the code now matches its comment.

Local on ac6f9d943: testOnly *CandidateRetryReorgSpec *CandidateGeneratorSpec, 28/28 passed, including the second-collection rollback regression.

The non-blocking items in my earlier comment still stand, in particular release notes for the mempool JSON cost field, the digest-mode response shape, and the wallet change-address behaviour change.

@kushti
kushti merged commit 23aabea into master Sep 21, 2026
7 of 8 checks passed
@kushti
kushti deleted the v6.0.6 branch September 21, 2026 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants