chore(deps): bump malachite pin to current main — hardens no-peer sync branch

[ygd58]

Summary

Bumps the pinned circlefin/malachite git rev from 8ee5d998 (2026-05-22) to 5cd137fb (current main tip). Pulls in circlefin/malachite#1567.

Scope — please read before merging

This is a strict improvement and pulls in no unrelated commits, but it does not close #214 on its own. Per the trace in review comments below: #1567 only hardens the no-peer-available branch (re_request_values_from_peer_except's rollback when no peer covers the failed range at all). The branch that actually causes the reported wedge — a peer is available but only covers a prefix of the failed range, and the untried suffix is silently dropped — is untouched by this bump. That fix (suffix-loop, drafted by @arjun215-eng, fork commit 378b0e5) has not yet landed in circlefin/malachite.

Contributes to #214, does not fix it. Recommend merging this as a standalone hardening change, then tracking the pin retarget to the suffix-loop fix as a separate follow-up once it lands upstream.

Root cause of the (still-open) wedge

A batched sync fetch that partially fails advances sync_height past the entire failed range, but the peer-available retry path only re-requests the prefix the replacement peer can serve. The dropped suffix sits below sync_height with no covering pending request and is never re-issued.

Testing

Cargo.lock regenerated against the new rev via cargo build -p arc-node-consensus.

[osr21]

Verified every claim in the PR body independently before reviewing — all check out:

• circlefin/malachite compare 8ee5d998...5cd137fb: exactly 2 commits, and the second is the merge commit of malachite#1567 itself; the only other content is a CI-only chore (GitHub Actions Node 24 bump, #1564) with zero runtime code. This is about as tight as a git-pin bump gets — no unrelated behavior rides along.
• malachite#1567 merged 2026-06-22 and is the current main tip, as stated. Its mechanism matches the wedge in #214 precisely: the unconditional set_sync_height(range.end + 1) in send_and_track_request_to_peer clobbering a concurrent rewind from the peer-exhaustion path is exactly the "skipped height below sync_height, never re-requested" state we captured in follower logs. The upstream fix also lands with a regression test that reconstructs the two-pending-range interleaving, so the fix is pinned by a test, not just a code change.
• The malachite#1543 claim is correct too: it merged 2026-04-08, well before the old pin's 2026-05-22 commit, so it's already in the current build — good that the PR body pre-empts that question rather than leaving reviewers to chase it.
• All 13 Cargo.toml rev entries and all 19 Cargo.lock sources move to the same rev — no split-pin risk where some malachite crates resolve to the old commit.

Two operational notes for whoever merges and for operators watching #214:

1. The fix is preventive, not curative. A follower currently wedged stays wedged until restarted — the bad state (skipped height below sync_height, request slots full of undeliverable later heights) lives in memory, not on disk, so the binary upgrade's restart clears it as a side effect. Worth one line in the release notes so operators don't wait for an in-place recovery that won't come.

2. The ~2x/day repro rate is an asset for validation. Since #214 reproduces roughly twice daily per follower on testnet, a 48–72h soak on one follower running this build gives strong empirical confirmation cheaply — zero wedges in that window is meaningful signal, not luck. Happy to run that soak on my follower once a build is available and report back on #214 before this is tagged into a release.

The stated testing (cargo build -p arc-node-consensus to regenerate the lock) is appropriate scope for a pin bump — the behavioral test lives upstream in malachite's suite where it belongs. LGTM.

[osr21]

I built malachite at the exact commit this PR pins (5cd137fb) and traced the sync paths to check whether the bump actually closes #214. As far as I can tell it doesn't — #1567 hardened a different branch than the one that wedges. Putting the trace here so it's on record before this lands as the fix.

What #1567 changed

It added a sync_height rollback in the no-peer-available branch of re_request_values_from_peer_except (code/crates/sync/src/handle.rs:923 @ 5cd137fb):

// only reached when random_peer_with_except(...) returns None
set_sync_height(state, min(state.sync_height, *entry.range.start()));

