Block announcements: bind difficulty to the known parent one block below; drop unbindable announcements

[cafebedouin]

Prepared with Claude Code (Anthropic): Claude Opus 4.8; the issue was originally identified with Claude Fable 5.1 and Claude Opus 5.

When the parent was unknown, an input-block or ordering-block announcement's difficulty was not checked at all — only its proof-of-work, against its own declared nBits — so a peer could declare a low difficulty and have the announcement processed (the InputBlocksProcessor.scala:862 todo). This binds the expected difficulty to the known parent one block below and drops any announcement that cannot be bound.

• A known parent one block below supplies the expected difficulty via requiredDifficultyAfter, which also handles an off-best-chain parent. Restricting processing to best-chain parents is a separate policy, left out of this change.
• At genesis height the expected difficulty is the configured initial difficulty, guarded on an empty header chain and parentId == GenesisParentId.
• Anything that cannot be bound — unknown parent, parent not exactly one block below, or a genesis-height announcement that arrives once headers exist or whose parent is not the genesis parent — is dropped; a real parent arrives via normal header sync.

The sender-only header request for an unknown parent is removed from this PR; it will return as a follow-up once request-attempt identity is available, so it can carry a proper gate then.

Tests

CI cannot build this branch (it stops at the sigma-state snapshot dependency), so this was verified locally:

sbt "testOnly org.ergoplatform.network.InputBlockParentBindingSpec org.ergoplatform.network.OrderingBlockAnnouncementParentCheckSpec"

24 properties pass (12 each). PoW is validated with the real Autolykos scheme (the default test config accepts any header). The two "accepted on replay" cases are retained. The genesis guard was mutation-checked: removing the parentId == GenesisParentId conjunct makes the non-genesis-parent test fail.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F9aGNTUgEQRrSKKBLTCqcY

[a-shannon]

20 September follow-up, head b164fc06a363f6af48e6d01c69b83a023acff2bd: source review, including a separate reviewer, confirms that both benign-drop paths now clear the exact pending announcement only when the responding peer matches its current supplier. setUnknown cancels that request's timer and removes the pending entry. Our four test additions are included unchanged. The author reports 28/28 passing and 26 passing/2 failing when only the production correction is reverted; I have not rerun this revision locally.

One focused regression test is still needed for the new supplier guard: request the announcement from peer A, deliver the unbindable announcement from peer B, then assert that A's RequestedInfo and timer remain unchanged and uncancelled. The current tests only deliver from A, so deleting the peer-equality guard would not make them fail. Cover both announcement paths using the existing fixtures.

The immediate receipt fix matches the intended change at source level. It does not establish generation-safe retry handling: the generic already-queued CheckDelivery versus replacement-request race remains under #2528, outside this correction. Drop-vs-waitlist remains a maintainer policy decision. Current CI run 35460769061 has four failures resolving the Sigma snapshot before tests and four cancellations. No new runtime or merge-readiness claim is made here.

Historical reproduction, 18 September, head 642b2370c2759a24c5a28d02224a9ff3b16c27dc: the narrowed change removes the new pre-PoW parent requests and their expiry machinery. Known off-best-chain parents can supply difficulty again; both genesis paths now require an empty header chain and the genesis parent ID. This addresses the mechanism behind the earlier header-budget objection. The pre-existing input height+2 request path is unchanged.

One remaining receive-lifecycle issue, reproduced for both announcement types: through the normal serialized Inv -> RequestModifier -> announcement route, a requested announcement with an unknown parent is received and benignly dropped, but remains Requested with its timer active. The new returns occur before setReceivedIfRequested. A subsequent CheckDelivery then emits NonDeliveryPenalty against the peer that delivered the response. Input blocks also trigger a redundant retry.

Two focused actor tests fail on unchanged production code with (Requested, false, true) for (status after delivery, original timer cancelled, penalty after delivery), instead of (Unknown, true, false). They first verify the outgoing request and the pending entry, deliver an announcement that passes its self-declared PoW checks, and trigger the delivery check deterministically with a long scheduler timeout. The failure concerns the announcement's own pending request, not a newly requested parent. These auxiliary types are excluded from headersToDownload and modifiersToDownload; this is not another shared download-capacity claim.

