feat(core): implement priority

[iamquang95]

Fix #177

[iamquang95]

@claude review this pr

[claude]

Claude finished @iamquang95's task in 4m 59s —— View job

---

Review: feat(core): implement priority

I read the full priority crate and compared it against the Charon Go source (charon/core/priority/{calculate,prioritiser,component}.go). This is a careful, well-documented port — the module/doc comments are excellent, the test coverage is strong (unit + 3-host integration), and CI is green across all 8 checks. Below are findings ordered by significance. Most are notes/questions rather than blockers.

---

Functional equivalence vs Charon

1. Protocol ID has a leading / — breaks Charon interop (deliberate, please confirm it's accepted)
PROTOCOL_ID = "/charon/priority/2.0.0" (prioritiser.rs:45) vs Charon's protocolID2 = "charon/priority/2.0.0" (prioritiser.go:39). The leading slash is mandatory in rust-libp2p (StreamProtocol rejects slash-less tokens), so this is unavoidable, and the doc comment is admirably honest about it. But it means a Pluto node and a Charon node will fail multistream negotiation on this protocol — they can never form a mixed cluster for priority. Given AGENTS.md's "functional equivalence" rule, this is the one place the port cannot be wire-compatible. Worth an explicit maintainer sign-off that Pluto↔Charon priority interop is out of scope (vs. all-Pluto clusters only).

2. Consensus value hash covers the per-peer msgs field — confirm QBFT adopts the leader's proposal
calculate_result sets msgs: msgs.to_vec() (calculate.rs:120), faithfully mirroring Go. But run_instance accumulates msgs in arrival order (prioritiser.rs:371, add_msg), which differs per node, and the QBFT layer hashes the entire PriorityResult including msgs (consensus/src/qbft/component.rs:193). So different nodes propose different value hashes for the same duty. Real multi-node consensus therefore depends on the QBFT definition accepting a valid leader proposal rather than requiring the local computation to be byte-identical. This is consistent with Charon's design, but note that neither the 3-host integration test nor the unit tests exercise it — TestConsensus/MockConsensus "decide on the first proposal" and only assert topics equality, never round-driven agreement over divergent msgs. Suggest a follow-up confirming definition.rs validates priority proposals structurally. Not a blocker for this PR.

3. Stable vs unstable sort on ties (minor, non-issue)
all_priorities.sort_by(|a, b| scores[b].cmp(&scores[a])) (calculate.rs:95) is a stable sort; Charon uses sort.Slice (unstable, calculate.go:73). For equal scores the tie-break order can differ from Go. Within a homogeneous Pluto cluster this is deterministic (and arguably more correct), and interop is already off the table per (1) — so this is fine, just flagging the deviation.

4. DeadlineComputeFailed is a new error path vs Go's silent drop (intentional)
prioritise returns Err(DeadlineComputeFailed) on AddOutcome::FailedToCompute (prioritiser.rs:337), whereas Charon's deadliner.Add returning false always drops silently with nil. This is the deliberate change from commit bdbdf95 ("surface deadline compute failed error") and is an improvement — just noting it diverges from strict Go behavior.

---

Correctness & concurrency (looks solid)

• Shared receive deadline in handle_request (prioritiser.rs:191-206): pinning one RECEIVE_TIMEOUT across both the enqueue and response-wait selects is a clean translation of Go's single-ctx semantics, and it correctly prevents a peer from pinning the handler. Nicely covered by handle_request_times_out_waiting_for_response.
• OnceLock cycle-break in new_internal (prioritiser.rs:240-270) for the inbound-handler↔Inner cycle is correct and the invariant (filled before the swarm drives the behaviour) holds.
• Keep-alive: the inbound/outbound futures own the Stream for the duration of the exchange, and (unlike parsigex) do not call ignore_for_keep_alive(), so in-flight exchanges keep the connection alive in libp2p 0.56. Good.
• Outbound routing by identity (handler.rs:81, OutboundOpenInfo = OutboundRequest): pairing the request with its negotiated substream rather than relying on negotiation order is correct and well-tested by concurrent_same_peer_exchanges_route_by_identity.
• The seeding of msgs with own while leaving dedup_peers empty (prioritiser.rs:371) correctly preserves Go's behavior of surfacing a duplicated-own-peer as a DuplicatePeer validation error.

---

Minor / style nits

• dedup_peers: HashMap<String, ()> (prioritiser.rs:372, add_msg) and connected: HashMap<PeerId, ()> (behaviour.rs:33) are sets — prefer HashSet<String> / HashSet<PeerId> for intent. Fix this →
• behaviour.rs:97 fail_awaiting/clone_error: only Transport/Unsupported ever reach the awaiting path, so reconstructing via to_string() is fine, but a short assertion/debug_assert documenting that invariant would harden it against future error variants.
• error.rs: Transport(String) loses the structured source — acceptable given libp2p's error types, just noting the downgrade to string.
• consensus.rs:21 ConsensusError = Box<dyn StdError + Send + Sync> is fine, but consider whether a concrete enum would give callers better matching; low priority.

---

Verdict

Strong, faithful port with thorough docs and tests. No correctness blockers found. The two items worth an explicit decision before merge are (1) the protocol-ID interop break and (2) confirming the QBFT priority definition tolerates per-peer msgs ordering in a real multi-node round (the current tests don't cover it). Everything else is minor.
· branch feat/core-priority