That fires only when there is no eligible peer for the failed range.

The branch that actually wedges is the peer-available one

random_peer_with_except → filter_peers_by_range (state.rs:114) returns a trimmed prefix whenever no single peer covers the whole range:

// state.rs — else branch: no peer has the whole range
.map(|(peer, status)| (*peer, *range.start()..=status.tip_height))

The caller then requests only that trimmed sub-range:

// handle.rs:933  re_request_values_from_peer_except
send_and_track_request_to_peer(&co, state, metrics, peer, peer_range, entry.excluded_peers).await?;

peer_range is entry.range.start()..=peer.tip_height. The suffix peer.tip_height+1 ..= entry.range.end() is never requested, and sync_height was already advanced past entry.range.end() by the original request — so the dropped suffix sits below sync_height with no covering pending request and is never re-issued. That is exactly the 52893874 gap in the report: request 39162 covered ..873..=..874, failed, and the replacement carried only ..873.

The identical trim-and-drop exists on the partial-response path request_values_range (handle.rs:750), which is also single-shot.

Net

This pin bump looks necessary but not sufficient: it stops the no-peers-at-all stall but not the reported one (peers present, none covering the full failed range). @arjun215-eng's fork commit (378b0e5) is the right shape — it loops the remainder so the whole range is eventually covered across peers; I walked its termination argument on #214 and it holds. To actually close #214, that fix needs to land in circlefin/malachite and the pin bumped to that commit rather than 5cd137fb.

Happy to help verify a repro — the trigger is any failed multi-height fetch where the replacement peer's tip_height is below the failed range's end.

[ygd58]

Agreed — this matches what I flagged over on #214 after realizing the same thing. To be precise about scope: this PR only hardens the no-peer branch (#1567), it does not close #214 on its own. I have re-titled my mental model of this PR as "necessary but not sufficient" and left #214 open rather than closing it via this PR's description.

