Skip to content

Upgrade reliable sender to libnet-rs v0.2.1 upstream crates #4

Description

@akhilsb

Motivation

The peer-to-peer layer currently depends on the fork akhilsb/libnet-rs (bd109bc), a single network crate whose sender/receiver live under a plaintcp module. Upstream libdist-rs/libnet-rs has since been restructured into a per-transport workspace and tagged v0.2.1, with a meaningfully different API.

Doing the reliable-sender upgrade on TCP first de-risks #1: the API-shape change and the TLS change are independent, and landing them together makes it impossible to tell a migration bug apart from a TLS handshake bug. After this issue, #1 becomes largely a matter of swapping tcp-* crates for tls-* crates, which expose the same shape.

Current vs. target

Fork (today) Upstream v0.2.1
Packaging one network crate, plaintcp module tcp-reliable-sender, tcp-receiver, tcp-sender, common (all 0.2.0)
Sender type TcpReliableSender<Id, SendMsg, RecvMsg> TcpReliableSender<Id, SendMsg>
send -> CancelHandler<RecvMsg> -> Result<CancelHandler, OpError<Id>>
CancelHandler oneshot::Receiver<RecvMsg> oneshot::Receiver<Result<Bytes, SendError>>
Payload typed SendMsg, serialized inside the crate Bytes, serialized by the caller
Receiver Handler::dispatch callback TcpReceiver<RecvMsg> implements Stream<Item = Result<RecvMsg, _>>
Ack Acknowledgement::Pong enum common::Ack / ACK_BYTES behind the ack feature
Tuning hardcoded in source common::Options

Work items

  1. Repoint the 14 Cargo.toml files from the network git dependency to the upstream tcp-reliable-sender / tcp-receiver crates at tag v0.2.1.
  2. Drop the third generic parameter and thread Result through the ~39 net_send.send(..) / broadcast(..) call sites and ~73 CancelHandler references.
  3. Move bincode encoding of WrapperMsg<ProtMsg> to the call sites, since the sender now takes Bytes.
  4. Convert the 14 TcpReceiver::spawn sites from handler-dispatch to the Stream interface.
  5. Replace the ~175 Acknowledgement references with common::Ack under the ack feature.
  6. Set Options::max_frame_length explicitly — see the gotcha below.

Affected crates: broadcast/{ctrbc,ccbrb,ecc_rbc}, consensus/{acs,binary_ba,fin_mvba,ibft,ra}, dissemination/{asks,avid}, node/src/syncer.rs.

The NetSender trait (blocking_send, randcast, blocking_broadcast) has no call sites in this repo, so its removal upstream costs nothing.

Gotcha: frame length

The fork hardcodes set_max_frame_length(400 GiB) in src/codec.rs:11,56 — the "temp fix" from 289520c / cf13e88 / 13b06e8. Upstream's Options::max_frame_length defaults to 8 MB. Reed-Solomon shard messages will silently exceed that, so the default must be raised deliberately rather than inherited. This is a good outcome: the limit becomes a tunable instead of a local patch, and this issue removes the reason the fork existed.

What we gain

  • No more carrying a fork for frame-size patches.
  • Options tuning: SO_SNDBUF / SO_RCVBUF, TCP_NODELAY, write-buffer sizing, batch drain caps, retry backoff.
  • send_many for pipelined sends to one peer without per-message ACK round-trips — directly useful for shard dissemination.
  • Bounded channels with a try_send fast path; the unbounded cargo feature preserves the fork's current unbounded semantics if we want a like-for-like first step.

Acceptance criteria

  • cargo check --workspace clean, no network git dependency remaining.
  • All protocols run end-to-end via the existing scripts at current node counts.
  • Large-shard broadcasts unaffected (frame length explicitly configured and covered).
  • No throughput regression against current benchmarks.

Relationship to other issues

Prerequisite for #1 (TLS channels). Independent of #2 (PKI), which supplies the certificate material #1 needs.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions