[v0.1.x-branch] Backport #987: multi: Preview refresh fees and gate refresh on consent - #1003
Merged
Merged
Conversation
In this commit, we add a RefreshFeeEstimate message to RefreshVTXOsResponse so a dry_run refresh can carry an advisory, itemized operator-fee preview for the selected VTXOs. Today the refresh path never surfaces the fee anywhere: the binding amount is set by the server-issued JoinRoundQuote at seal time and auto-accepted against the client's MaxOperatorFeeSat cap, so the user only learns the charge afterwards via the fee history. The per-outpoint rows carry the amount and remaining lifetime the daemon resolved for each VTXO (removing the manual amount / remaining-blocks input the standalone fees estimate command requires) plus the operator's liquidity / on-chain share / margin breakdown. Rows are deliberately NOT waiver-adjusted so their components always sum to their total; the selection-level free_refresh_eligible flag and zeroed total express the free-late-refresh waiver instead, mirroring the operator's all-or-nothing seal-time rule from lumos#675. estimate_error keeps dry_run usable as a validity probe when the operator or chain height is unavailable: the preview still returns and the caller is told the numbers are absent rather than zero. The daemon-side population of the new field lands in the next commit. Part of #986. (cherry picked from commit f532418)
In this commit, we populate the new RefreshVTXOsResponse fee estimate on the dry-run path. The daemon resolves every selected VTXO to its full descriptor, computes each remaining lifetime from the chain tip, and fetches the operator's itemized quote through the existing EstimateFee proxy, deduped on (amount, remaining blocks). The remaining-blocks figure is clamped to 1 for expiring VTXOs because the operator treats zero as "price the full sweep-delay lifetime". The explicit-outpoint path now looks targets up in the VTXO store instead of only parsing strings: an unknown or non-live outpoint surfaces as InvalidArgument instead of echoing back as a plausible preview, mirroring the LiveState filter the --all path already applies — the real refresh can never execute either, and dry_run is the validity probe the CLI consent prompt trusts. Explicit selections are also deduped (order-preserving) so a repeated outpoint can neither double-count in the estimate nor register a doomed duplicate forfeit pair with the wallet. The free-late-refresh waiver (lumos#675) is applied locally: the operator's EstimateFee prices every refresh as paid, so quoting a free late refresh through it alone would over-quote. The daemon already caches the advertised window in its operator terms and knows a refresh selection is the pure one-for-one renewal shape the waiver requires, so a selection fully inside the window previews with an explicit zero total while the rows keep the ordinary paid quote to show what the waiver saves. The estimate is strictly best-effort and degrades all-or-nothing: every quote is fetched and vetted (no negative or beyond-money- supply values, no overflowing selection total) before any component is written to a row, an unreachable operator or chain backend sets estimate_error while the preview and the locally computed waiver verdict survive, and the total uses explicit proto presence — it is only set when meaningful, so a degraded estimate can never be misread as a free refresh. The dry-run branch also moves ahead of the wallet-ready gate, matching the LeaveVTXOs ordering rule that pure-argument validation and previews must not depend on wallet state (the H-5 fix's comment already claimed this parity). A side effect is that an empty selection=all dry run now honestly reports status "preview" instead of "queued". The real refresh path still gates on wallet readiness before queuing. Part of #986. (cherry picked from commit b5eefe7)
In this commit, we render the dry-run fee estimate on the refresh command. The itemized numbers stay in the JSON body on stdout; stderr gains a short human-readable summary so an operator reading the terminal sees the headline cost — or the free-refresh-window verdict, or the degraded-mode warning — without stdout consumers having to strip prose. Every wording branch repeats that the value is advisory and the binding fee is set at seal time, and the degraded branch explicitly says a fee still applies so a missing estimate is never read as a free refresh. VTXOs below the operator's minimum viable amount add a count-level warning, mirroring the fees estimate command's below-dust handling. A non-empty dry-run preview that carries no estimate at all can only come from a daemon predating the feature, so the CLI warns about that skew explicitly instead of silently dropping the preview the flag help promises. The fees estimate help now points refresh users at `ark vtxos refresh --dry_run`, which resolves each selected VTXO's amount and remaining lifetime automatically instead of requiring both by hand. Part of #986. (cherry picked from commit 1a25ab3)
In this commit, we stop dispatching a real refresh without consent. Before this change the number a dry run can now surface was not actionable: by the time a user read anything, the refresh was queued, auto-joined, and priced by the auto-accepted seal-time quote, with no cancel surface anywhere in between. The refresh command now mirrors the leave --all consent pattern: --yes skips the gate for scripted use, non-interactive stdin refuses to prompt and directs the caller to --yes or --dry_run (so agents are never blocked, per the issue's acceptance criteria), and only an interactive TTY prompts. The interactive prompt shows the advisory estimate first — fetched through the same RPC in dry-run form — so the operator consents to a number rather than a mystery fee, and the prompt goes to stderr: stdout stays reserved for the JSON body, so a piped invocation can never swallow the question and read as a hung command. A preview rejected as InvalidArgument aborts outright (the real dispatch would reject the same request shape); any other preview failure degrades to prompting with an explicit "still charged the seal-time fee" warning, so a broken estimate path never makes refreshes unconfirmable and a missing estimate is never read as free. An empty selection (refresh --all with no live VTXOs) skips the prompt entirely — there is no fee to consent to, and warning about a charge for a no-op would be false. The schema registry entry gains the yes parameter and now names the fee in its description, so schema consumers see the same contract as the flag surface. getDaemonClient becomes a package-level indirection (mirroring stdinIsTTY) so wiring tests can drive the full command path against an in-process bufconn daemon and pin that the gate runs before any dispatch: a refused invocation must reach the daemon zero times. Part of #986. (cherry picked from commit 4f4cf8c)
In this commit, we move the leave --all confirmation prompt from stdout to stderr, matching the refresh confirmation gate and the package's stream-split invariant: stdout is reserved for the JSON body, diagnostics go to stderr. Before this change, piping the command (e.g. into jq) on an interactive terminal swallowed the prompt into the pipe — the command read as hung, and a user who typed y blind fed prose into the JSON consumer. Part of #986. (cherry picked from commit 35ab417)
In this commit, we extend the refresh fee-consent contract to the MCP surface. The MCP tool calls the daemon directly rather than executing the cobra command, so the CLI gate never ran there: an agent could queue a fee-incurring refresh with no warning, while the schema registry — documented as the shared source of truth for CLI commands and MCP tools — promised that a real refresh requires yes. The tool description also never mentioned the fee. The ark.vtxos.refresh tool now takes a yes acknowledgement: a real (non-dry-run) call without it returns an immediate, actionable error naming both the dry_run:true preview and the yes:true acknowledgement path — nothing can block on MCP, matching the CLI's non-interactive refusal — and the description now names the seal-time fee and the preview-first flow. The dry-run preview carries the same itemized fee estimate as every other surface. Part of #986. (cherry picked from commit c503b62)
In this commit, we document the refresh-fee visibility surface. The CLI guide's refresh section now names the operator fee and the seal-time quote that binds it, describes the dry-run estimate (per-outpoint amounts and lifetimes resolved by the daemon, the free-refresh-window verdict, the degraded estimate_error mode), and covers the interactive confirmation plus the non-interactive --yes requirement, with examples updated to match. The waveclicommands agent docs gain the refresh consent-gate invariant — CLI prompt, non-interactive refusal, the MCP tool's yes argument, and the absent-not-zero wire semantics of a degraded total — alongside the existing leave --all and recovery escalate prompt-refusal postures. The waved agent docs gain the dry-run estimation invariant (pre-wallet-gate ordering, store-resolved live-only outpoints, quote dedupe and validation, locally computed waiver, degrade-only failure mode). The repo's own agent-facing refresh examples in the waved skill and CONTEXT.md pick up --dry_run/--yes so an agent following them is not stopped cold by the new non-interactive refusal. Both members of each touched CLAUDE/AGENTS pair receive the same edit; the waved pair's pre-existing divergence on unrelated OOR text is left to the doc-gardening workflow. Part of #986. (cherry picked from commit 021094c)
In this commit, we extend the refresh quote sanity check to enforce the reconciliation the proto documents: OutpointFeeEstimate promises its components always sum to the row's total_fee_sat, and the operator's fee schedule defines the total as exactly liquidity + on-chain share + margin, but refreshQuoteSane only vetted each field for negative or beyond-money-supply values. A broken (or hostile) operator could return an itemization that contradicts its own total and the daemon would echo the inconsistency into the advisory rows. Each component is now individually bounded by MaxSatoshi before the wrap-free sum is compared against total_fee_sat, so a mismatched quote degrades the estimate all-or-nothing exactly like a negative one. Raised in the automated review of #987. Part of #986. (cherry picked from commit bb82577)
In this commit, we extract the refresh selection parsing into a shared buildRefreshVTXOsRequest builder, mirroring the buildLeaveVTXOsRequest pattern, and add the mutual-exclusion guard the leave surface already has: passing both --outpoint and --all previously let --all win silently, refreshing (and charging for) every live VTXO while the named outpoints were dropped. The MCP tool now goes through the same builder, closing the same silent-drop gap on that surface and turning an empty selection into an immediate client-side error instead of a daemon round trip. Raised in the automated review of #987. Part of #986. (cherry picked from commit 5551d2f)
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
2 participants
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.
Backport of #987
In this PR, we fix #986, where
ark vtxos refreshqueued a fee-incurring refresh with no estimate, warning, or consent anywhere in the flow — the binding seal-time quote is auto-accepted againstmaxoperatorfeesat, so the user only learned the charge afterwards via the fee history.A dry-run refresh now returns an itemized advisory fee estimate for the selected VTXOs. The daemon resolves each target's amount and remaining lifetime from its own store and chain tip (no more manual
--amount/--remaining-blocksinput), fetches the operator's liquidity / on-chain share / margin breakdown through the existingEstimateFeeproxy (deduped and validated per quote), and applies the free-late-refresh waiver locally — the operator'sEstimateFeeprices every refresh as paid, so a selection fully inside the advertised window previews with an explicit zero total while the per-outpoint rows keep the ordinary paid quote to show what the waiver saves (all-or-nothing, mirroring the seal-timeisFreeRefreshrule from lightninglabs/lumos#675). Estimate failures degrade all-or-nothing to anestimate_errorfield rather than failing the preview, and the total uses explicit proto presence so a degraded estimate is absent on the wire, never a zero an agent could misread as free.A real refresh is now gated on consent on every surface. The CLI mirrors the
leave --allpattern: an interactive TTY shows the estimate and prompts (on stderr, preserving the stdout-JSON stream split — the pre-existing leave prompt moves to stderr in its own commit for symmetry),--yesskips the gate for scripted use, and non-interactive stdin refuses with an actionableINVALID_ARGSerror instead of blocking on a prompt an agent cannot answer. The MCP tool — which calls the daemon directly and never ran the CLI gate — enforces the same contract through a newyesargument with an immediate, actionable error, and its description now names the fee and the preview-first flow, matching what the schema registry promises.The dry-run path also becomes an honest validity probe: it moves ahead of the wallet-ready gate for parity with the
LeaveVTXOsH-5 ordering (whose comment already claimed this symmetry), unknown and non-live explicit outpoints now failInvalidArgumentinstead of echoing back as a plausible preview, repeated outpoints are collapsed so they can neither double-count in the estimate nor register doomed duplicate forfeit pairs, and an empty--allpreview honestly reports"preview". The real refresh path is unchanged beyond the shared selection canonicalization, and a new wallet-actor test pins its--allexpansion.Downstream surfaces are unaffected:
swapdk-serverreads onlyqueued_outpoints+statusfrom refresh responses, and the SDK passes the proto through verbatim. Version skew is handled in both directions — a new CLI against a pre-feature daemon warns that no estimate came back, and old clients simply ignore the new fields. A follow-up issue against lumos for a waiver-aware / outpoint-keyed server-side estimate will be filed once this lands.See each commit message for a detailed description of the individual changes.
Validation
make unit pkg=./waved(also withtags="test_sqlite")make unit pkg=./cmd/wavecli/waveclicommandsmake unit pkg=./waverpcgo test -race ./waved/ -run TestRefresh -count=1andgo test -race ./cmd/wavecli/waveclicommands/ -count=1make fmt-changed-check && make tidy-module-check && make sample-conf-checkmake lint-changed-local(0 issues)make commitmsg-lint range="origin/main..HEAD"make rpc(regenerated output committed; tree clean on re-run)--help,schema ark.vtxos.refresh, non-TTY refusal exits 2 with theINVALID_ARGSenvelope before any RPC reaches the daemon,--yesproceeds straight to dispatchfix #986