Modifying extend to return adopted orphan beads #474
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the braid extension flow so Braid::extend() reports which previously-parked orphan beads were promoted (i.e., became connectable) as a result of adding a bead. This fits into the node’s DAG (“braid”) maintenance by making orphan promotion observable to callers.
Changes:
- Extend
AddBeadStatus::BeadAddedintoBeadAdded { promoted_orphans: Vec<Bead> }and plumb throughprocess_orphan_beads()to return the promoted set. - Update call sites to match the new
BeadAdded { .. }shape. - Add tests covering: no orphan promotion, single orphan promotion, and transitive orphan-chain promotion.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
Sansh2356
force-pushed
the
ibd/braid-add-bead-outcome
branch
from
e48e658 to
48deb1d
Compare
Sansh2356
changed the title
extend to return promoted orphan beads extend to return ~~promoted~~ adopted orphan beads
Sansh2356
changed the title
extend to return ~~promoted~~ adopted orphan beads extend to return adopted orphan beads
nkatha23
left a comment
nkatha23
left a comment
There was a problem hiding this comment.
ACK 48deb1d
The change is clean. process_orphan_beads() now returns the promoted set and the recursive promoted.extend(self.process_orphan_beads()) correctly handles transitive chains in a single extend call, the transitive test confirms this.
One observation: the call sites in main.rs, lib.rs, and rpc_server.rs all use { .. } to ignore promoted_orphans for now. That's fine given the PR description says usage comes in a followup, but it's worth noting that promoted orphans are currently not being persisted to DB or forwarded to peers at those sites, so during IBD, orphans that get promoted through extend won't trigger the downstream DB write path. Assuming that's intentional and the followup PR will wire that up.
Also noting: test_invalid_json fails due to a pre-existing port 5050 leak in server_start_test unrelated to this PR.
Tests binding fixed ports (3353, 3356, 3357, 3358, 5050) caused AddrInUse failures when tests ran concurrently or a previous test left a server listening. This was a known flake documented across multiple PRs (braidpool#309, braidpool#474). Add bound_addr_tx: Option<oneshot::Sender<SocketAddr>> to run_stratum_service. After binding, the actual address is sent through the channel so tests can connect to the real port. All 5 tests now use port 0 (OS-assigned) and await the oneshot receiver before connecting, no sleep needed, no race window. Production call in main.rs passes None.
Sansh2356
force-pushed
the
ibd/braid-add-bead-outcome
branch
from
48deb1d to
565c377
Compare
Tests binding fixed ports (3353, 3356, 3357, 3358, 5050) caused AddrInUse failures when tests ran concurrently or a previous test left a server listening. This was a known flake documented across multiple PRs (braidpool#309, braidpool#474). Add bound_addr_tx: Option<oneshot::Sender<SocketAddr>> to run_stratum_service. After binding, the actual address is sent through the channel so tests can connect to the real port. All 5 tests now use port 0 (OS-assigned) and await the oneshot receiver before connecting, no sleep needed, no race window. Production call in main.rs passes None.
…477) * fix(tests): replace hardcoded stratum ports with OS-assigned port 0 Tests binding fixed ports (3353, 3356, 3357, 3358, 5050) caused AddrInUse failures when tests ran concurrently or a previous test left a server listening. This was a known flake documented across multiple PRs (#309, #474). Add bound_addr_tx: Option<oneshot::Sender<SocketAddr>> to run_stratum_service. After binding, the actual address is sent through the channel so tests can connect to the real port. All 5 tests now use port 0 (OS-assigned) and await the oneshot receiver before connecting, no sleep needed, no race window. Production call in main.rs passes None. * fix: use actual bound port in stratum endpoint logging server_endpoints and the warning log were using self.stratum_config.port which shows 0 when OS port assignment is used. Switch to bound_addr.port() so logs always show the real listening port.
Sansh2356
force-pushed
the
ibd/braid-add-bead-outcome
branch
2 times, most recently
from
d6c9716 to
2b9c914
Compare
mcelrath
left a comment
mcelrath
left a comment
There was a problem hiding this comment.
Looks good to me. Just a note on terminology, I'd prefer if it was called adopted rather than promoted_orphans.
… bead to return promoted orphans Instead of fetching promoted orphans upon receiving their parents and being successfully validated and extended the orphan beads that are dependent on these to be received and are then promoted from the orphan set. I have modified the functions to directly return the promoted orphans instead that will be used for orphan persistance in db .
Sansh2356
force-pushed
the
ibd/braid-add-bead-outcome
branch
from
2b9c914 to
5ac2442
Compare
extendfunction signatures to returnpromoted_orphan_beadsadopted_orphan_beads .