Please cancel/clear the matching pending request on these benign-drop exits, preserving the ability to request or replay the announcement later. Check the current supplier before clearing it so an unsolicited response from another peer cannot erase that supplier's request. Keeping the dropped announcement permanently Received would suppress later requests. The author can make this correction in this PR; no parallel implementation is needed.

Validation: the unmodified head passes 24/24 targeted tests locally. I added the two headers-present/no-full-block genesis cases mentioned in the author response; those plus the existing cases pass 26/26. The two separate receipt regressions above then fail 2/2. Java 8u504, Scala 2.12.20, sbt 1.11.1, cached declared Sigma snapshot 6.0.5-22-368a860b-SNAPSHOT. A separate reviewer checked the changed source, affected delivery path and the four added tests. These are receiver/history checks; they do not establish UTXO application, automatic replay, network propagation or release readiness.

CI run 35363337844 has four failed jobs and four cancelled jobs. The failed jobs stop while resolving the Sigma snapshot, before tests. Maintainer agreement on dropping unbindable announcements, the receipt correction and executable hosted CI remain open.

Test-only patch against 642b237: two passing genesis cases and two failing receipt regressions

diff --git a/src/test/scala/org/ergoplatform/network/InputBlockParentBindingSpec.scala b/src/test/scala/org/ergoplatform/network/InputBlockParentBindingSpec.scala
index 795fdd8b5..48a0297a0 100644
--- a/src/test/scala/org/ergoplatform/network/InputBlockParentBindingSpec.scala
+++ b/src/test/scala/org/ergoplatform/network/InputBlockParentBindingSpec.scala
@@ -5,10 +5,11 @@ import akka.testkit.{TestActorRef, TestProbe}
 import org.ergoplatform.AutolykosSolution
 import org.ergoplatform.mining.{AutolykosPowScheme, InputBlockFields}
 import org.ergoplatform.mining.difficulty.DifficultySerializer
-import org.ergoplatform.modifiers.ErgoFullBlock
+import org.ergoplatform.modifiers.{ErgoFullBlock, InputBlockTypeId}
 import org.ergoplatform.modifiers.history.header.{Header}
 import org.ergoplatform.network.ErgoNodeViewSynchronizerMessages._
-import org.ergoplatform.network.message.{InvData, RequestModifierSpec}
+import org.ergoplatform.network.message.{InvData, InvSpec, Message, RequestModifierSpec}
+import org.ergoplatform.network.message.inputblocks.InputBlockMessageSpec
 import org.ergoplatform.network.peer.PeerInfo
 import org.ergoplatform.nodeView.history.{ErgoHistory, ErgoHistoryUtils, ErgoSyncInfoMessageSpec}
 import org.ergoplatform.nodeView.mempool.ErgoMemPool
@@ -153,6 +154,39 @@ class InputBlockParentBindingSpec extends AnyPropSpec with Matchers with FileUti
 
   private def penalized(msgs: Seq[Any]): Boolean = msgs.exists(_.isInstanceOf[PenalizePeer])
 
