Repository navigation
Conversation
…ream, cursors per publisher The admin publishes ROSTER chunks naming who else may publish and an EOS that ends the channel. A stream declaring a rostered address is pending and gets the roster as its cue; publisher streams receive what the others publish; the broker keeps a cursor per publisher and per service kind; a roster that drops an address demotes its streams to spectators at once; publisher streams per address are bounded. Subscribers recover the owner, admit it against the latest roster and keep a Position across sessions. The lite bridge forwards the owner's updates only until the SWIP-87 bridge is settled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, not punished SWIP-87 rev 2. A spectator never authenticates in place: a grantee that sees itself on a roster rejoins, is admitted pending and sends its AUTH after the roster cue. A roster that drops an address writes itself to that address's streams and closes them; what they send meanwhile is dropped and counted (revoked), never a violation, so a frame in flight is not punished. A stream being closed writes its queue within a flush deadline, then is reset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… reach the dApp The publish socket takes an identity: the owner by default, or a seat the owner's roster names, which waits for the broker's roster cue. ROSTER and EOS frames are the owner's alone, and a roster that does not decode is refused. Both sockets deliver kind | owner | challenge | index | soc, so a dApp sees rosters and verifies every publisher's chunks; a publisher socket receives what the others publish. An EOS closes the sockets with 4004. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…a swallowed error Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It is a large, concurrency-heavy protocol extension that the author marks as AI-generated, not fully self-reviewed, and with acknowledged gaps (flush-deadline test, fuzz targets, pending adversarial review), so it needs human sign-off.
2 open findings
What changed in this PR
This PR implements the explicit multi-publisher mode of BPS (SWIP-87) on top of BPS-lite (SWIP-74) in pkg/bps, with the matching HTTP/WebSocket bridge in pkg/api and OpenAPI documentation. The cohort principal ("admin") can now publish a roster of additional allowed publishers and an EOS to end the channel, both carried as new Kind values (ROSTER=4, EOS=3) on the existing Broadcast frame over admin-owned service feeds. The broker tracks a cursor per publisher plus a roster cursor, fans accepted frames out to all receiving streams except the author, revokes dropped addresses, and tears the cohort down on EOS; the client carries a Position (per-publisher cursors, roster, ended flag) across sessions and exposes Granted() for rostered seats.
Changes:
- Add
ROSTER/EOSkinds and aRosterprotobuf payload; add roster decode, owner recovery, and service-kind verification helpers. - Rework the broker to support multiple publishers (per-publisher cursors, roster application/revocation, EOS teardown, bounded publisher streams, flush-on-close) and the client session to admit publishers against the roster and track
Position. - Extend the
/bps/publishand/bps/subscribebridge endpoints (newidentityseat param, newkind|owner|challenge|index|socdelivery frame, close code 4004) and update the OpenAPI spec.
| File | Description |
|---|---|
| pkg/bps/pb/bps.proto | Adds EOS/ROSTER enum values and the Roster message. |
| pkg/bps/pb/bps.pb.go | Regenerated protobuf code for the Roster message and new kinds. |
| pkg/bps/id.go | New kinds, recoverOwner, decodeRoster/ParseRoster/RosterPayload, refactored verify/checkFrame. |
| pkg/bps/bps.go | Core multi-publisher broker/client logic: roster/EOS, cursors, revocation, Position, Granted. |
| pkg/bps/metrics.go | New InvalidRoster/Revoked counters and help-text updates. |
| pkg/bps/export_test.go | Exports ErrChannelEnded for tests. |
| pkg/bps/multipublisher_test.go | New tests covering grant, revocation, cursors, EOS, stream bounds. |
| pkg/api/bps.go | Bridge support for identity seats, delivery frames, roster/EOS, EOS close 4004. |
| pkg/api/export_test.go | Exports BPSCloseEndOfStream. |
| pkg/api/bps_test.go | Updated fake session and new publish/subscribe tests. |
| openapi/Swarm.yaml | Documents the multi-publisher publish/subscribe behavior and close codes. |
Files not reviewed (1)
- pkg/bps/pb/bps.pb.go: Generated file
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| func decodeRoster(chunk []byte) ([][]byte, error) { | ||
| off := swarm.HashSize + swarm.SocSignatureSize + swarm.SpanSize | ||
| if len(chunk) < off { | ||
| return nil, errInvalidRoster | ||
| } | ||
| var r pb.Roster | ||
| if err := r.Unmarshal(chunk[off:]); err != nil { | ||
| return nil, fmt.Errorf("%w: %w", errInvalidRoster, err) | ||
| } | ||
| for _, a := range r.Publishers { | ||
| if len(a) != crypto.AddressSize { | ||
| return nil, fmt.Errorf("address length %d: %w", len(a), errInvalidRoster) | ||
| } | ||
| } | ||
| return nil | ||
| return r.Publishers, nil | ||
| } |
There was a problem hiding this comment.
Fixed in 4d07b53. FuzzVerify now covers every kind (DATA, AUTH, EOS, ROSTER), with seeds for valid ROSTER and EOS chunks. On every input it also runs recoverOwner and the roster decode, and it asserts that a chunk valid at an owner recovers that owner. A new FuzzRoster fuzzes the ROSTER payload decode: it checks that every address returned is 20 bytes and that what comes back re-encodes to itself. Both ran clean for 60 s locally (2.8M and 5.4M executions).
| // KindAuth is the empty chunk at index 0 of the session's AUTH feed: it authenticates | ||
| // a pending stream, is a heartbeat on a publisher stream, and is never delivered. | ||
| // a pending stream, or a spectator stream a roster names, is a heartbeat on a | ||
| // publisher stream, and is never delivered. |
There was a problem hiding this comment.
Right, that was rev 1's wording. Fixed in 4d07b53: the KindAuth comment, and the same line in bps.proto (with bps.pb.go regenerated), now say that the AUTH authenticates a pending stream only, and that a grantee rejoins as pending.
…tes a pending stream only FuzzVerify now covers EOS and ROSTER, runs recoverOwner on every input and asserts that a chunk valid at an owner recovers that owner; FuzzRoster fuzzes the ROSTER payload decode. The Kind AUTH comments drop the rev-1 wording that a spectator a roster names may authenticate in place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…OS ends the cohort A grantee joins as a spectator and authenticates where it is once a roster naming it has been delivered there; a rostered identity is admitted past a full fan-out set and may simply listen. Pending is the admin's alone. An AUTH racing the roster that revokes it closes the spectator, unpunished. A ROSTER or EOS is delivered back to its sender, so the admin admits by where its roster lands. An accepted EOS marks the cohort ended, reaches pending streams too, and stops all reading; a Join meanwhile creates a fresh cohort. A stream being closed is reset at the flush deadline even mid-write. The auth timeout can no longer race an AUTH. The bridge tells a seat it is granted and gives the owner 25 s to authenticate, under the broker's 30 s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A stream being closed by an EOS or a revocation, whose peer has stopped reading, is reset at the flush deadline even with a write in progress; a revoked peer that keeps sending, and a silent peer that never closes its side, are closed promptly, not reset, their frames dropped unpunished. The broker's handler runs over a stream whose writes the test can stall, which the in-memory streamtest transport cannot do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A delivery that does not validate, or whose owner the roster does not admit, is something no honest broker sends: the subscriber resets the stream and blocklists the broker for the longest period there is, forever. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Checklist
Description
The explicit multi-publisher mode of BPS, SWIP-87, on top of BPS-lite (#5626). Stacked on
bps-simplified: this PR shows only the three SWIP-87 commits.The admin (the cohort's principal) publishes a roster naming who else may publish, and can end the channel with an EOS. Both are new kinds on the existing
Broadcastframe, each on its own feed owned by the admin. No new frame, and no change toJoin,Ackor the cohort spec.Broker (
pkg/bps)KindgainsEOS = 3andROSTER = 4;Roster{publishers}is the payload of a ROSTER chunk. ROSTER and EOS are accepted only on the admin's publisher stream and validated with the owner forced to the principal.Joindeclaring a rostered address is pending, like the admin's, and is sent the latest roster at once as its cue.revoked, never punished — it may be in flight, or in multihop it may come from a relay that has not yet seen the roster.FlushTimeout, 5 s), then is reset.MaxPublisherStreams, 2); an AUTH beyond the bound resets the oldest.invalid_roster,revoked.Client (
pkg/bps)bps.Positionholds the cursor per publisher, the roster with its cursor, and whether the channel has ended. It is carried across sessions, and a channel seen ended is not rejoined.Session.Grantedfires once a grantee may send its AUTH: it joined knowing it was rostered, and the broker's first frame is the roster naming it.Bridge (
pkg/api,openapi/Swarm.yaml)identity: the owner by default, or a seat on the owner's roster.kind | owner | challenge | index | soc, and the publish socket also delivers what the others publish.Open API Spec Version Changes (if applicable)
The
/bps/publishand/bps/subscribedescriptions change (newidentityon publish, a new delivery frame format, close code 4004). These endpoints are introduced in #5626 and have not been released.Motivation and Context (Optional)
SWIP-87 is the roster-and-EOS part of SWIP-60, split out so that it can be implemented and conformance-tested on top of BPS-lite on its own.
Related Issue (Optional)
Depends on #5626. Spec: ethersphere/SWIPs#114.
AI Disclosure
The flush deadline is tested on both sides in
flush_test.go. Fuzz targets:FuzzVerifycovers every kind and owner recovery, andFuzzRostercovers the roster decode. An adversarial review (5 reviewers, each checked by a refuting verifier) ran against rev 2; its findings are addressed in rev 3.🤖 Generated with Claude Code