unroller: client-side VTXO unilateral exit via on-chain tree unrolling - #183
unroller: client-side VTXO unilateral exit via on-chain tree unrolling#183ellemouton wants to merge 11 commits into
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 introduces a critical feature enabling clients to unilaterally reclaim their Virtual Transaction Outputs (VTXOs) by broadcasting the associated transaction trees directly onto the Bitcoin blockchain. This mechanism ensures that users can exit the Ark protocol either by choice or necessity (e.g., VTXO expiry), providing a robust and trust-minimized way to make their funds spendable on-chain after a specified time delay. The changes involve significant updates to transaction broadcasting, wallet integration, and daemon logic to support this complex multi-transaction process. 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 new feature: client-side unilateral exit for VTXOs via on-chain tree unrolling. The implementation is comprehensive, touching many parts of the system from the chain backend and source, to new RPCs and CLI commands, and a new unroller actor to manage the process. The architecture is well-thought-out, with good separation of concerns.
I've identified a few issues, including a critical bug in the ExitVTXO RPC handler that causes it to only process one outpoint while reporting success for all, a high-severity bug in the CPFP logic that could lead to a loss of funds, and a potential panic during crash recovery. I've provided detailed comments and suggestions for these issues. Overall, this is a solid implementation of a complex feature.
| } | ||
|
|
||
| func (m *ExitVTXORequest) Reset() {} | ||
| func (m *ExitVTXORequest) String() string { return m.Outpoints[0] } |
There was a problem hiding this comment.
The String() method for ExitVTXORequest will panic if m.Outpoints is empty. While callers might check for an empty slice, String() methods should generally be safe from causing panics.
| func (m *ExitVTXORequest) String() string { return m.Outpoints[0] } | |
| func (m *ExitVTXORequest) String() string { | |
| if len(m.Outpoints) == 0 { | |
| return "" | |
| } | |
| return m.Outpoints[0] | |
| } |
There was a problem hiding this comment.
~cc Fixed — exit_vtxo.go is now deleted entirely. Types are proper protoc-generated code via make rpc. The generated String() uses protobuf reflection and is safe.
| // lwwallet doesn't expose lndclient.WalletKit, so | ||
| // WalletKit stays nil. The unroller falls back to | ||
| // direct broadcast (no CPFP), which works on regtest. | ||
| log.WarnS(ctx, "Unroller using direct broadcast "+ | ||
| "(no CPFP) in lwwallet mode", nil) |
There was a problem hiding this comment.
The comment here is misleading. It states that the unroller falls back to "direct broadcast (no CPFP)" in lwwallet mode. However, the implementation in lwwallet/chain_backend.go does perform CPFP by automatically constructing a child transaction. The comment should be updated to reflect the actual behavior.
| // lwwallet doesn't expose lndclient.WalletKit, so | |
| // WalletKit stays nil. The unroller falls back to | |
| // direct broadcast (no CPFP), which works on regtest. | |
| log.WarnS(ctx, "Unroller using direct broadcast "+ | |
| "(no CPFP) in lwwallet mode", nil) | |
| // lwwallet doesn't expose lndclient.WalletKit, so | |
| // WalletKit stays nil. The unroller falls back to the | |
| // chain backend's automatic CPFP child construction. | |
| log.WarnS(ctx, "Unroller using chain backend's auto-CPFP "+ | |
| "in lwwallet mode", nil) |
There was a problem hiding this comment.
~cc Fixed — comment now reads "chain backend's auto-CPFP" instead of "direct broadcast (no CPFP)".
7d50817 to
5cc25a9
Compare
| @@ -1,19 +1,28 @@ | |||
| package lwwallet | |||
There was a problem hiding this comment.
I don't think this commit is what we want as we don't want the lwwallet to require a bitcoind connection. afaik esplora has submitpackage available?
There was a problem hiding this comment.
There was a problem hiding this comment.
yeah cool - will update today. this was v1 just getting local stuff working :)
There was a problem hiding this comment.
~cc Done — refactored to use esplora /txs/package endpoint. No bitcoind dependency. BitcoindRPCURL config removed entirely. Tested end-to-end on regtest with mempool/electrs.
There was a problem hiding this comment.
~cc Thanks for the reference — used the same pattern. EsploraClient.SubmitPackage now POSTs to /txs/package with JSON array of hex txs, matching the ark-og SDK approach.
|
I think a topic for discussion should be if we want to auto unroll from the client side (e.g. on vtxo expiry). I'm on the no side, as this occurs cost to the client and the operator itself. |
d374d8f to
9fdb823
Compare
yeah - i think we can remove the ability later though if we agree? just added it so we can test nicely & deterministically. just during this "push until all basic flows are in place" |
aa21bfe to
b33d54c
Compare
Roasbeef
left a comment
Roasbeef
left a comment
There was a problem hiding this comment.
We should make a tracking issue to make this re-org aware.
| // persisted client trees contain valid signatures for | ||
| // unilateral exit (unrolling). | ||
| for _, clientTree := range s.ClientTrees { | ||
| if err := clientTree.SubmitTreeSigs( |
There was a problem hiding this comment.
Is this a bug fix? Should we have a test for this?
There was a problem hiding this comment.
Ah I see, we need them for unrolling.
There was a problem hiding this comment.
~cc Agreed a focused unit test would be good — will track as follow-up.
| CREATE TABLE IF NOT EXISTS unrolls ( | ||
| vtxo_outpoint_hash BLOB NOT NULL, | ||
| vtxo_outpoint_index INTEGER NOT NULL, | ||
| status INTEGER NOT NULL DEFAULT 0, |
There was a problem hiding this comment.
~cc Other tables in the project (vtxos, rounds) use raw ints for status. Added a status values comment in the migration instead. Happy to add an enum table if preferred.
| vtxo_outpoint_hash BLOB NOT NULL, | ||
| vtxo_outpoint_index INTEGER NOT NULL, | ||
| status INTEGER NOT NULL DEFAULT 0, | ||
| current_level INTEGER NOT NULL DEFAULT 0, |
There was a problem hiding this comment.
~cc Derived from the VTXO tree on load via extractLevelOrder. Not persisting avoids staleness — the tree is the source of truth.
There was a problem hiding this comment.
~cc Good point — for single-commitment trees the level count is immutable and re-derived on load. But with cross-batch ancestry (#199), the tree could span multiple commitment txs and the depth at unroll time may differ from what we re-derive later. Added a TODO to persist total_levels as a sanity check. Tracking alongside #199 since it only matters once we support multi-commitment unrolling.
| vtxo_outpoint_index INTEGER NOT NULL, | ||
| status INTEGER NOT NULL DEFAULT 0, | ||
| current_level INTEGER NOT NULL DEFAULT 0, | ||
| leaf_confirm_height INTEGER NOT NULL DEFAULT 0, |
There was a problem hiding this comment.
Doesn't default zero defeat the purpose of not null?
There was a problem hiding this comment.
~cc Removed all DEFAULT 0 from the migration. Code always provides values explicitly.
| @@ -0,0 +1,14 @@ | |||
| -- Unroll store: tracks in-progress VTXO tree unrolls so that the unroller | |||
| -- can resume broadcasting from where it left off after daemon restart. | |||
| CREATE TABLE IF NOT EXISTS unrolls ( | |||
There was a problem hiding this comment.
Txid's of the unroll aren't relevant? Or we already store that in the vtxt tree (tlv encoded rn?).
There was a problem hiding this comment.
~cc Txids are derived from the tree nodes (TLV-encoded in the VTXO store). BroadcastTxids are now rebuilt on restart from LevelOrder (see fix for the "not derived on restart" comment).
|
|
||
| // newWalletFundingAddressCmd creates the wallet funding-address | ||
| // subcommand for generating a plain on-chain P2TR address. | ||
| func newWalletFundingAddressCmd() *cobra.Command { |
There was a problem hiding this comment.
Should also update the schema command as well. i thought i had a ci check for that but maybe it's not working for some reason
There was a problem hiding this comment.
~cc Added wallet.funding-address and wallet.exit entries to schema_registry.go.
|
|
||
| txid := levelTxids.Txids[i] | ||
|
|
||
| signedTx, err := node.ToSignedTx() |
There was a problem hiding this comment.
Where does the fee bump actually take place?
Also I think we can use lnd's fee function here.
There was a problem hiding this comment.
~cc Fee bumping now properly rebuilds an explicit CPFP child in WalletKit mode with a bumped fee rate (2x multiplier via FeeMultiplier). In lwwallet mode, Child=nil lets the chain backend auto-build a fresh child. Added clarifying comment explaining the V3 package RBF mechanism.
| // etc.) in the `unrolls` table. | ||
| // | ||
| // Complex derived fields (LevelOrder, BroadcastTxids, ConfirmedTxids) | ||
| // are NOT persisted. They are re-derived from the VTXO tree on load |
There was a problem hiding this comment.
Don't see these derived on restart.
There was a problem hiding this comment.
~cc Start() now fetches the VTXO descriptor, re-derives LevelOrder via extractLevelOrder, and rebuilds BroadcastTxids from all levels 0..CurrentLevel.
|
|
||
| // CurrentFeeRate is the fee rate (sat/vB) used in the most | ||
| // recent broadcast. Used to calculate the next bump rate. | ||
| CurrentFeeRate int64 |
There was a problem hiding this comment.
Doesn't look like we persist this and last broadcast height.
There was a problem hiding this comment.
~cc Added last_broadcast_height and current_fee_rate columns to migration, sqlc queries, and store save/update/load paths.
| // new package if the total fee is higher. | ||
| submitReq := &chainsource.SubmitPackageRequest{ | ||
| Parents: []*wire.MsgTx{signedTx}, | ||
| Child: nil, |
There was a problem hiding this comment.
So this is bumping by RBF'ing the parent?
There was a problem hiding this comment.
~cc Not RBF on the parent — it's V3 package RBF via a replacement CPFP child. The same presigned parent is resubmitted with a new child that pays a higher total package fee. Added comment clarifying this.
9335cb4 to
b94c79f
Compare
b94c79f to
a07453e
Compare
586effe to
df166e0
Compare
The LND TLS cert and macaroon paths are needed by consumers that configure arkd to connect to the harness-managed LND instance. Expose them via read-only accessor methods.
Add atomic parent+child transaction package submission support to the ChainBackend interface. Required for broadcasting V3 transactions with ephemeral anchors via bitcoind's submitpackage RPC. - Add SubmitPackage method to ChainBackend interface - Add SubmitPackageRequest/Response message types - Add handleSubmitPackage handler in ChainSourceActor - Update test mocks to satisfy interface
Add PackageSubmitter interface and SubmitPackage implementation to the LND chain backend for atomic parent+child transaction package submission. Validates per-transaction results and provides detailed error diagnostics.
After SubmitTreeSigs applies aggregated MuSig2 signatures to VTXOTreePaths, propagate them to each ClientTree and verify cryptographic validity via VerifySigned(). ClientTrees are separate node copies from ExtractPathForCoSigners that don't share memory with VTXOTreePaths. Uses failWithNotification to emit RoundFailedNotification on signature propagation or verification failure, ensuring wallet/manager actors are notified.
Add SQLite persistence for in-progress VTXO tree unrolls so they survive daemon restart. The unrolls table tracks status, current level, leaf confirmation height, error message, and retry count. Complex derived fields (LevelOrder, BroadcastTxids, ConfirmedTxids) are re-derived from the VTXO tree on load via extractLevelOrder. - Add migration 000006_unroll_store - Add sqlc query definitions and generated code - Implement UnrollPersistenceStore with direct SQL queries - Delegate GetVTXO to VTXOPersistenceStore for tree path access - Add 9 unit tests covering CRUD, status filtering, and persistence - Bump LatestMigrationVersion to 6
Add the unroller package for on-chain broadcasting of presigned VTXO transaction trees during unilateral exit. The UnrollerActor coordinates level-by-level tree broadcast with V3 package relay, tracks confirmation progress, and monitors CSV delay completion. Key components: - UnrollerActor with crash recovery via persistent store - broadcastLevel: CPFP package relay with fee rate cap (500 sat/vB) - broadcastLevelDirect: fallback via chain backend auto-CPFP - findAnchorOutput: P2A script scan (not positional assumption) - extractLevelOrder: BFS traversal with sanity checks - Per-level UTXO tracking to prevent double-spend - Tell error logging for confirmation/block registrations - OOR VTXO guard with clear error message - Unit tests for BFS, status, actor request handling Based on the unroller design by sputnik (PR #98).
Add SubmitPackage support to the lwwallet/esplora chain backend using the esplora /txs/package HTTP API. When a nil child is provided and a CPFPWallet is configured, automatically constructs a CPFP child: - Scans for P2A anchor via scripts.AnchorPkScript - Estimates fee with 500 sat/vB cap and 1 sat/vB fallback - Selects smallest sufficient wallet UTXO with exclude set - Builds V3 child with anchor + wallet UTXO inputs - Signs via ComputeInputScript with proper PrevOutputFetcher - Rejects if total fee exceeds VTXO value (sanity check) - Thread-safe wallet access via mutex Also adds CPFPWallet interface, SetWallet lifecycle method, and EsploraClient.SubmitPackage for /txs/package endpoint.
Integrate the unroller into the daemon startup sequence with persistent SQLite storage and chain resolver for automatic expiry-triggered exit. - Add initUnrollerActor using SQLite-backed UnrollPersistenceStore - Create chainResolverAdapter bridging vtxo.ExpiringNotification to unroller.UnrollRequest via outpoint mapping - Wire ChainResolver into vtxo.ManagerConfig in initVTXOManager - Unit tests for chain resolver adapter
7c0ecbb to
020c4e5
Compare
Add two new RPCs to the daemon service: - ExitVTXO: initiates unilateral exit for specified VTXOs by sending UnrollRequest to the unroller actor. Validates VTXO status (must be live) before initiating. Guards FundingAddress against panic in LND mode. - FundingAddress: returns a plain BIP-86 P2TR address from the internal btcwallet for fee-funding UTXOs (lwwallet mode only). CLI commands: - darepocli wallet exit [outpoint...] — trigger unilateral exit - darepocli wallet funding-address — get on-chain fee funding address Includes protoc-generated code for both RPCs.
When tree transactions don't confirm within BumpAfterBlocks (default 6 blocks), the unroller automatically resubmits the package with a fresh CPFP child. The chain backend builds a new child using the current fee estimate, which V3 package RBF accepts if the total fee is higher than the previous child. - Add feeBumpLevel method: resubmits current level's parent txs with Child: nil so chain backend auto-constructs higher-fee child - Monitor Broadcasting status in handleBlockEpoch for bump triggers - Subscribe to block epochs during broadcasting (not just CSV wait) - Track LastBroadcastHeight and CurrentFeeRate in UnrollState - Max 10 bump retries before permanent failure - Best-effort: individual bump errors are logged, not fatal - Resume bump monitoring after crash recovery
After the unroller completes tree unrolling and the CSV delay is satisfied, automatically construct and broadcast a sweep transaction that spends the VTXO via the timeout script path back to the user's on-chain wallet. The sweep transaction: - Version 2 (no anchor needed, pays its own fees) - Input: VTXO outpoint with sequence = CSV delay - Witness: <sig> <timeout_script> <control_block> - Output: fresh wallet P2TR address minus estimated fee This completes the full unilateral exit flow per ARK-05: Board → Round → VTXO created → Exit triggered (voluntary or expiry) → Tree unrolled on-chain (CPFP + fee bumping) → CSV delay satisfied → VTXO swept to wallet ← NEW Wiring: - LND mode: Signer from lndbackend.ClientWallet, SweepAddress from lndSvc.WalletKit.NextAddr - lwwallet mode: Signer from btcwallet, SweepAddress from lwwallet.NewAddress Sweep failures are logged but do not fail the unroll — the VTXO remains on-chain and can be swept manually.
020c4e5 to
e4d6e7f
Compare
…sts-part-2 Part 2: real-daemon integration tests (durability + indexer)
|
@sputn1ck: review reminder |
|
replaced by #235 |
5 participants
Summary
Implements client-side unilateral exit (on-chain tree unrolling) for VTXOs. When a client needs to exit the Ark protocol — either voluntarily or because a VTXO is approaching expiry — this broadcasts the presigned VTXO transaction tree on-chain, waits for the CSV delay, and sweeps the funds back to the client's on-chain wallet.
Key changes
chainsource/chainbackends: SubmitPackage interface — adds atomic parent+child package submission for V3 transactions with ephemeral anchors (PR #98 by @sputn1ck)round: signature propagation + verification — ensures extracted client trees receive aggregated MuSig2 signatures and verifies them cryptographically viaVerifySigned()(PR #98 by @sputn1ck)db: persistent SQLite UnrollStore — migration 000006, sqlc-generated queries viaBatchedRoundStore+ExecTx, crash-safe persistence of unroll state (status, level, confirmation heights, fee rate, broadcast height)unroller: VTXO tree unroller actor — BFS level-by-level broadcast with CPFP package relay, confirmation tracking, CSV delay monitoring, automatic VTXO sweep after CSV, fee bumping via CPFP child replacement, crash recovery with state re-derivation from persistent store (PR #98 by @sputn1ck)lwwallet: SubmitPackage via esplora — enables tree unrolling in lwwallet mode using the esplora/txs/packageHTTP API with automatic CPFP child construction from wallet UTXOs (no bitcoind dependency)darepod: unroller wiring + chain resolver — integrates unroller into daemon startup with SQLite store, bridgesvtxo.ExpiringNotification→unroller.UnrollRequestfor automatic expiry-triggered exitdaemonrpc/CLI: ExitVTXO + FundingAddress RPCs —darepocli wallet exit <outpoint>for voluntary exit with VTXO status validation,darepocli wallet funding-addressfor on-chain fee UTXO funding, schema registry updatedArchitecture
Fee bumping (when broadcasts don't confirm within
BumpAfterBlocks):Tested on regtest
Full end-to-end with mempool/electrs (esplora
/txs/package):darepocli wallet funding-address→ fund wallet UTXO for CPFP feesdarepocli wallet exit <outpoint>→ tree broadcast via esplora package submissionCompanion server PR
lightninglabs/darepo#161 — adds 8 system tests exercising the unroller with real LND + bitcoind backends, including restart recovery and CSV completion tests.
Known limitations / follow-up work
Credits
Core unroller design and SubmitPackage infrastructure ported from PR #98 by @sputn1ck. Commits attributed accordingly.
Test plan
go build ./cmd/darepod/ ./cmd/darepocli/— both binaries compilemake fmt-check— formatting cleanmake lint— linter cleanmake sqlc-check— SQL models up to date