Skip to content

Commit c571953

Browse files
committed
fix(review): close the remaining findings from the round on this PR
The unsigned-acceptance warning fired before two gates that could still drop the event, so a message the operator reads as an admission could be followed by a forwarder-budget shed or an unknown-peer refusal. It now fires only once the event is genuinely admitted, and a test pins that it fires exactly once, which nothing checked before. The publish site hardcoded the format version while the ingest gate compared against the constant, so emitter and gate could drift apart silently. The publish site now reads the constant. The known-peer gate changed a federation precondition without documenting it: a gossip publisher must also be a known peer on the receiving node, via bootstrap or a prior announce, or its events are dropped in both flag modes. That is now in the README beside the rollout ordering, derived from what the ingest path does rather than from another docstring. Six tests each hand-rolled the same both-modes rejection loop; they now share one helper. The log-capture helper was left under a tmp_ prefix from the round that added it and is renamed, since scaffolding vocabulary should not ship.
1 parent 27f834c commit c571953

3 files changed

Lines changed: 217 additions & 89 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,8 @@ The flag is not HTTP-only. It also gates inbound gossip ref-update events on the
326326

327327
Because the flag now spans both transports, the rollout order matters in one direction: **upgrade every gossip-publishing peer to a build that signs events before you set this to `true`.** Enabling it while an old publisher is still live drops that publisher's ref-updates on arrival, and the publisher sees no error, because gossip has no response to carry one. Upgrading the HTTP peers alone is not sufficient.
328328

329+
There is a second precondition on gossip ingest that the flag does not control. A publisher's `node_did` must already exist in the receiving node's peers table, or the event is dropped as an unknown peer DID. This holds in both modes, signed and unsigned alike: a valid signature proves key possession, not membership. Rows reach that table over HTTP, either from `GITLAWB_BOOTSTRAP_PEERS` when this node contacts a bootstrap peer and records the DID it reports, or from a prior `POST /api/v1/peers/announce`. So a peer that only ever joined the libp2p mesh, with no HTTP announce and no bootstrap contact in either direction, will have its ref-updates dropped even with a good signature and the flag off. If gossip is silently not landing from a peer you can see on the mesh, check that its DID is in `GET /api/v1/peers` on the receiving side first.
330+
329331
---
330332

331333
## Configuration

‎crates/gitlawb-node/src/api/repos.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2461,7 +2461,11 @@ async fn post_receive_replication_tail(
24612461
if announce {
24622462
if let Some(p2p) = &p2p_handle {
24632463
p2p.publish_ref_update(crate::p2p::RefUpdateEvent {
2464-
v: 0,
2464+
// Named, not literal: the ingest gate compares `v`
2465+
// against this same constant, so a bump has to move
2466+
// both ends together rather than leaving the emitter
2467+
// on a version the gate no longer accepts.
2468+
v: crate::p2p::CURRENT_REF_UPDATE_VERSION,
24652469
node_did: node_did_str.clone(),
24662470
pusher_did: pusher_did_clone.clone(),
24672471
repo: repo_slug.clone(),

0 commit comments

Comments
 (0)