+  property("requested input block with an unknown parent is delivered without a non-delivery penalty") {
+    withFixture(new Fixture(requestTimeout = 30.seconds)) { f =>
+      f.synchronizer ! ChangedState(f.state)
+      f.synchronizer ! ChangedHistory(f.hist)
+      f.synchronizer ! ChangedMempool(f.mempool)
+      networkMessages(f)
+      val unknownParent = bytesToId(Array.fill(32)(0x5a.toByte))
+      val ib = announcement(f, unknownParent, f.hist.fullBlockHeight + 1,
+        DifficultySerializer.encodeCompactBits(1))
+      ib.valid(f.realPowScheme, f.state.stateContext.currentParameters, None) shouldBe true
+      val inv = InvData(InputBlockTypeId.value, Seq(ib.id))
+      f.synchronizer ! Message(InvSpec, Left(InvSpec.toBytes(inv)), Some(f.peer))
+      val requests = networkMessages(f).collect {
+        case SendToNetwork(msg, _) if msg.spec.messageCode == RequestModifierSpec.messageCode =>
+          msg.data.get.asInstanceOf[InvData]
+      }
+      requests should contain(inv)
+      f.deliveryTracker.status(ib.id, InputBlockTypeId.value, Seq.empty) shouldBe ModifiersStatus.Requested
+      val attempt = f.deliveryTracker.getRequestedInfo(InputBlockTypeId.value, ib.id).get
+
+      f.synchronizer ! Message(InputBlockMessageSpec, Left(InputBlockMessageSpec.toBytes(ib)), Some(f.peer))
+      viewHolderGotInputBlock(f) shouldBe false
+      networkMessages(f) shouldBe empty
+      val statusAfterDelivery = f.deliveryTracker.status(ib.id, InputBlockTypeId.value, Seq.empty)
+      val timerCancelledAfterDelivery = attempt.cancellable.isCancelled
+      f.synchronizer ! CheckDelivery(f.peer, InputBlockTypeId.value, ib.id)
+      val penaltyAfterDelivery = penalized(networkMessages(f))
+
+      (statusAfterDelivery, timerCancelledAfterDelivery, penaltyAfterDelivery) shouldBe
+        ((ModifiersStatus.Unknown, true, false))
+    }
+  }
+
   property("input block with unknown parent header is dropped without requesting the header") {
     withFixture { f =>
       val unknownParent = bytesToId(Array.fill(32)(0x5a.toByte))
@@ -341,6 +375,26 @@ class InputBlockParentBindingSpec extends AnyPropSpec with Matchers with FileUti
     }
   }
 
+  property("input block at genesis height is dropped when headers exist but no full block has been applied") {
+    withFixture(new Fixture(applyLocalChain = false)) { f =>
+      f.hist.append(f.chain.head.header).get
+      f.hist.bestHeaderOpt.isDefined shouldBe true
+      f.hist.bestFullBlockIdOpt shouldBe None
+      f.hist.fullBlockHeight shouldBe 0
+      val initial = f.historySettings.chainSettings.initialNBits
+      val ib = announcement(f, Header.GenesisParentId, ErgoHistoryUtils.GenesisHeight, initial)
+      f.hist.contains(ib.header) shouldBe false
+      ib.valid(f.realPowScheme, f.state.stateContext.currentParameters, Some(initial)) shouldBe true
+
+      f.process(ib)
+
+      viewHolderGotInputBlock(f) shouldBe false
+      val msgs = networkMessages(f)
+      penalized(msgs) shouldBe false
+      headerRequests(msgs) shouldBe empty
+    }
+  }
+
   property("input block at genesis height whose parent is not the genesis parent is dropped") {
     withFixture(new Fixture(applyLocalChain = false)) { f =>
       f.hist.bestFullBlockIdOpt shouldBe None
diff --git a/src/test/scala/org/ergoplatform/network/OrderingBlockAnnouncementParentCheckSpec.scala b/src/test/scala/org/ergoplatform/network/OrderingBlockAnnouncementParentCheckSpec.scala
index c1f3f3504..d13b7b2a0 100644
--- a/src/test/scala/org/ergoplatform/network/OrderingBlockAnnouncementParentCheckSpec.scala
+++ b/src/test/scala/org/ergoplatform/network/OrderingBlockAnnouncementParentCheckSpec.scala
@@ -174,6 +174,35 @@ class OrderingBlockAnnouncementParentCheckSpec extends AnyPropSpec with Matchers
     )
   }
 