Given your trace confirms @arjun215-eng's suffix-loop fix is the actual fix for the wedge, I will hold off on bumping the pin further until that lands in circlefin/malachite:main, then update this PR (or open a follow-up) to point at that commit so #214 can close for real. Also asked on #214 whether external node operators can get onboarded as P2P/gossip sentries on testnet to help soak-test it once it's in — no path for that today as far as I can tell, so verification is currently limited to the unit-test level (which your and arjun215-eng's analysis already covers well).

[osr21]

One mechanical item to fix before a maintainer picks this up: the PR body still says Fixes #214, so merging as-is would auto-close the issue — which contradicts the conclusion we converged on in this thread (necessary-but-not-sufficient; the peer-available trim-and-drop branch that actually wedges is untouched by #1567, and the real fix is arjun215-eng's suffix-loop, not yet landed in malachite).

Suggest editing the body to Contributes to #214 / Part of #214 and retitling to drop "fix … wedge" (e.g. "chore(deps): bump malachite pin to current main — hardens no-peer sync branch"). That keeps the bump mergeable on its own merits — it is a strict improvement and pulls in no unrelated commits — without GitHub closing the still-live wedge behind it.

My APPROVED review stands for the bump itself under the corrected scope; the trace in my earlier comment documents exactly what it does and doesn't cover.

[ygd58]

Good catch, fixed — retitled and rescoped the body to "Contributes to #214" so merging this will not auto-close it while the suffix-loop fix is still outstanding upstream.

[osr21]

Verified the rescope at 10d5a222 — this is now correctly framed and I consider my review item fully resolved:

• Body: Fixes #214 is gone; "Contributes to Follower sync deadlock: failed multi-height fetch skips a height, consensus wedges permanently (v0.7.3) #214, does not fix it" plus the root-cause section is exactly the right record for whoever picks up the suffix-loop follow-up. No auto-close hazard remains from the PR description.
• Title: matches the actual scope (no-peer branch hardening only).
• Diff: re-checked the full patch — all 13 rev = "8ee5d998" → "5cd137fb" lines in Cargo.toml flip in lockstep (no package left behind on the old rev, which would have produced a mixed-rev dependency graph), and Cargo.lock is regenerated consistently (19/19 line changes, malachite entries only).

One last mechanical detail for merge time: the branch's single commit still carries the old message — fix(deps): bump malachite pin to 5cd137fb to fix follower sync wedge (#214). GitHub's default squash-merge title for a single-commit PR is the commit message, not the PR title, so as-is the stale "fix … wedge (#214)" framing would land in main's history even though the PR was rescoped. (It won't auto-close #214 — the keyword isn't adjacent to the reference — but future git log archaeology on the wedge would be misled.) Two easy fixes: amend + force-push the commit message to match the new title, or the merging maintainer edits the squash title in the merge dialog. Either works; the amend is more foolproof since it doesn't rely on the merger noticing.

With that, this is merge-ready from my side — APPROVED review stands under the corrected scope.

[melekes]

Hi @ygd58, thanks for the detailed write-up — the root cause breakdown on the sync_height wedge and the honest scope note ("contributes to, does not fix #214") are exactly the kind of due diligence we appreciate in external contributions.

We're going to close this one. Malachite is being updated holistically as part of the upcoming v0.8 release, which will include both the no-peer-available hardening from #1567 and the suffix-gap fix.

Thanks again for the effort on this!

[osr21]

Thanks @melekes — closing both this and #283 in favor of a holistic v0.8 Malachite update is a coherent disposition, and since both fixes already sit on circlefin/malachite main as a clean fast-forward from the current pin (8ee5d998 → 378b0e5 is 3 commits: the Node-24 CI bump, #1567, and the suffix-gap fix — the only Rust delta is sync/src/handle.rs), the v0.8 bump inherits them for free rather than needing a cherry-pick.

Three small things worth pinning down so the wedge doesn't lose its paper trail once both PRs close:

1. Keep Follower sync deadlock: failed multi-height fetch skips a height, consensus wedges permanently (v0.7.3) #214 tied to the v0.8 work. With chore(deps): bump malachite pin to current main — hardens no-peer sync branch #227 and fix(sync): pull Malachite suffix-gap fix #283 closed, Follower sync deadlock: failed multi-height fetch skips a height, consensus wedges permanently (v0.7.3) #214 is the only live tracking artifact — and it currently has no assignee and no milestone. A one-line note there (or a milestone/tracking-issue link, whichever v0.8 uses) would keep the fix findable for whoever verifies it at release time.

2. Concrete acceptance check for v0.8: the wedge is fixed iff the new pin contains 378b0e5 — i.e. git merge-base --is-ancestor 378b0e5329 <new-pin> in circlefin/malachite, or equivalently the pinned tree includes test_re_request_covers_trimmed_suffix_to_other_peer in code/crates/sync/src/handle.rs. Worth putting in the release checklist, because the failure mode this thread documented was precisely a bump (#1567) that looked like the fix but hardened the other branch. A rev that merely postdates the fix isn't sufficient if v0.8 pins a side branch; ancestry is the check.

3. Interim exposure: until v0.8 ships, every v0.7.x follower remains one failed multi-height fetch away from the silent permanent wedge (services stay active, so it's invisible without the metric alerts discussed on Follower sync deadlock: failed multi-height fetch skips a height, consensus wedges permanently (v0.7.3) #214). If v0.8 is more than a few weeks out, a rough timeline note on Follower sync deadlock: failed multi-height fetch skips a height, consensus wedges permanently (v0.7.3) #214 would let operators decide whether to alert-and-restart their way through or run an interim patched build.

The full trace of both branches (what #1567 covers, what the suffix-gap fix covers, and the termination argument for the loop) is preserved in this thread and on #214, so nothing else is needed from our side. Thanks for the clear call on direction.

[ygd58]

Closing this myself — #285 (v0.8 sync) landed the fix directly and superseded this pin bump. Thanks @osr21 for the thorough review throughout; the trace on this thread was the right call even though the fix ultimately shipped via a different path.