Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds RHP4 shared-balance pool support end-to-end (host RPC handlers + renter helpers) and updates contractor semantics so debits can fall through from an account’s own balance to attached pools.
Changes:
- Extend the RHP4
Contractorinterface with pool operations and update the test contractor implementation to support pools + attachment-ordered draining. - Add host-side RPC handlers for funding/replenishing pools and attaching/detaching pools; add renter-side helper functions/types for these RPCs.
- Add comprehensive RPC tests for pool funding, attachment authorization, drain ordering, and pooled-backed read/write flows.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
testutil/host.go |
Implements pool balance tracking, attachments, and updated debit semantics in the ephemeral contractor. |
rhp/v4/server.go |
Extends the server’s Contractor interface and wires new pool RPC handlers into dispatch. |
rhp/v4/rpc.go |
Adds renter helper APIs/results for pool fund/replenish/attach/detach RPCs. |
rhp/v4/rpc_test.go |
Adds new pool-related integration tests covering funding, auth, draining order, and pooled-backed RPC costs. |
go.mod / go.sum |
Bumps go.sia.tech/core dependency to a newer pseudo-version. |
.changeset/add_rhp4_pool_rpc_handlers_and_renter_helpers.md |
Documents the breaking/major change via changeset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Pass host pubkey through to PoolAttachment/PoolDetachment ValidSignature on the server and SigHash on the renter (binds signatures to the host and adds a domain separator between attach and detach via the RPC ID). - Fix EphemeralContractor.DebitAccount partial-drain bug: pre-check combined drawable balance before mutating; ErrNotEnoughFunds now leaves account and pool balances untouched. Adds regression test. - Add a cross-domain replay test asserting an attach signature does not validate as a detach authorization. - Drop a stale comment in TestPoolBatchAttachDetach.
peterjan
left a comment
peterjan
left a comment
There was a problem hiding this comment.
Left two small nits but otherwise LGTM
| return RPCFundPoolsResult{}, err | ||
| } | ||
|
|
||
| if len(resp.Balances) != len(deposits) { |
There was a problem hiding this comment.
nit should we add a similar length check in RPCReplenishPools?
Hosts can now back accounts with shared balance pools so renters don't need to pre-fund and continually rebalance every account. A renter funds a pool once and attaches as many accounts as they want; debits drain the account's own balance first and fall through to attached pools, which keeps per-account allowances small and reduces the total capital sitting idle in account balances. This is the host-side companion to the new RHP4 pool RPCs.