+  property("requested ordering announcement with an unknown parent is delivered without a non-delivery penalty") {
+    withFixture(new Fixture(requestTimeout = 30.seconds)) { f =>
+      val unknownParent = bytesToId(Array.fill(32)(0x5a.toByte))
+      val oba = announcement(f, unknownParent, f.hist.fullBlockHeight + 1,
+        DifficultySerializer.encodeCompactBits(1))
+      oba.valid(f.realPowScheme, None) shouldBe true
+      val inv = InvData(OrderingBlockAnnouncementTypeId.value, Seq(oba.header.id))
+      f.synchronizer ! Message(InvSpec, Left(InvSpec.toBytes(inv)), Some(f.peer))
+      val requests = f.ncProbe.receiveWhile(max = 1.second, idle = 300.millis) { case m => m }.collect {
+        case SendToNetwork(msg, _) if msg.spec.messageCode == RequestModifierSpec.messageCode =>
+          msg.data.get.asInstanceOf[InvData]
+      }
+      requests should contain(inv)
+      f.deliveryTracker.status(oba.header.id, OrderingBlockAnnouncementTypeId.value, Seq.empty) shouldBe ModifiersStatus.Requested
+      val attempt = f.deliveryTracker.getRequestedInfo(OrderingBlockAnnouncementTypeId.value, oba.header.id).get
+
+      f.send(oba)
+      outcome(f, oba) shouldBe Outcome(stored = false, relayed = false, handedOff = false, penalized = false,
+        headerRequests = Seq.empty)
+      val statusAfterDelivery = f.deliveryTracker.status(oba.header.id, OrderingBlockAnnouncementTypeId.value, Seq.empty)
+      val timerCancelledAfterDelivery = attempt.cancellable.isCancelled
+      f.synchronizer ! CheckDelivery(f.peer, OrderingBlockAnnouncementTypeId.value, oba.header.id)
+      val penaltyAfterDelivery = outcome(f, oba).penalized
+
+      (statusAfterDelivery, timerCancelledAfterDelivery, penaltyAfterDelivery) shouldBe
+        ((ModifiersStatus.Unknown, true, false))
+    }
+  }
+
   property("ordering block announcement with unknown parent header is dropped without requesting the header") {
     withFixture { f =>
       val unknownParent = bytesToId(Array.fill(32)(0x5a.toByte))
@@ -315,6 +344,24 @@ class OrderingBlockAnnouncementParentCheckSpec extends AnyPropSpec with Matchers
     }
   }
 
