WIP: mailbox round/OOR plumbing + toy SDK e2e surface - #130
Conversation
Summary of ChangesHello @bhandras, 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 establishes a foundational embedded SDK for the Ark protocol, primarily aimed at facilitating system tests. It integrates core Ark functionalities such as round participation, out-of-round transfers, and VTXO state management through a mailbox-based RPC transport. The changes introduce a new client daemon ( 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
Activity
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 set of changes to wire up end-to-end functionality for mailbox-based rounds and out-of-round (OOR) payments, including a new darepod daemon and a toy SDK. The changes are extensive, adding new protobuf definitions for arkrpc, indexer, and daemonrpc, along with their corresponding generated Go code. The PR also introduces new wire formats for OOR and round messages, a client for the indexer service, and a high-level SDK client. The overall structure is well-organized, with clear separation of concerns between the daemon, RPC services, and the SDK. The use of cobra and viper for the new daemon is a good choice. The mailbox transport logic is extended to support routed events, which is a key piece of the new plumbing. My review found one minor issue regarding error handling in the new daemon's main command file.
| v.SetEnvPrefix("DAREPOD") | ||
| v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) | ||
| v.AutomaticEnv() | ||
| _ = v.BindPFlags(f) |
There was a problem hiding this comment.
The error returned by v.BindPFlags is being ignored. While an error is unlikely here (it seems to only error if the flag set is nil), it's a best practice to handle all returned errors for robustness. A panic with a descriptive message would be appropriate for such an unrecoverable setup error.
| _ = v.BindPFlags(f) | |
| if err := v.BindPFlags(f); err != nil { | |
| panic(fmt.Errorf("error binding pflags: %w", err)) | |
| } |
0efdd12 to
42818df
Compare
79074d3 to
19c8951
Compare
This commit introduces the client/runtime-side plumbing needed to run a real
mailbox-backed round+OOR flow and expose it through a minimal SDK surface that
systests can drive end-to-end.
What is added:
- Round mailbox wire protocol:
- add `roundwire` payload schema/serialization for round request/response and
server event envelopes.
- add round mailbox codec + dispatchers to convert mailbox envelopes to
existing round actor messages and vice versa.
- OOR mailbox wire protocol:
- add `oorwire` payload schema/serialization for submit/finalize package RPCs.
- add mailbox outbox handler for OOR FSM side effects (submit, finalize,
checkpoint-signing follow-through, and local spend marking).
- Connector/runtime wiring:
- extend `serverconn` actor/runtime dispatch integration so inbound mailbox
round notifications are routed into the client round actor cleanly.
- update `darepod/server` dispatch map construction for mailbox request/event
handling through the existing actor system.
- Toy SDK layer for e2e demonstration:
- add `sdk/client.go` with a high-level API for round output request/join,
recipient address generation, OOR send, incoming sync/materialization, and
live balance/VTXO listing.
- hide mailbox envelope plumbing from SDK consumers; keep advanced knobs at
config boundaries.
- Harness CI stability hardening:
- extend container startup retry behavior to also retry transient image pull
failures (e.g. registry "unknown blob") in addition to port bind conflicts.
- add targeted unit tests for image-pull error detection and retry behavior.
- Tests:
- add SDK receive address tests and update round outbox tests to cover new
mailbox payload encode/decode paths.
Notes:
- This is intentionally an incomplete/"toy" SDK path aimed at validating e2e
plumbing shape.
- Includes lint-driven cleanup in touched files.
19c8951 to
3c58148
Compare
| return err | ||
| } | ||
|
|
||
| startResp := c.oorActor.Receive( |
There was a problem hiding this comment.
This should send the message rather than calling Receive directly (so hook up the actual actor system).
| } | ||
|
|
||
| rpcClient := h.oorRPCClient() | ||
| resp, err := rpcClient.FinalizePackage(ctx, req) |
There was a problem hiding this comment.
Perhaps this should purely go through serverconn?
| } | ||
|
|
||
| message JoinRoundAuthPayload { | ||
| string message_hex = 1; |
There was a problem hiding this comment.
If we just make this bytes, then we don't need the hex encode/decode portions in oorwire or roundwire. Then we just have that map to/from protos.
|
Great example here of hooking everything up, will incorporate the commits into #142. |
|
Integrated more of the commits from this PR into: #144 Main thing remaining is the sdk client wrapper itself. |
|
Closing this as it is now superseded by production plumbing. |
2 participants
Summary
This draft introduces a toy SDK + mailbox plumbing path in
darepo-clientthat is sufficient to drive a real end-to-end demo in systest.The intent is to prove integration shape (round join + OOR send + incoming sync) with real mailbox transport and actor wiring, not to claim production-complete SDK semantics.
Detailed Changes
1) Mailbox wire payload layer for round/OOR
roundwire).oorwire).2) Runtime dispatch and actor bridging
SubmitPackage/FinalizePackage).serverconn/darepoddispatch integration to route inbound round notifications into the local actor system.3) Toy SDK surface for e2e plumbing
sdk/client.gowith a high-level API for:4) Tests
sdk/address_test.go).Why Draft / Incomplete
This branch intentionally optimizes for proving full plumbing quickly. It is not yet the final API/contract boundary.
Missing For Completeness
Builds On
Validation
make lintgo test ./sdk