roundpb+round: add proto defs and serialization for round mailbox transport - #144
Merged
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the internal messaging and state management within the application by introducing a durable actor model. This foundational change moves critical communication and state persistence from volatile in-memory structures to a robust, database-backed system. The primary impact is enhanced reliability and crash-recovery capabilities for core protocol operations, ensuring that messages are processed exactly once and actor states are consistently maintained across restarts. This lays the groundwork for more resilient and scalable distributed components. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-architected durable actor framework, complete with persistent mailboxes and a transactional outbox pattern for crash-resilient message processing. The new darepod daemon provides a solid foundation for the client application, and the protobuf definitions for the round protocol are a crucial step towards a production-ready transport layer. The changes are extensive but appear to be of high quality, with thorough test coverage for the new components. The enhanced error handling in the Tell methods and the robust transaction broadcast logic in the chainsource actor are notable improvements.
Note: Security Review did not run due to the size of the PR.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6439160226
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Roasbeef
force-pushed
the
round-actor-protos
branch
3 times, most recently
from
March 6, 2026 06:38
a0f2f6d to
08fbbcd
Compare
Roasbeef
force-pushed
the
round-actor-protos
branch
from
March 7, 2026 01:54
08fbbcd to
d0f8824
Compare
Define the protobuf schema for all round protocol messages that flow between client and server through the mailbox transport. The proto file covers both directions: Server-to-client (S2C) events: ClientSuccessResp, ClientBatchInfo, ClientAwaitingInputSigsResp, ClientVTXOAggNonces, ClientVTXOAggSigs, ClientRoundFailedResp, ClientErrorResp. Client-to-server (C2S) requests: JoinRoundRequest, SubmitNoncesRequest, SubmitPartialSigRequest, SubmitForfeitSigRequest, SubmitVTXOForfeitSigsRequest. A companion convert.go file provides helpers for converting between wire types (outpoints, tx outputs, PSBT, schnorr signatures, musig2 nonces, tree paths) and their proto representations. These helpers are shared by both the ToProto and FromProto implementations in the round package. The gen_protos.sh script is updated to include the new roundpb package in the generation pipeline. Co-authored-by: András Bánki-Horváth <554360+bhandras@users.noreply.github.com>
Wire up the ToProto methods on all client-to-server outbox message types so they produce real proto messages instead of returning nil stubs. This enables the serverconn egress to serialize these messages into mailbox envelopes for transport. Covered types: JoinRoundRequest (boarding requests, VTXO requests, forfeit requests, leave requests, auth payload), SubmitNoncesRequest (per-signer nonce maps), SubmitPartialSigRequest (per-signer partial signature maps), SubmitForfeitSigRequest (boarding input signatures), and SubmitVTXOForfeitSigsToServer (forfeit tx + client VTXO signatures). The corresponding test is updated from asserting nil (stub behavior) to asserting non-nil (real proto output). Co-authored-by: András Bánki-Horváth <554360+bhandras@users.noreply.github.com>
Add FromProto methods to all server-to-client event types so the serverconn ingress can deserialize mailbox envelopes back into domain objects. Each method validates the proto message type, converts wire representations (outpoints, PSBT, tree paths, nonces, signatures) to their Go equivalents, and populates the event struct. Covered S2C types: RoundJoined (from ClientSuccessResp), CommitmentTxBuilt (from ClientBatchInfo), AwaitingBoardingSigs (from ClientAwaitingInputSigsResp), NoncesAggregated (from ClientVTXOAggNonces), OperatorSigned (from ClientVTXOAggSigs), BoardingFailed (from ClientRoundFailedResp or ClientErrorResp). Also adds FromProto on JoinRoundRequest for test code that needs to deserialize captured mailbox envelopes back to the client domain type (used by join auth forgery tests). Compile-time assertions verify all S2C types implement the inboundServerMessage interface.
Roasbeef
force-pushed
the
round-actor-protos
branch
from
March 7, 2026 02:04
d0f8824 to
287c129
Compare
Move the OOR mailbox wire protocol package from the top-level oorwire/ directory into rpc/oorpb/ to colocate it with the other RPC proto packages (roundpb, etc.) under the rpc/ tree. This rename also changes the Go package name from oorwire to oorpb for consistency with the naming convention used by roundpb. All proto stubs are regenerated via make rpc with the updated go_package option pointing to rpc/oorpb, and all import sites are updated accordingly. Co-Authored-By: bhandras <bhandras@users.noreply.github.com>
Replace the broken fmt.Sscanf-based parser with chainhash.NewHashFromStr + strconv.ParseUint. The old implementation scanned hex bytes in forward order, but OutpointToMapKey (via wire.OutPoint.String) produces byte-reversed hex. This caused a round-trip mismatch that would break connector leaf map deserialization. Also update the ConnectorLeafMap proto comment to describe the actual "hash:index" string key format instead of the 36-byte binary encoding, and remove the dead OutpointKeyBytes function that implemented the old format.
Schnorr signatures are 64 bytes (32-byte R.x + 32-byte s), not 32 bytes as previously documented. Fix all proto field comments and Go doc comments that incorrectly stated "32 bytes" for schnorr signatures.
Change the ServerMessage.ToProto() interface from returning a bare proto.Message to returning fn.Result[proto.Message]. This surfaces serialization errors that were previously silently swallowed: - SubmitPartialSigRequest: partial sig encode error was discarded with `_ = sig.Encode(&buf)`. - SubmitVTXOForfeitSigsToServer: missing forfeit tx map entries and MsgTx serialization failures were silently skipped with `continue`. All callers in serverconn (Encode, handleSendClientEvent) and the oor outbox messages are updated to use .Unpack() for the (T, error) pattern. Test assertions use .UnwrapOrFail(t) for concise success checks. Also adds a nil guard for LeafOutput in CommitmentTxBuilt.FromProto to prevent nil pointer dereference when the server sends a ConnectorLeafInfo with a missing leaf_output.
Add doc comments clarifying three edge cases identified during review: - TreeFromProto: note that FinalKey is nil after deserialization and callers must run Materialize to recompute it from CoSigners. - ConnectorLeafInfo.LeafIndex: document that this field is not populated by FromProto since the server's ConnectorLeafInfo proto does not carry it; only local tree-building code sets it. - ClientConnectorLeafInfo proto: note it is currently unused in wire protocol messages and reserved for future client-to-server forfeit flows.
Add rapid-based property tests covering round-trip correctness for all conversion helpers in roundpb/convert.go: - OutpointToProto/FromProto and OutpointsToProto/FromProto - OutpointToMapKey/FromMapKey (byte-reversed hex format) - TxOutToProto/FromProto - SchnorrSigToBytes/FromBytes - TxIDToHex/FromHex - TreeToProto/FromProto (recursive node structure) - MsgTxToBytes/FromBytes - PSBTToBytes/FromBytes Each property test generates random inputs via rapid generators and verifies the value survives a full encode→decode round-trip unchanged. Also includes nil-handling tests and negative tests for malformed map keys and hex strings.
Add registerRoundEventRoutes to wire up server-push round protocol events to the round actor via the EventRouter. Each push event is deserialized from its roundpb proto, populated via FromProto, and wrapped in a ServerMessageNotification for delivery to the round actor's durable mailbox. The following push event routes are registered: - BatchInfo (ClientBatchInfo → CommitmentTxBuilt) - AwaitingInputSigs (ClientAwaitingInputSigsResp → AwaitingBoardingSigs) - AggNonces (ClientVTXOAggNonces → NoncesAggregated) - AggSigs (ClientVTXOAggSigs → OperatorSigned) - RoundFailed (ClientRoundFailedResp → BoardingFailed) - Error (ClientErrorResp → BoardingFailed) Also adds roundpb/service.go with ServiceName and push event method name constants following the same pattern as oorpb. Co-Authored-By: bhandras <bhandras@users.noreply.github.com>
Add rapid-based property tests covering round-trip correctness for all OOR proto conversion helpers: - encodeOutPoint/decodeOutPoint - encodeSigningDescriptor/decodeSigningDescriptor - NewSubmitPackageRequest/ParseSubmitPackageRequest - NewSubmitPackageResponse/ParseSubmitPackageResponse - NewFinalizePackageRequest/ParseFinalizePackageRequest - NewFinalizePackageResponse/ParseFinalizePackageResponse - decodeSessionID invalid length rejection Each property test generates random inputs via rapid generators and verifies the value survives a full encode→decode round-trip unchanged.
Move lib/actormsg import to correct alphabetical position per gci linter. Extract inline Adapt closure to standalone roundEventAdapt function to stay under the 80-char line limit. Co-Authored-By: bhandras <bhandras@users.noreply.github.com>
This function became unused after the fn.Result[proto.Message] migration replaced error-envelope returns with fn.Err.
Fix gci import ordering and split long hex string literals across lines to stay under the 80-char line limit.
malicious server input Address several security findings from code review: C1/M1: Add pre-order invariant check (childIdx > i) in TreeFromProto to prevent cycle injection and diamond DAGs in deserialized VTXO trees. A malicious server could previously craft self-referential or mutually-referential node graphs that cause stack overflow on any recursive tree traversal (signing, verification, unrolling). M2: Add output index bounds check in TreeFromProto child wiring. Previously, a child could reference an output index that doesn't exist in the parent node, causing downstream OOB panics. M3: Add configurable max node count (DefaultMaxTreeNodes = 50,000) with WithMaxTreeNodes functional option to prevent OOM DoS from unbounded tree allocations. H1: Reject nil PSBT in CommitmentTxBuilt.FromProto. An omitted batch_psbt field previously produced a nil Tx that would panic on dereference during sighash computation. H2: Validate non-negative values in TxOutFromProto. The proto int64 value field could carry negative values that corrupt fee calculations and tree node amounts. Change TxOutFromProto signature to return error. M4: Reject negative tree path indices in CommitmentTxBuilt.FromProto since they are semantically invalid as commitment tx output indices.
deserialization Add comprehensive regression tests that verify the security hardening from the previous commit. Tests cover: - TreeFromProto cycle rejection (self-reference, mutual cycle, back-edge exploit chain) - TreeFromProto large node count rejection (OOM DoS) - TreeFromProto output index bounds checking - TreeFromProto negative node amount rejection - CommitmentTxBuilt.FromProto nil PSBT rejection - CommitmentTxBuilt.FromProto negative tree path index rejection - CommitmentTxBuilt.FromProto negative connector amount rejection - Diamond DAG behavior documentation (accepted but noted) - Byte order consistency verification - Nil passthrough behavior for PSBT, schnorr sigs, MsgTx - Outpoint map key edge cases Tests that previously demonstrated vulnerabilities (assert NoError on malicious input) are converted to regression tests that verify the new validation rejects the input.
Add a BoardingInputSigToProto conversion helper that validates the domain InputIndex (int) fits within int32 range before casting to the proto field. Without this check, indices exceeding MaxInt32 would silently truncate, potentially causing the server to sign the wrong input in a forfeit transaction. Also fix two pre-existing lint issues: rename the WithMaxTreeNodes parameter from `max` to `maxNodes` to avoid shadowing the builtin, and add the required blank line after the multi-line TreeFromProto function signature.
Wire the new BoardingInputSigToProto helper into SubmitForfeitSigRequest.ToProto so that input index bounds checking happens in the roundpb conversion layer rather than inline. Add a TreeOpts field to CommitmentTxBuilt so that VTXO tree deserialization options (e.g., max node count) can be injected by the event router from daemon configuration and passed through to roundpb.TreeFromProto during FromProto. Document that map iteration in SubmitNoncesRequest.ToProto, SubmitPartialSigRequest.ToProto, and SubmitVTXOForfeitSigsToServer.ToProto is non-deterministic. This is acceptable because proto map fields have no ordering semantics and downstream code does not derive idempotency keys from raw proto bytes. Fix lint line-length violations in security test roundID literals.
Add a MaxTreeNodes field to ServerConfig that caps the number of nodes accepted in a VTXO tree received from the server, preventing memory exhaustion from oversized payloads. The default is set to roundpb.DefaultMaxTreeNodes (50,000) in DefaultConfig. In registerRoundEventRoutes, capture the configured value and build a TreeFromProtoOption slice that is injected into CommitmentTxBuilt via the newEvent closure. This threads the limit through FromProto into roundpb.TreeFromProto without changing any interface signatures.
Roasbeef
force-pushed
the
round-actor-protos
branch
from
March 7, 2026 03:30
287c129 to
cbd092f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In this PR, we add the protobuf definitions and wire serialization needed for
the round protocol's mailbox transport layer. Up until now, the systest bridge
used direct in-memory type switches to shuttle messages between client and
server actors. With this change, we define a proper proto schema for all round
protocol messages and wire up
ToProto/FromProtoon the client-side types sothe production
clientconn/serverconntransport can serialize them asTLV-wrapped proto envelopes through the mailbox.
Fixes https://github.com/lightninglabs/darepo/issues/109
Proto Schema (
rpc/roundpb)We define the full set of messages for both directions of the round protocol:
Server-to-client (S2C) events:
ClientSuccessResp,ClientBatchInfo,ClientAwaitingInputSigsResp,ClientVTXOAggNonces,ClientVTXOAggSigs,ClientRoundFailedResp,ClientErrorResp. These map 1:1 to the domain eventtypes the round FSM consumes.
Client-to-server (C2S) requests:
JoinRoundRequest,SubmitNoncesRequest,SubmitPartialSigRequest,SubmitForfeitSigRequest,SubmitVTXOForfeitSigsRequest. These are the outbox messages the round actorproduces during its state transitions.
A companion
convert.goprovides shared helpers for converting between wiretypes (outpoints, tx outputs, PSBT bytes, schnorr sigs, musig2 nonces, tree
paths) and their proto representations. Both
ToProtoandFromProtoin theroundpackage rely on these helpers.Egress:
ToProtoon Outbox MessagesWe wire up
ToProtoon all C2S outbox message types so the serverconn egresspath can serialize them into mailbox envelopes. Covered types include
JoinRoundRequest(with boarding, VTXO, forfeit, and leave request variantsplus the auth payload),
SubmitNoncesRequest,SubmitPartialSigRequest,SubmitForfeitSigRequest, andSubmitVTXOForfeitSigsToServer.Ingress:
FromProtoon Server Event TypesWe add
FromProtomethods to all S2C event types so the serverconn ingress candeserialize mailbox envelopes back into domain objects. Each method validates
the proto message variant, converts wire representations back to their Go
equivalents, and populates the event struct. Compile-time assertions verify all
S2C types satisfy the
inboundServerMessageinterface.See each commit message for a detailed description w.r.t the incremental
changes.