+  property("ordering block announcement at genesis height is dropped when headers exist but no full block has been applied") {
+    withFixture(new Fixture(applyLocalChain = false)) { f =>
+      f.hist.append(f.chain.head.header).get
+      f.hist.bestHeaderOpt.isDefined shouldBe true
+      f.hist.bestFullBlockIdOpt shouldBe None
+      f.hist.fullBlockHeight shouldBe 0
+      val initial = f.historySettings.chainSettings.initialNBits
+      val oba = announcement(f, Header.GenesisParentId, ErgoHistoryUtils.GenesisHeight, initial)
+      f.hist.contains(oba.header) shouldBe false
+      oba.valid(f.realPowScheme, Some(initial)) shouldBe true
+
+      f.send(oba)
+
+      outcome(f, oba) shouldBe Outcome(stored = false, relayed = false, handedOff = false, penalized = false,
+        headerRequests = Seq.empty)
+    }
+  }
+
   property("ordering block announcement at genesis height whose parent is not the genesis parent is dropped") {
     withFixture(new Fixture(applyLocalChain = false)) { f =>
       f.hist.bestHeaderOpt shouldBe None

[jozanek]

The bug is real — @kushti flagged this exact attack at InputBlocksProcessor.scala:862 back in Aug 2025. No argument on the difficulty binding.

My question is scope. I read three decisions here: (a) the binding (~37 production lines, uncontested), (b) the drop-vs-waitlist policy, (c) the sender-only request lifecycle (~48 lines, a self-described placeholder for #2528). Would you consider landing (a) alone?

On (b): #2552 carried an explicit "Open question" citing the paper's disconnected waitlist at main.tex:366-374; this body replaced it with "the decision to make once". @kushti — drop or waitlist for unknown and off-best-chain parents? Third PR touching this and no maintainer has commented on any of them.

Not approving as-is for one reason: the sender-only request fires before any proof-of-work check, so an unauthenticated peer gets a metered Requested slot for free. Details at :1857.

[jozanek]

papers/inputblocks/main.tex:363-374 specifies the opposite — store in the disconnected waitlist, request from the network, evict on timeout — and that waitlist is partly real at InputBlocksProcessor.scala:703.

I think drop is the better call (a waitlist entry is unvalidated by construction, and here carries nonBroadcastedTransactions), but that argument isn't written down anywhere. Could it go in this comment once @kushti has ruled?

[cafebedouin]

This always drops an unbindable announcement now; no waitlist. Whether to waitlist an announcement whose parent is unknown is a design call — a waitlist entry is unvalidated (and an ordering announcement carries nonBroadcastedTransactions), which is why the default is drop. @kushti, drop or the paper's waitlist for announcements whose parent is unknown? I'll record the ruling here.

[cafebedouin]

Reduced this PR to the difficulty binding plus the genesis fix, per the review. Dropped isInBestChain (the binding now derives from the known parent via requiredDifficultyAfter), removed the sender-only request lifecycle, fixed the genesis guard, and lowered the drop logs to debug. Pushed as a second commit 642b237; both parent-check specs pass (24 properties). (For reference: CandidateGeneratorSpec and OrderingBlockMessageFlowSpec, which this PR does not touch, fail the same way on the previous head fe452fa as on this revision — pre-existing, not introduced here.) Your two replay cases are kept (the first now asserts no header request on first delivery); the sender-delivery and expiration cases went with the request. The best-chain restriction and the drop-vs-waitlist policy are flagged as maintainer calls. @a-shannon — the sender-only request is deferred; when it returns it will build on the request-attempt identity from #2528, so it can be reviewed alongside that.

[jozanek]

Five of six resolved — request lifecycle gone, isInBestChain out with the reasoning in the comment, logs at debug. Thanks for the turnaround.

You were right to decline the unconditional Some(initialNBits): on a headers-first node fullBlockHeight stays 0, so a height-1 announcement would bind to the initial difficulty and reach store/relay. Your guard is tighter than what I asked for.

One new regression, found by @a-shannon and verified here: both returns sit above setReceivedIfRequested, so a requested-then-delivered announcement stays Requested and CheckDelivery penalizes the peer that delivered it. Details at :1499. I missed it in round one.

Still open and not yours to close: @kushti on drop-vs-waitlist.

Holding changes requested for the receipt clear alone.

[cafebedouin]

Addressed the receipt-lifecycle regression @a-shannon reported and @jozanek verified, in b164fc0.

Both unbindable-parent drops (expectedNBits.isEmpty) now clear the pending request before returning, via a clearRequestedIfFromSupplier helper that mirrors the getRequestedInfo(..) if ri.peer == remote guard already used on the snapshot download paths: it sets the entry to Unknown (re-requestable, not Received, which would suppress later requests) and only when the delivering peer is the one it was requested from, so an unsolicited response from another peer cannot erase the real supplier's pending request.

@a-shannon's two receipt regressions plus the two headers-present/no-full-block genesis cases are included. Targeted specs pass 28/28; reverting only the production change fails exactly the two receipt regressions (26 succeeded / 2 failed), confirming the tests exercise the fix rather than passing vacuously (Java 8u502, Scala 2.12).

The drop-vs-waitlist policy question (@kushti) remains the open maintainer call and is untouched here.

— Amendment authored with Claude (Opus 4.8).

[jozanek]

The receipt clear looks right. clearRequestedIfFromSupplier uses setUnknown rather than setReceived, so the replay properties still hold, and setUnknown cancels the timer via clearStatusForModifier (:286) — CheckDelivery then no-ops instead of penalizing. The ri.peer == remote shape does genuinely mirror :989-991. Checking that reverting the production change fails exactly the two regressions was the right move.

That clears everything I raised over the two rounds, so I'm dropping my block.

One non-blocking ask, @a-shannon's: the supplier guard itself has no coverage — both fixtures deliver only from f.peer (subBlockPeer only sets sync status), so removing if info.peer == remote leaves 28/28 green. Noted at :1397.

Still open and still not yours to close: @kushti on drop-vs-waitlist. Three rounds across #2552, #2553 and #2554 with no maintainer word — you've asked twice and recorded the reasoning in the comment, which is as far as you can take it.