From 4f33104c545000d171c3784267e06f49d8a07e01 Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 8 Oct 2026 18:28:47 +0200 Subject: [PATCH 1/2] fix(tx): a send is sent only once the node has it, and a released one is never final hsd 8.0.0 answers sendrawtransaction with the txid whatever its mempool does with the transaction, so a send whose coins were already spent, or one hsd refused for any other reason, read as sent. This implements docs/specs/2026-10-07-honest-broadcast-result.md: - R1-R3: after sendrawtransaction the wallet looks the transaction up up to five times, 400 ms apart; the first find ends the window. hsd's not-found on every check, with no output of the transaction mined, is "not taken": the broadcast errors "The node did not take the transaction", the draft is broadcast_pending with its coins held. An output already mined (a node without a transaction index) confirms it; a check without hsd's answer is no verdict. - R5: a Shakedex purchase goes through the same check, and so does its one automatic rebroadcast: not taken loses it with nothing paid, unless the chain traces to it. - R6: a broadcasted, confirmed, dropped or failed draft is not signed or broadcast again. Signing flipped any draft back to "signed", so a confirmed one could be turned back and a dropped one's released coins spent again. - R7: dropped and failed are no longer final. For 72 hours (hsd's mempool expiry) the confirmation poll keeps looking for such a transaction and confirms it once mined, so a payment that went through never keeps reading "not sent". The look-up and the chain evidence move to noncustodial::tx_evidence, which the purchase job may use (CODING_STANDARDS layering). Tests: unit and mock tests pin R1-R7 (each checked to fail with its rule removed); live regtest tests cover the double spend, the revived send on both nodes, a purchase and finalize paying their fee rate on vsize at 20 doos/byte, and a purchase rebroadcast refused after a price drop. Seven older live tests now fail because hsd refuses a transaction they build (bad-redeem-owner, bad-renewal-premature, bad-finalize-maturity, an orphan): they passed only because the broadcast never said so. They are fixed next. --- CHANGELOG.md | 3 + docs/REGTEST_TESTING.md | 15 +- docs/USER_MANUAL.md | 2 +- .../2026-10-07-honest-broadcast-result.md | 19 +- docs/specs/README.md | 1 + src-tauri/src/commands/tx.rs | 205 +++++----- src-tauri/src/db/queries.rs | 25 ++ src-tauri/src/noncustodial/mod.rs | 1 + src-tauri/src/noncustodial/tx_evidence.rs | 146 +++++++ src-tauri/src/shakedex_jobs.rs | 14 +- src-tauri/src/tests/live_node_it.rs | 369 ++++++++++++------ src-tauri/src/tests/mock_node_rpc.rs | 10 + .../src/tests/node_rpc_injected_tests.rs | 150 ++++++- .../tests/shakedex_purchase_state_tests.rs | 71 ++++ src-tauri/src/tests/tx_lifecycle_tests.rs | 165 +++++++- 15 files changed, 949 insertions(+), 247 deletions(-) create mode 100644 src-tauri/src/noncustodial/tx_evidence.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 96dc5d9..487ac11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ - **Remote-node onboarding** — the first-run flow now opens with a "How do you want to connect?" step offering three choices: **Local full node** (default; start hsd on this device), **Remote node** (point at an existing hsd RPC, with a "Test connection" button that probes the node before you commit), and **SPV** (lightweight headers-only, read-only). Choosing a source persists `chain_source` + `node_mode` (plus `node_rpc_url` / API key for Remote node) up front so a new user reaches a working read+send wallet without waiting for a full local sync. Your recovery phrase never leaves the device — remote/SPV is a privacy/trust tradeoff, not custody. New Tauri command `check_node_connection` validates a candidate RPC (reachable / height / synced, and whether the node's network matches the wallet's — during onboarding, the network you picked on the previous step; a cross-network node, e.g. testnet-for-mainnet, is flagged with an amber warning under the "Connected" line and is not treated as a usable node) without persisting anything, honoring the existing plaintext-key / non-loopback transport guard. Settings' Chain source selector now offers the same four modes (local full / SPV / remote / explorer) and replaces the separate Node mode dropdown; Settings also gained the same "Test connection" affordance and an "Allow sending via remote node" toggle (`allow_remote_broadcast`, off by default) — the toggle appears both on the onboarding Remote step and in Settings. ### Fixed +- **A send the node did not take says "Not sent".** hsd answers `sendrawtransaction` with the txid whatever its mempool does with the transaction, so a send whose coins were already spent, or that the mempool turned away for any other reason, read as sent and was quietly marked dropped ten minutes later. The wallet now looks the transaction up right after sending (up to five times over two seconds, since hsd adds it to its mempool after answering). When the node does not have it, the dialog says "The node did not take the transaction", the draft waits with its coins held, and the confirmation check resolves it later: dropped, saying another transaction spent its coins, when that is what the chain shows. A send already mined by the time it is looked up (a node without a transaction index finds it only by its outputs) reads as confirmed. A Shakedex purchase follows the same rule, and its one automatic rebroadcast too: a rebroadcast the node does not take loses the purchase with nothing paid. +- **A send already sent, dropped or failed is not signed or sent again.** Signing or broadcasting such a draft again would turn a confirmed one back into "signed", or spend coins a dropped or failed one had released, which another draft may hold by now. Both are refused before anything reaches the node; build a new send instead. +- **A send marked dropped or failed that is mined after all turns confirmed.** Dropped and failed were final, so a transaction another node still held, or that this node forgot when restarted, kept reading "not sent" once mined, inviting a second payment. The confirmation check now keeps looking for such a transaction for 72 hours (hsd's mempool expiry) and confirms it once it is mined. - **A sent transaction is no longer reported as "not sent" because the node keeps no transaction index.** hsd finds a transaction by its txid only in its mempool and, once mined, in its transaction index (`--index-tx`). On a remote node started with the address index alone, which is enough to send from, every mined transaction read as unknown: ten minutes later the wallet marked the send dropped (or failed) with "The coins were not moved" and released its coins, inviting you to pay again, although the payment had gone through. When hsd does not find a transaction, the wallet now asks the chain about the coins instead, which needs no transaction index: one of the transaction's own outputs mined is the block it is in, and it is confirmed at that height; every coin it spends still unspent is what "dropped, the coins were not moved" now rests on. When another transaction spent its coins, a node with the index drops it and says so ("Another transaction spent this transaction's coins … Nothing was paid by this transaction"); a node without the index cannot tell that apart from a mined transaction whose outputs were spent since, and the wallet leaves it as sent. - **Importing a listing says what is wrong.** A listing for another network now reads "this listing is for mainnet, but this wallet is on regtest" instead of "paymentAddr is not a regtest address"; a market link pasted into the listing-file box is imported as a link instead of failing as JSON; and a link to the listing's own file (`…/listing//proof.json`) is accepted like the listing's page. A node that refuses the API key now reads "the node refused the API key (HTTP 401): set the node's API key in Settings → Connections" everywhere, instead of "node returned non-JSON body (status 401 Unauthorized)". - **A freshly registered name can be updated, transferred and renewed again.** The wallet read "a transfer is pending" off the number of items in the name's covenant, and a REGISTER carries as many as a TRANSFER. So every name straight after registering was shown as waiting to finalize a transfer, with Update, Transfer and Renew refused "while a transfer is pending" and Finalize offered instead; since Update was refused, nothing changed that, and the name could not be renewed from the app. The pending transfer is now read from the covenant's type. diff --git a/docs/REGTEST_TESTING.md b/docs/REGTEST_TESTING.md index 8917ea7..ff2e235 100644 --- a/docs/REGTEST_TESTING.md +++ b/docs/REGTEST_TESTING.md @@ -279,8 +279,8 @@ This runs the following tests: | `live_send_max_sweeps_all_coins` | `max=true` sweeps every coin, no change, single output | | `live_send_immature_coinbase_rejected_then_matures` | Guards commit `a520456`: immature coinbase not spendable until `height+maturity ≤ tip+1` | | `live_send_wrong_network_address_rejected` | Mainnet `hs1q…` on regtest → rejected before signing | -| `live_send_broadcast_double_spend_releases_reservation` | A draft whose coin another mined draft spent never confirms (hsd still answers its txid). It does not check the draft's status; `live_coins_spent_by_another_tx_drop_the_draft_and_say_so` does | -| `live_send_rebroadcast_same_draft_rejected` | Second broadcast of a mined draft → node rejects (no RBF) | +| `live_send_double_spend_is_not_sent` | A draft whose coin another mined draft spent is not sent: hsd answers its txid, the look-up after it finds nothing, the broadcast errors "did not take" and the draft is `broadcast_pending`; it is never mined | +| `live_send_rebroadcast_same_draft_rejected` | A mined draft is not sent again: refused "already sent" before anything reaches the node, still `confirmed` | | `live_send_estimate_fee_smoke` | `estimate_tx_draft_fee` matches a real build's fee/change/inputs | | `live_send_txid_matches_node` | Local txid == node-returned txid (sighash/serialization guard) | @@ -303,7 +303,8 @@ These drive REAL reorgs via a `#[cfg(test)]` `invalidateblock`/`reconsiderblock` | `live_confirm_finality_ceiling` | ≥ `CONFIRMATION_FINALITY_DEPTH` confs → refresh stops churning | | `live_broadcast_pending_promotes` | `broadcast_pending` draft on-chain → refresh promotes via `local_txid_from_summary` | | `live_coinbase_reorg_immaturity` | Unmine a confirmed coinbase → wallet treats the re-mined one as freshly immature | -| `live_coins_spent_by_another_tx_drop_the_draft_and_say_so` | A sent draft whose coin another mined transaction spent is `dropped` after the grace window, saying another transaction spent its coins, never "the coins were not moved" | +| `live_coins_spent_by_another_tx_drop_the_draft_and_say_so` | A draft whose coin another mined transaction spent is not sent (`broadcast_pending`), then `dropped` after the grace window, saying another transaction spent its coins, never "the coins were not moved" | +| `live_dropped_send_mined_after_all_is_confirmed` | A send judged `dropped` (coins released) that is then mined is `confirmed` at its block by the next poll | ### A node without a transaction index (Group C, second node) @@ -322,7 +323,8 @@ HNS_IT_NOINDEX_NODE_URL=http://127.0.0.1:24037 HNS_IT_NOINDEX_NODE_API_KEY=test | `live_noindex_mined_send_is_confirmed_not_dropped` | A mined send is `confirmed` at its block after the grace window, and stays so on the next poll; never `dropped` | | `live_noindex_mined_pending_broadcast_is_confirmed_not_failed` | A mined `broadcast_pending` draft is `confirmed` with its txid; never `failed` | | `live_noindex_unsent_draft_with_unspent_coins_is_dropped` | A draft the node never had, its coins unspent, is `dropped` and its coins released | -| `live_noindex_coins_spent_by_another_tx_give_no_verdict` | A draft whose coin another transaction spent stays `broadcasted`: without the index the wallet cannot tell that from a mined one | +| `live_noindex_coins_spent_by_another_tx_give_no_verdict` | A draft whose coin another transaction spent is not sent and stays `broadcast_pending`: without the index the wallet cannot tell that from a mined one | +| `live_noindex_dropped_send_mined_after_all_is_confirmed` | A send judged `dropped` that is then mined is `confirmed`, found by its outputs | ### Covenant actions (Group D) @@ -390,9 +392,10 @@ What the tests check: - `shakedex_cli_listing_is_bought` — a fixed-price listing is bought: unconfirmed in the mempool, awaiting finalize once mined, owned after the lockup and the FINALIZE, with the node's owner coin paying the purchase's destination. - `shakedex_finalized_name_moved_on_before_a_sync_is_owned` — the purchase is finalized and the name then sent elsewhere (TRANSFER, lockup, FINALIZE) before the purchase job runs again; the job finds our FINALIZE as the TRANSFER's spender in the node's history of our destination and marks the purchase owned. The shakedex lock script lets the TRANSFER be spent only into a FINALIZE, so there is no other ending to test. -- `shakedex_cli_buyer_first_loses_ours_with_nothing_paid` — the CLI buys the listing between our review and our broadcast. hsd 8.0.0's `sendrawtransaction` answers with the txid whatever its mempool does with the transaction (here it is kept as an orphan, its lock coin already spent), so the broadcast reads as sent; the purchase job then loses it as bought by someone else, with nothing paid, and frees its coins. +- `shakedex_cli_buyer_first_loses_ours_with_nothing_paid` — the CLI buys the listing between our review and our broadcast. hsd 8.0.0's `sendrawtransaction` answers with the txid anyway; the look-up after it finds the node did not take the purchase, so the broadcast says "did not take" and the draft waits as `broadcast_pending`. The purchase job then finds the lock coin spent and loses the purchase with nothing paid, its coins free again, and the listing is no longer offered. - `shakedex_purchase_follows_reorgs_of_its_own_blocks` — the purchase's block is invalidated (hsd's `invalidateblock` empties the mempool too, so the purchase is gone from the node), the app's sync rebroadcasts it once after six blocks missing, and it is mined again; then the FINALIZE's block is invalidated and the purchase awaits finalize again until the same FINALIZE is mined. - `shakedex_reverse_auction_pays_the_current_step_and_refuses_a_stale_one` — a purchase signed at one step is refused at broadcast once the node's median time makes a cheaper step valid, and built again it pays that step. +- `shakedex_rebroadcast_after_a_price_drop_is_not_sent` — the purchase's block is invalidated, which leaves it nowhere; a cheaper step of the reverse auction becomes valid before its one rebroadcast is due, so the old price is not sent again: the purchase is lost with nothing paid, its coins free, and the node never sees it again. - `shakedex_cancelled_listing_is_not_offered` — after the seller cancels, the listing is "sold or cancelled" and cannot be bought. - `shakedex_listing_expiring_before_finalize_is_not_offered` — R9 at the boundary: the chain is mined until the name's expiry (hsd's own `renewalPeriodEnd`) is one block past the margin (tip + 1 + transfer lockup + one day, a day being the lockup on regtest), where the listing is buyable with the expiry warning; one block later it is "expires before it can be finalized" and building the purchase is refused. It mines about 5000 blocks, regtest's renewal window. @@ -405,7 +408,7 @@ Some listings the CLI cannot write: it publishes a market fee only through Learn - `shakedex_purchase_at_a_same_price_step_not_valid_is_not_sent` — two steps at one price, the purchase signed at the later; the blocks that moved the median time past it are taken back, so only the earlier step is valid, and the purchase is refused before it is sent. Checking the price alone sent it, and hsd answered with its txid although it could only hold it as non-final. - `shakedex_purchase_the_node_lost_is_given_up_after_mempool_expiry` — the purchase's block is invalidated (the node then has nothing of it); with the daemon's no-resend rule it waits, still unconfirmed one block before hsd's mempool expiry and lost with nothing paid at it, and nothing is sent again. -`live_send_pays_its_fee_rate_on_vsize` (no CLI needed) checks a send against the rate hsd itself reports for it, worked out on the virtual size. +`live_send_pays_its_fee_rate_on_vsize` (no CLI needed) checks a send against the rate hsd itself reports for it, worked out on the virtual size. `shakedex_purchase_and_finalize_pay_their_fee_rate_on_vsize` does the same for a purchase and its FINALIZE built at 20 doos/byte, above the 5 doos/byte floor every other test's rate falls under. The tests move the node's clock forward with `setmocktime` to make price steps valid. hsd keeps that as an offset that goes on ticking, and the median time never goes back, so the clock is only ever moved forward; never run `setmocktime 0` against this node, which sets its clock to 0 and stalls mining (`scripts/regtest.sh reset` starts over). diff --git a/docs/USER_MANUAL.md b/docs/USER_MANUAL.md index b230ac1..e474773 100644 --- a/docs/USER_MANUAL.md +++ b/docs/USER_MANUAL.md @@ -740,7 +740,7 @@ A freshly imported wallet won't show any HNS until: ### Send fails with "Not sent" -The dialog stays open with the exact error. Common causes: not enough HNS to cover amount + fee, or the node could not be reached mid-broadcast. A transaction the node does not accept still reads as sent (hsd answers with its txid either way) and is marked dropped once it is not seen. Fix the issue and click Sign & Broadcast again — the draft is still there. +The dialog stays open with the exact error. Common causes: not enough HNS to cover amount + fee, or the node could not be reached mid-broadcast. After sending, the wallet looks the transaction up on the node: if the node does not have it, the dialog says "The node did not take the transaction". hsd does not say why; most often its coins were already spent by another transaction. The coins stay held, and Activity shows the send as waiting until the node is checked again. Fix the issue and click Sign & Broadcast again — the draft is still there. A send already sent, or one marked dropped or failed (its coins were released), cannot be signed or sent again: build a new one. A send marked dropped or failed that is mined after all, within 72 hours, turns confirmed by itself. ### CSV import shows errors diff --git a/docs/specs/2026-10-07-honest-broadcast-result.md b/docs/specs/2026-10-07-honest-broadcast-result.md index ea46050..793d545 100644 --- a/docs/specs/2026-10-07-honest-broadcast-result.md +++ b/docs/specs/2026-10-07-honest-broadcast-result.md @@ -1,6 +1,6 @@ # Honest broadcast result -Status: specified, not implemented. Builds on Shakedex name sales (merged) and on the chain evidence the confirmation poll reads for hsd's not-found (`fix/no-tx-index-send-verdicts`, `commands/tx.rs::chain_evidence_with_client`), which merges first. Its row in `docs/specs/README.md` is added with the implementation. +Status: implemented. Builds on Shakedex name sales and on the chain evidence the confirmation poll reads for hsd's not-found (`noncustodial/tx_evidence.rs`). ## 1. Summary @@ -15,7 +15,7 @@ When the user sends a transaction, the wallet says "sent" only once the node has ## 3. Requirements **R1 — Sent means taken.** After `sendrawtransaction` returns a txid, the wallet checks the transaction with `getrawtransaction` up to 5 times, 400 ms apart (hsd adds the transaction to its mempool asynchronously after answering). The first check that finds it ends the window: the draft is `broadcasted` with that txid, as today. A check that gets hsd's not-found also reads the chain evidence for the draft; `Mined` ends the window too, the draft `confirmed` at that height. -*Enforced:* `commands/tx.rs::classify_broadcast_outcome_with_client`. +*Enforced:* `commands/tx.rs::classify_broadcast_outcome_with_client`, `noncustodial/tx_evidence.rs::taken_by_node_with_client`. *Pinned:* `node_rpc_injected_tests::{broadcast_taken_by_the_node_is_success, broadcast_taken_on_a_later_check_is_success, broadcast_mined_without_tx_index_is_success}`. **R2 — Not taken is not sent, and the user is told.** When every check gets hsd's not-found, the broadcast returns an error and the draft becomes `broadcast_pending` with the note "The node did not take the transaction. hsd does not say why; often its coins are already spent elsewhere. You can try again; the coins stay held until the node is checked again." Its coin reservation is kept. The send dialog shows the error as it shows any failed send ("Not sent"), and a retry is allowed, as for any `broadcast_pending` draft. @@ -28,11 +28,19 @@ When the user sends a transaction, the wallet says "sent" only once the node has **R4 — The draft lifecycle resolves it as before.** `refresh_tx_confirmations` promotes a `broadcast_pending` draft the node later knows or finds mined by its outputs, and after its grace window resolves one hsd still does not know by the chain evidence (unchanged): every coin it spends unspent marks it `failed` and releases its coins; a coin it spends spent by another transaction, on a node with a transaction index, marks it `dropped` saying so and releases its coins; otherwise it stays held. *Enforced:* `commands/tx.rs::refresh_tx_confirmations`. -*Pinned:* `live_node_it::live_coins_spent_by_another_tx_drop_the_draft_and_say_so`, changed to expect the R2 error from the second spend of a coin (`broadcast_pending`), then `dropped` after the grace window. +*Pinned:* `live_node_it::live_coins_spent_by_another_tx_drop_the_draft_and_say_so` (the second spend of a coin gets the R2 error and is `broadcast_pending`, then `dropped` after the grace window), `live_node_it::live_send_double_spend_is_not_sent`. **R5 — A Shakedex purchase follows the same rule.** The purchase broadcast goes through R1–R3. The one automatic rebroadcast (spec 2026-10-05 R13) uses the same check: a rebroadcast that is not taken loses the purchase with the existing REFUSED reason ("the node refused to take the purchase, so nothing was paid"), unless the chain still traces to it. *Enforced:* `shakedex_jobs.rs::refresh_missing`. -*Pinned:* `shakedex_purchase_state_tests::rebroadcast_not_taken_is_lost`, `live_node_it::shakedex_cli_buyer_first_loses_ours_with_nothing_paid` (the purchase broadcast now returns the R2 error). +*Pinned:* `shakedex_purchase_state_tests::rebroadcast_not_taken_is_lost`, `live_node_it::shakedex_cli_buyer_first_loses_ours_with_nothing_paid` (the purchase broadcast returns the R2 error), `live_node_it::shakedex_rebroadcast_after_a_price_drop_is_not_sent`. + +**R6 — A sent or released draft is not signed or sent again.** Only a draft never sent (`draft`, `signed`) or one whose send is not settled (`broadcast_pending`) is signed or broadcast. A `broadcasted` or `confirmed` one is refused ("this transaction was already sent"); a `dropped` or `failed` one is refused because its coins were released and another draft may hold them ("build it again to send it"). Signing is refused too, since it would flip the draft back to `signed`. Nothing reaches the node. +*Enforced:* `commands/tx.rs::refuse_unless_sendable`, called by `sign_tx_draft_inner` and `broadcast_tx_draft`. +*Pinned:* `tx_lifecycle_tests::sign_and_broadcast_refuse_a_sent_or_released_draft`, `live_node_it::live_send_rebroadcast_same_draft_rejected`. + +**R7 — A released draft that is mined after all is confirmed.** A `dropped` or `failed` draft is still looked for on chain for 72 hours after its verdict, hsd's mempool expiry (`policy.MEMPOOL_EXPIRY_TIME`): a node that held it, or this node before a restart emptied its mempool, may still have it mined. Found mined (by `getrawtransaction`, or by its outputs on a node without a transaction index), it is `confirmed` at that height, so a payment that went through never keeps reading "not sent". Only a mined transaction changes the verdict; a purchase is left to the purchase job. +*Enforced:* `commands/tx.rs::refresh_tx_confirmations`, `db/queries.rs::list_released_drafts_to_watch`. +*Pinned:* `tx_lifecycle_tests::refresh_confirms_a_dropped_draft_mined_after_all`, `live_node_it::{live_dropped_send_mined_after_all_is_confirmed, live_noindex_dropped_send_mined_after_all_is_confirmed}`. ## 4. Explicitly not enforced @@ -43,9 +51,10 @@ When the user sends a transaction, the wallet says "sent" only once the node has - **Mined inside the window on a node without a transaction index, its outputs already spent.** R1 finds a mined transaction on such a node only through an output that is still a coin. One whose every output was spent within the window reads as not taken, and R4 leaves it held. - **Up to two seconds more per send** when the node does not have the transaction; a transaction the node takes is found on the first check. +- **A released draft mined more than 72 hours after its verdict** keeps reading `dropped` or `failed`; hsd's own mempool forgets a transaction by then. ## 6. Pointers -- `src-tauri/src/commands/tx.rs` (`classify_broadcast_outcome_with_client`, `broadcast_tx_draft`), `src-tauri/src/shakedex_jobs.rs` (`refresh_missing`). +- `src-tauri/src/commands/tx.rs` (`classify_broadcast_outcome_with_client`, `broadcast_tx_draft`, `refuse_unless_sendable`, `refresh_tx_confirmations`), `src-tauri/src/noncustodial/tx_evidence.rs`, `src-tauri/src/shakedex_jobs.rs` (`refresh_missing`). - hsd 8.0.0: `lib/node/rpc.js` (`sendRawTransaction`, `getRawTransaction`), `lib/node/fullnode.js` (`relay`, `sendTX`, `getMeta`). - `docs/USER_MANUAL.md` ("Send fails with Not sent"), `CHANGELOG.md`. diff --git a/docs/specs/README.md b/docs/specs/README.md index 6e7f9e8..126e3ae 100644 --- a/docs/specs/README.md +++ b/docs/specs/README.md @@ -25,3 +25,4 @@ Each spec has these sections, in this order: | [2026-09-20 Multiple bids per name](./2026-09-20-multiple-bids-per-name.md) | Implemented | | [2026-09-21 Paid name swaps](./2026-09-21-paid-name-swaps.md) | Superseded by 2026-10-05 Shakedex name sales | | [2026-10-05 Shakedex name sales](./2026-10-05-shakedex-name-sales.md) | Accepted — buying implemented; selling and clean-up not yet | +| [2026-10-07 Honest broadcast result](./2026-10-07-honest-broadcast-result.md) | Implemented | diff --git a/src-tauri/src/commands/tx.rs b/src-tauri/src/commands/tx.rs index 66d4e0c..806bf8e 100644 --- a/src-tauri/src/commands/tx.rs +++ b/src-tauri/src/commands/tx.rs @@ -36,6 +36,10 @@ use crate::noncustodial::send; use crate::noncustodial::shakedex::purchase::{ PurchaseFinalizeSummary, PurchaseSummary, PURCHASE_ACTION, PURCHASE_FINALIZE_ACTION, }; +use crate::noncustodial::tx_evidence::{ + chain_evidence_with_client, node_has_tx_index_with_client, taken_by_node_with_client, + ChainEvidence, Taken, NOT_TAKEN, +}; use crate::noncustodial::types::{doos_to_hns_string, BroadcastResult, TxDraftSummary, TxSummary}; use crate::noncustodial::{derivation, sync}; use crate::AppState; @@ -706,6 +710,9 @@ pub(crate) async fn sign_tx_draft_inner( let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; let draft = db::queries::get_tx_draft(&conn, draft_id)? .ok_or_else(|| AppError::NotFound(format!("draft {draft_id}")))?; + // Signing flips a draft to `signed`, which `broadcast_tx_draft` sends: + // the same draft statuses are refused here. + refuse_unless_sendable(&draft.status)?; let profile = db::queries::get_wallet_profile(&conn, &draft.wallet_profile_id)? .ok_or_else(|| { AppError::NotFound(format!("wallet profile {}", draft.wallet_profile_id)) @@ -774,6 +781,22 @@ pub(crate) async fn sign_tx_draft_inner( load_draft_summary(state, draft_id) } +/// Only a draft never sent, or one whose send is not settled +/// (`broadcast_pending`), is signed or sent. A sent one is the node's to +/// confirm; a `dropped` or `failed` one has had its coins released, which +/// another draft may hold by now. +fn refuse_unless_sendable(status: &str) -> Result<(), AppError> { + match status { + "draft" | "signed" | "broadcast_pending" => Ok(()), + "broadcasted" | "confirmed" => Err(AppError::InvalidInput( + "this transaction was already sent".to_string(), + )), + other => Err(AppError::InvalidInput(format!( + "this transaction is {other} and its coins were released; build it again to send it" + ))), + } +} + /// Persist the signed hex + summary and flip the draft's status to `signed`. fn persist_signed_draft( state: &State<'_, AppState>, @@ -1483,6 +1506,11 @@ pub async fn sign_name_message( pub(crate) enum BroadcastOutcome { /// Node accepted the tx and returned a txid. Success(String), + /// Node returned a txid and the chain already shows the tx mined at this + /// height (found by its outputs on a node without a transaction index). + Mined(String, i64), + /// Node returned a txid but did not take the tx ([`Taken::No`]). + NotTaken(String), /// hsd refused the tx with its own JSON-RPC error (for /// `sendrawtransaction`, only a body it cannot decode) — the tx was /// definitively rejected and coins are unspent. The wrapped @@ -1497,8 +1525,9 @@ pub(crate) enum BroadcastOutcome { } /// Client-injected broadcast outcome classification for [`broadcast_tx_draft`]. -/// Calls `send_raw_transaction` and classifies the result into three -/// categories: success (txid), RPC error (hsd's own refusal, see +/// Calls `send_raw_transaction`, then [`taken_by_node_with_client`], and +/// classifies the result: success (txid, the node has it), mined, not taken, +/// RPC error (hsd's own refusal, see /// [`crate::noncustodial::rpc::is_node_rejection`]), or transport error /// (ambiguous: no reply, or a reply that is not hsd's). Testable against a /// mock. @@ -1507,7 +1536,13 @@ pub(crate) async fn classify_broadcast_outcome_with_client( signed_hex: &str, ) -> BroadcastOutcome { match client.send_raw_transaction(signed_hex).await { - Ok(txid) => BroadcastOutcome::Success(txid), + // hsd answers with the txid whatever its mempool does with the + // transaction: only a look-up says whether the node has it. + Ok(txid) => match taken_by_node_with_client(client, signed_hex, &txid).await { + Taken::Yes | Taken::Unknown => BroadcastOutcome::Success(txid), + Taken::Mined(height) => BroadcastOutcome::Mined(txid, height), + Taken::No => BroadcastOutcome::NotTaken(txid), + }, // Only hsd's own refusal proves the transaction will not be accepted; // a reply that is not hsd's (a proxy's error page) is as ambiguous // as a dropped connection. @@ -1661,6 +1696,7 @@ pub async fn broadcast_tx_draft( } else { None }; + refuse_unless_sendable(&draft.status)?; let maybe_sent = db::queries::may_have_reached_chain(&draft.status); // An unsent purchase draft past its coin reservation is discarded // with its purchase, as the sync would: its coins may fund another @@ -1735,6 +1771,33 @@ pub async fn broadcast_tx_draft( status: "broadcasted".to_string(), }) } + BroadcastOutcome::Mined(txid, height) => { + db::queries::update_tx_draft_status( + &conn, + &draft_id, + "broadcasted", + None, + Some(&txid), + )?; + db::queries::update_tx_draft_confirmation(&conn, &draft_id, height, None)?; + Ok(BroadcastResult { + draft_id, + txid, + status: "confirmed".to_string(), + }) + } + // Not sent: held as `broadcast_pending` (coins kept, retry allowed) + // for the confirmation poll to resolve from the chain. + BroadcastOutcome::NotTaken(_) => { + db::queries::update_tx_draft_status( + &conn, + &draft_id, + "broadcast_pending", + Some(NOT_TAKEN), + None, + )?; + Err(AppError::Rpc(NOT_TAKEN.to_string())) + } BroadcastOutcome::RpcError(e) => { // hsd's refusal itself, so its words are said once. let msg = e.to_string(); @@ -1799,6 +1862,12 @@ const EVICTION_GRACE_SECS: i64 = 600; /// used for finality on Bitcoin-family chains. const CONFIRMATION_FINALITY_DEPTH: i64 = 12; +/// How long a `dropped` or `failed` draft is still looked for on chain: hsd +/// keeps a transaction in its mempool for 72 hours +/// (`policy.MEMPOOL_EXPIRY_TIME`), so a node that held it may still have it +/// mined that long after this wallet's verdict. +const RELEASED_WATCH_SECS: i64 = 72 * 60 * 60; + /// Extract the txid a draft's `summary_json` already carries. Both plain-send /// [`crate::noncustodial::types::TxSummary`] and the covenant-action /// `ActionSummary` (`commands::names`) serialize a `"txid"` field, computed @@ -1825,74 +1894,10 @@ fn local_txid_from_summary(summary_json: &str) -> Option { }) } -/// What the chain shows about a draft's transaction that hsd answered with -/// its own "Transaction not found.". -/// -/// That answer is not evidence the transaction is in no block: hsd looks a -/// transaction up in its mempool and then in its transaction index -/// (`node.getMeta` → `chaindb.getMeta`, null without `--index-tx`), so on a -/// node without the index every mined transaction reads as not found. The -/// coin lookups below need no transaction index (`GET /coin/:hash/:index`, -/// mempool then chain UTXO set), and decide the verdict instead. -#[derive(Debug, PartialEq, Eq)] -pub(crate) enum ChainEvidence { - /// One of its outputs is a coin mined at this height: the transaction is - /// in that block. - Mined(i64), - /// Every coin it spends is unspent: it is in no block and no mempool of - /// this node, and its coins were not moved. - InputsUnspent, - /// A coin it spends is spent and none of its outputs is a coin: another - /// transaction spent the coin, or — only on a node without a transaction - /// index — it was mined and its outputs were spent since. - InputsSpent, - /// No answer that decides it (a transport error, a reply that is not - /// hsd's, an output only in the mempool, a draft that does not parse). - Unknown, -} - -/// Read [`ChainEvidence`] for draft `d`, whose transaction is `txid`, from -/// the coins of its own outputs and of its inputs. -pub(crate) async fn chain_evidence_with_client( - client: &dyn crate::noncustodial::node_rpc::NodeRpc, - d: &db::queries::TxDraftRow, - txid: &str, -) -> ChainEvidence { - let raw = d.signed_tx_hex.as_deref().unwrap_or(&d.unsigned_tx_hex); - let Some(tx) = hex::decode(raw) - .ok() - .and_then(|b| crate::noncustodial::tx::Transaction::decode(&b).ok()) - .filter(|tx| tx.txid() == txid) - else { - return ChainEvidence::Unknown; - }; - for vout in 0..tx.outputs.len() as u32 { - match client - .get_coin(txid, vout) - .await - .map(|c| c.map(|c| c.mined_height())) - { - Ok(Some(Ok(Some(height)))) => return ChainEvidence::Mined(height), - // Its coin in the mempool, though hsd just did not find the - // transaction there; or a height that is not hsd's. - Ok(Some(_)) | Err(_) => return ChainEvidence::Unknown, - Ok(None) => {} - } - } - let mut all_unspent = true; - for input in &tx.inputs { - let prev = hex::encode(input.prevout.hash); - match client.get_coin(&prev, input.prevout.index).await { - Ok(Some(_)) => {} - Ok(None) => all_unspent = false, - Err(_) => return ChainEvidence::Unknown, - } - } - if all_unspent { - ChainEvidence::InputsUnspent - } else { - ChainEvidence::InputsSpent - } +/// A draft's transaction bytes: signed once it is, else unsigned (the same +/// inputs, outputs and txid). +fn draft_hex(d: &db::queries::TxDraftRow) -> &str { + d.signed_tx_hex.as_deref().unwrap_or(&d.unsigned_tx_hex) } /// Whether the node keeps a transaction index, asked at most once per @@ -1915,20 +1920,6 @@ impl TxIndexProbe { } } -pub(crate) async fn node_has_tx_index_with_client( - client: &dyn crate::noncustodial::node_rpc::NodeRpc, - tip: i64, -) -> Option { - let hash = client.get_block_hash(tip).await.ok()?; - let block = client.get_block(&hash).await.ok()?; - let coinbase = block.get("tx")?.get(0)?.get("txid")?.as_str()?.to_string(); - match client.get_raw_transaction(&coinbase).await { - Ok(_) => Some(true), - Err(e) if crate::noncustodial::rpc::is_tx_not_found(&e) => Some(false), - Err(_) => None, - } -} - /// Re-poll the node for the on-chain status of this profile's in-flight drafts. /// /// Three states are tracked (I5): @@ -1966,7 +1957,7 @@ pub async fn refresh_tx_confirmations( "walletProfileId": profile_id, "nodeReachable": node_reachable, "checked": 0, "confirmed": 0, "dropped": 0, - "reverted": 0, "promoted": 0, "failed": 0, + "reverted": 0, "promoted": 0, "failed": 0, "revived": 0, }) } @@ -2003,7 +1994,11 @@ pub async fn refresh_tx_confirmations( CONFIRMATION_FINALITY_DEPTH, )? }; - if drafts.is_empty() { + let released = { + let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; + db::queries::list_released_drafts_to_watch(&conn, &profile_id, RELEASED_WATCH_SECS)? + }; + if drafts.is_empty() && released.is_empty() { return Ok(empty_result(Some(&profile_id), true)); } @@ -2042,7 +2037,7 @@ pub async fn refresh_tx_confirmations( if d.action == PURCHASE_ACTION { continue; } - match chain_evidence_with_client(&client, d, &txid).await { + match chain_evidence_with_client(&client, draft_hex(d), &txid).await { ChainEvidence::Mined(height) => { confirmed_updates.push((d.id.clone(), height, Some(txid))) } @@ -2087,7 +2082,7 @@ pub async fn refresh_tx_confirmations( if d.action == PURCHASE_ACTION && d.status != "confirmed" { continue; } - match chain_evidence_with_client(&client, d, txid).await { + match chain_evidence_with_client(&client, draft_hex(d), txid).await { ChainEvidence::Mined(height) => { confirmed_updates.push((d.id.clone(), height, None)) } @@ -2121,6 +2116,35 @@ pub async fn refresh_tx_confirmations( } } + // 4b. A `dropped` or `failed` draft mined after all (other nodes held it, + // or this node lost its mempool on a restart): confirmed, so it never + // reads "not sent" for a payment that went through. Only a mined + // transaction is a verdict here; a purchase is the purchase job's. + let mut revived = 0; + for d in released.iter().filter(|d| d.action != PURCHASE_ACTION) { + let Some(txid) = d + .txid + .clone() + .or_else(|| local_txid_from_summary(&d.summary_json)) + else { + continue; + }; + let height = match client.get_raw_transaction(&txid).await { + Ok(tx) => raw_tx_mined_height(&tx, tip).ok().flatten(), + Err(e) if crate::noncustodial::rpc::is_tx_not_found(&e) => { + match chain_evidence_with_client(&client, draft_hex(d), &txid).await { + ChainEvidence::Mined(height) => Some(height), + _ => None, + } + } + Err(_) => None, + }; + if let Some(height) = height { + confirmed_updates.push((d.id.clone(), height, Some(txid))); + revived += 1; + } + } + // 5. Persist under the lock. let (n_conf, n_drop, n_revert, n_promote, n_fail) = { let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; @@ -2212,7 +2236,8 @@ pub async fn refresh_tx_confirmations( Ok(serde_json::json!({ "walletProfileId": profile_id, "nodeReachable": true, - "checked": drafts.len(), + "checked": drafts.len() + released.len(), + "revived": revived, "confirmed": n_conf, "dropped": n_drop, "reverted": n_revert, diff --git a/src-tauri/src/db/queries.rs b/src-tauri/src/db/queries.rs index 64c688d..70507b8 100644 --- a/src-tauri/src/db/queries.rs +++ b/src-tauri/src/db/queries.rs @@ -1945,6 +1945,31 @@ pub fn list_drafts_awaiting_confirmation( Ok(out) } +/// `dropped` and `failed` drafts last updated within `max_age_secs`: their +/// coins were released, but a transaction other nodes still hold can be mined +/// after the verdict, and the confirmation poll keeps looking for it that long. +pub fn list_released_drafts_to_watch( + conn: &rusqlite::Connection, + profile_id: &str, + max_age_secs: i64, +) -> Result, AppError> { + let sql = format!( + "SELECT {DRAFT_COLS} FROM wallet_tx_drafts + WHERE wallet_profile_id = ?1 + AND status IN ('dropped', 'failed') + AND signed_tx_hex IS NOT NULL + AND (julianday('now') - julianday(updated_at)) * 86400 < ?2 + ORDER BY created_at DESC" + ); + let mut stmt = conn.prepare(&sql)?; + let rows = stmt.query_map(params![profile_id, max_age_secs], row_to_draft)?; + let mut out = Vec::new(); + for r in rows { + out.push(r?); + } + Ok(out) +} + /// All derived address strings for a profile (both branches). Used by the sync /// engine to scan the node for coins. pub fn get_profile_addresses( diff --git a/src-tauri/src/noncustodial/mod.rs b/src-tauri/src/noncustodial/mod.rs index 3e3e3b1..d55ab53 100644 --- a/src-tauri/src/noncustodial/mod.rs +++ b/src-tauri/src/noncustodial/mod.rs @@ -24,5 +24,6 @@ pub mod session; pub mod shakedex; pub mod sync; pub mod tx; +pub mod tx_evidence; pub mod types; pub mod vault; diff --git a/src-tauri/src/noncustodial/tx_evidence.rs b/src-tauri/src/noncustodial/tx_evidence.rs new file mode 100644 index 0000000..b8bfbfb --- /dev/null +++ b/src-tauri/src/noncustodial/tx_evidence.rs @@ -0,0 +1,146 @@ +//! What the chain shows about a transaction hsd answers "Transaction not +//! found." for, and whether the node took one it answered +//! `sendrawtransaction` for. +//! +//! hsd looks a transaction up by txid in its mempool and then in its +//! transaction index (`node.getMeta` → `chaindb.getMeta`, null without +//! `--index-tx`), so its not-found is not evidence a transaction is in no +//! block. The coin lookups here need no transaction index +//! (`GET /coin/:hash/:index`: mempool, then the chain's UTXO set). + +use crate::noncustodial::node_rpc::NodeRpc; +use crate::noncustodial::rpc::is_tx_not_found; +use crate::noncustodial::tx::Transaction; + +/// What the chain shows about a draft's transaction that hsd answered with +/// its own "Transaction not found.". +/// +/// That answer is not evidence the transaction is in no block: hsd looks a +/// transaction up in its mempool and then in its transaction index +/// (`node.getMeta` → `chaindb.getMeta`, null without `--index-tx`), so on a +/// node without the index every mined transaction reads as not found. The +/// coin lookups below need no transaction index (`GET /coin/:hash/:index`, +/// mempool then chain UTXO set), and decide the verdict instead. +#[derive(Debug, PartialEq, Eq)] +pub enum ChainEvidence { + /// One of its outputs is a coin mined at this height: the transaction is + /// in that block. + Mined(i64), + /// Every coin it spends is unspent: it is in no block and no mempool of + /// this node, and its coins were not moved. + InputsUnspent, + /// A coin it spends is spent and none of its outputs is a coin: another + /// transaction spent the coin, or — only on a node without a transaction + /// index — it was mined and its outputs were spent since. + InputsSpent, + /// No answer that decides it (a transport error, a reply that is not + /// hsd's, an output only in the mempool, a draft that does not parse). + Unknown, +} + +/// Read [`ChainEvidence`] for the transaction `txid`, serialized as +/// `raw_hex`, from the coins of its own outputs and of its inputs. +pub async fn chain_evidence_with_client( + client: &dyn NodeRpc, + raw_hex: &str, + txid: &str, +) -> ChainEvidence { + let Some(tx) = hex::decode(raw_hex) + .ok() + .and_then(|b| Transaction::decode(&b).ok()) + .filter(|tx| tx.txid() == txid) + else { + return ChainEvidence::Unknown; + }; + for vout in 0..tx.outputs.len() as u32 { + match client + .get_coin(txid, vout) + .await + .map(|c| c.map(|c| c.mined_height())) + { + Ok(Some(Ok(Some(height)))) => return ChainEvidence::Mined(height), + // Its coin in the mempool, though hsd just did not find the + // transaction there; or a height that is not hsd's. + Ok(Some(_)) | Err(_) => return ChainEvidence::Unknown, + Ok(None) => {} + } + } + let mut all_unspent = true; + for input in &tx.inputs { + let prev = hex::encode(input.prevout.hash); + match client.get_coin(&prev, input.prevout.index).await { + Ok(Some(_)) => {} + Ok(None) => all_unspent = false, + Err(_) => return ChainEvidence::Unknown, + } + } + if all_unspent { + ChainEvidence::InputsUnspent + } else { + ChainEvidence::InputsSpent + } +} + +/// How many times, and how far apart, a send is looked up after +/// `sendrawtransaction` answered: hsd answers with the txid before its +/// mempool has decided (`rpc.js` `sendRawTransaction` relays without +/// awaiting), and adds an accepted transaction asynchronously. +pub const TAKEN_CHECKS: u32 = 5; +pub const TAKEN_CHECK_INTERVAL: std::time::Duration = std::time::Duration::from_millis(400); + +/// Whether the node took a transaction it answered `sendrawtransaction` for. +#[derive(Debug, PartialEq, Eq)] +pub enum Taken { + /// `getrawtransaction` finds it (mempool, or chain through the index). + Yes, + /// Not found, but one of its outputs is a coin mined at this height: on + /// a node without a transaction index, a send mined in the meantime. + Mined(i64), + /// Every check got hsd's own "Transaction not found." and the chain shows + /// no output of it mined: the node did not take it. hsd does not say why. + No, + /// A check got no answer, or one that is not hsd's: no verdict. + Unknown, +} + +/// Look `txid` up up to [`TAKEN_CHECKS`] times, [`TAKEN_CHECK_INTERVAL`] +/// apart; the first find ends the window. Only hsd's not-found on every check, +/// with no output mined ([`chain_evidence_with_client`]), is [`Taken::No`]. +pub async fn taken_by_node_with_client( + client: &dyn NodeRpc, + signed_hex: &str, + txid: &str, +) -> Taken { + for check in 0..TAKEN_CHECKS { + if check > 0 { + tokio::time::sleep(TAKEN_CHECK_INTERVAL).await; + } + match client.get_raw_transaction(txid).await { + Ok(_) => return Taken::Yes, + Err(e) if is_tx_not_found(&e) => {} + Err(_) => return Taken::Unknown, + } + } + match chain_evidence_with_client(client, signed_hex, txid).await { + ChainEvidence::Mined(height) => Taken::Mined(height), + ChainEvidence::Unknown => Taken::Unknown, + ChainEvidence::InputsUnspent | ChainEvidence::InputsSpent => Taken::No, + } +} + +/// The note a send the node did not take carries (spec honest-broadcast R2). +pub const NOT_TAKEN: &str = "The node did not take the transaction. hsd does not say why; often its coins are already spent elsewhere. You can try again; the coins stay held until the node is checked again."; + +/// Whether the node keeps a transaction index: the coinbase of the block at +/// `tip` is mined, so hsd finds it with `getrawtransaction` exactly when it +/// has the index. `None` when the answer is not hsd's or does not come. +pub async fn node_has_tx_index_with_client(client: &dyn NodeRpc, tip: i64) -> Option { + let hash = client.get_block_hash(tip).await.ok()?; + let block = client.get_block(&hash).await.ok()?; + let coinbase = block.get("tx")?.get(0)?.get("txid")?.as_str()?.to_string(); + match client.get_raw_transaction(&coinbase).await { + Ok(_) => Some(true), + Err(e) if is_tx_not_found(&e) => Some(false), + Err(_) => None, + } +} diff --git a/src-tauri/src/shakedex_jobs.rs b/src-tauri/src/shakedex_jobs.rs index 0f20899..23216bf 100644 --- a/src-tauri/src/shakedex_jobs.rs +++ b/src-tauri/src/shakedex_jobs.rs @@ -41,6 +41,7 @@ use crate::noncustodial::rpc::{self, NodeRpcClient}; use crate::noncustodial::send::RESERVATION_TTL_SECS; use crate::noncustodial::shakedex::purchase::{self, transfer_commits_to}; use crate::noncustodial::shakedex::verify; +use crate::noncustodial::tx_evidence; /// Blocks a sent purchase may be absent from the node's mempool and chain /// before it is rebroadcast (once) and then given up as lost. @@ -374,7 +375,18 @@ impl Job<'_> { verify::PaidStep::NotValid => return wait().await, } match self.client.send_raw_transaction(signed).await { - Ok(_) => self.unconfirmed(p, None, 1), + // hsd answers with the txid whatever its mempool does with the + // purchase: only a look-up says whether the node took it. + Ok(_) => { + match tx_evidence::taken_by_node_with_client(self.client, signed, &p.purchase_txid) + .await + { + tx_evidence::Taken::No => self.lose_unless_traced(p, REFUSED, Some(1)).await, + // Taken, mined meanwhile (the next sync traces it), or no + // answer: the one rebroadcast is spent either way. + _ => self.unconfirmed(p, None, 1), + } + } // hsd's own error: it did not take the transaction (in hsd 8.0.0 // only for input it cannot parse; a mempool refusal still // answers with the txid). diff --git a/src-tauri/src/tests/live_node_it.rs b/src-tauri/src/tests/live_node_it.rs index 5cea373..510f50a 100644 --- a/src-tauri/src/tests/live_node_it.rs +++ b/src-tauri/src/tests/live_node_it.rs @@ -1289,103 +1289,29 @@ async fn live_send_wrong_network_address_rejected() { ); } -/// A8. Double-spend safety: broadcast a competing tx spending one of a -/// draft's inputs via a second draft, then try to broadcast the first — -/// the node rejects it, the draft flips to `failed`, and its reservation -/// is released so future drafts can reuse the coin. +/// A8. Double spend: a draft whose coin another mined draft spent is not +/// sent. hsd answers its txid but never takes it, and the wallet says so +/// (honest-broadcast R2, in [`double_spent_draft`]); it is never mined. #[tokio::test] -async fn live_send_broadcast_double_spend_releases_reservation() { +async fn live_send_double_spend_is_not_sent() { let Some((url, key)) = it_env() else { - eprintln!( - "skip live_send_broadcast_double_spend_releases_reservation: set HNS_IT_NODE_URL" - ); + eprintln!("skip live_send_double_spend_is_not_sent: set HNS_IT_NODE_URL"); return; }; - let conn = seeded_conn_regtest(&url, &key); - let app = app_with(conn); - let cl = client(&url, &key); - let (addr, _, _) = leaf00(); - - // A single spendable coinbase. - fund(&cl, &addr, 103).await; - sync_wallet_state(app.state(), None).await.expect("sync"); - - // Draft A: build & sign but DO NOT broadcast yet. - let draft_a = build_send_hns_draft(app.state(), addr.clone(), 100_000, Some(1), None) - .await - .expect("build A"); - unlock(&app); - sign_tx_draft_inner(&app.state(), &draft_a.id) - .await - .expect("sign A"); - - // Manually free A's reservation so draft B can select the same coin(s). - { - let state = app.state::(); - let c = state.db.lock().unwrap(); - db::queries::release_reserved_utxos_for_draft(&c, &draft_a.id).unwrap(); - } - - // Draft B: send a DIFFERENT amount so its signed hex is a different tx - // (same inputs -> conflicts on broadcast). Sign + broadcast + mine. - let draft_b = build_send_hns_draft(app.state(), addr.clone(), 200_000, Some(1), None) - .await - .expect("build B"); - execute(&app, &cl, &addr, draft_b.id).await; - - // Now broadcast A. Its inputs were already spent on-chain by B, so A can - // never CONFIRM. hsd's mempool is permissive and may still accept the raw - // tx (returning a txid) OR reject it outright — both are safe. The real - // double-spend invariant is confirmation-level: A must NOT end up mined. - let res = broadcast_tx_draft(app.state(), draft_a.id.clone()).await; - let a_txid = draft_status(&app, &draft_a.id).txid; - - // Mine a block; a valid tx would confirm here. A conflicts with B's - // already-confirmed spend, so it cannot. - cl.generate_to_address(1, &addr).await.expect("mine"); - if let Some(txid) = a_txid.clone() { - let on_chain = cl.get_tx_by_hash(&txid).await.ok(); - let confirmed = on_chain - .as_ref() - .filter(|v| !v.is_null()) - .and_then(|v| v.get("height")) - .and_then(|h| h.as_i64()) - .map(|h| h >= 0) - .unwrap_or(false); - assert!( - !confirmed, - "double-spend tx A must NEVER confirm (B already spent the input)" - ); - } - - // If broadcast failed, the draft's reservation must have been released; - // if it succeeded into the mempool, release it here to mirror the app's - // eventual settle. Either way, A holds no reservation afterward. - if res.is_err() { - assert_eq!( - draft_status(&app, &draft_a.id).status, - "failed", - "a rejected double-spend draft must be marked failed" - ); - } else { - crate::commands::tx::release_tx_draft_reservation(app.state(), draft_a.id.clone()) + let (app, a) = double_spent_draft(&url, &key).await; + let txid = summary_txid(&app, &a); + assert!( + client(&url, &key) + .get_tx_by_hash(&txid) .await - .ok(); - } - let state = app.state::(); - let c = state.db.lock().unwrap(); - let held: i64 = c - .query_row( - "SELECT COUNT(*) FROM tracked_utxos WHERE reserved_by_draft_id = ?1", - params![draft_a.id], - |r| r.get(0), - ) - .unwrap(); - assert_eq!(held, 0, "A's reservation must be gone afterward"); + .expect("tx lookup") + .is_null(), + "the double spend is never mined" + ); } -/// A9. Rebroadcasting the same signed draft after it has been mined -> node -/// rejects (inputs already spent). Confirms there is no accidental RBF. +/// A9. A draft already sent and mined is not sent again: the wallet refuses +/// before anything reaches the node, and the draft keeps its one txid. #[tokio::test] async fn live_send_rebroadcast_same_draft_rejected() { let Some((url, key)) = it_env() else { @@ -1407,18 +1333,11 @@ async fn live_send_rebroadcast_same_draft_rejected() { .txid .expect("draft mined so it has a txid"); - // Same signed hex, second broadcast. hsd's mempool is idempotent here: - // `sendrawtransaction` on a tx already in the chain returns the same - // txid without error (older builds may reject it — either is fine). The - // safety-relevant invariant is that no SECOND, conflicting tx is created - // and the wallet's ledger sees exactly the one confirmed spend. - let res = broadcast_tx_draft(app.state(), draft.id.clone()).await; - if let Ok(bc) = &res { - assert_eq!( - bc.txid, original_txid, - "rebroadcast must yield the same txid, never a fresh conflicting one" - ); - } + let err = broadcast_tx_draft(app.state(), draft.id.clone()) + .await + .expect_err("a sent draft is not sent again"); + assert!(err.to_string().contains("already sent"), "{err}"); + assert_eq!(draft_status(&app, &draft.id).status, "confirmed"); // Draft's recorded txid is unchanged either way. assert_eq!( draft_status(&app, &draft.id).txid.as_deref(), @@ -1830,18 +1749,15 @@ async fn live_confirm_reorg_reverts_to_broadcasted() { ); // Restore the chain and drive the draft back to `confirmed`. hsd's - // `invalidateblock` doesn't automatically resurrect the invalidated - // block's transactions into the mempool for later re-mining — after - // `reconsiderblock` the previously-invalidated block is no longer - // invalid-marked, but the tx may or may not have been retained in the - // node's mempool. To make the "re-confirms after reorg" step robust - // against that upstream ambiguity, we re-broadcast the signed tx (a - // no-op if it's already known) and mine, then refresh. + // `invalidateblock` rewinds through `reset`, which empties the mempool, + // and `reconsiderblock` only clears the invalid mark: the tx is nowhere. + // A real reorg puts a disconnected block's transactions back in the + // mempool; the test does that by hand, handing the signed tx to the node + // directly (the wallet does not send a `broadcasted` draft again). cl.reconsider_block(&block_hash).await.expect("reconsider"); - // Rebroadcast is idempotent — either resubmits the tx to the mempool or - // returns the same txid if it's still there. Failure here is fine: on - // some hsd builds the reconsider path may already have the tx queued. - let _ = broadcast_tx_draft(app.state(), d.id.clone()).await; + let signed = row.signed_tx_hex.clone().expect("signed hex"); + cl.send_raw_transaction(&signed).await.expect("resubmit"); + wait_until_node_has(&cl, row.txid.as_deref().expect("txid")).await; cl.generate_to_address(1, &addr).await.expect("remine"); refresh_tx_confirmations(app.state(), None) .await @@ -5031,10 +4947,11 @@ async fn shakedex_finalized_name_moved_on_before_a_sync_is_owned() { /// R13 "Lost, nothing paid": the CLI buys the listing between our review and /// our broadcast. hsd 8.0.0's `sendrawtransaction` answers with the txid /// whatever its mempool does (`rpc.js`: `this.node.relay(tx)`, not awaited, -/// its error only logged), so the broadcast reads as sent; the purchase job -/// then finds the purchase missing and the lock coin spent, and loses it with -/// nothing paid. Its coins are free again, and the listing is no longer -/// offered. +/// its error only logged); the look-up after it finds the node did not take +/// the purchase (honest-broadcast R2, R5), so the broadcast says "not sent" +/// and the draft waits as `broadcast_pending`. The purchase job then finds +/// the lock coin spent and loses it with nothing paid. Its coins are free +/// again, and the listing is no longer offered. #[tokio::test] async fn shakedex_cli_buyer_first_loses_ours_with_nothing_paid() { let Some((url, key, cli)) = @@ -5051,10 +4968,12 @@ async fn shakedex_cli_buyer_first_loses_ours_with_nothing_paid() { let draft = b.sign_purchase(&listing).await; assert!(b.reserved_by(&draft.id) > 0, "the purchase holds its coins"); cli.fill(&listing); - let bc = broadcast_tx_draft(b.app.state(), draft.id.clone()) + let err = broadcast_tx_draft(b.app.state(), draft.id.clone()) .await - .expect("hsd answers with the txid"); - assert_eq!(bc.status, "broadcasted"); + .expect_err("the node did not take the purchase"); + assert!(err.to_string().contains("did not take"), "{err}"); + assert_eq!(draft_status(&b.app, &draft.id).status, "broadcast_pending"); + let purchase_txid = summary_txid(&b.app, &draft.id); b.refresh().await; let p = b.purchase(&name); @@ -5076,7 +4995,7 @@ async fn shakedex_cli_buyer_first_loses_ours_with_nothing_paid() { // whatever its answer to the broadcast said. b.cl.generate_to_address(1, &b.addr).await.expect("mine"); assert!( - b.cl.get_tx_by_hash(&bc.txid) + b.cl.get_tx_by_hash(&purchase_txid) .await .expect("tx lookup") .is_null(), @@ -5891,7 +5810,7 @@ async fn live_noindex_unsent_draft_with_unspent_coins_is_dropped() { /// Draft A's coin is spent by draft B, mined; A is then sent, and hsd's /// mempool turns it away while still answering its txid. On a node without a /// transaction index the wallet cannot tell A's spend from B's, so it gives -/// no verdict on A — above all not "dropped, the coins were not moved". +/// no verdict on A — above all not "failed, the coins were not moved". #[tokio::test] async fn live_noindex_coins_spent_by_another_tx_give_no_verdict() { let Some((url, key)) = noindex_env() else { @@ -5904,7 +5823,7 @@ async fn live_noindex_coins_spent_by_another_tx_give_no_verdict() { .await .expect("refresh"); let row = draft_status(&app, &a); - assert_eq!(row.status, "broadcasted", "{:?}", row.error_message); + assert_eq!(row.status, "broadcast_pending", "{:?}", row.error_message); } /// The same double spend on a node WITH a transaction index: hsd's not-found @@ -5931,8 +5850,9 @@ async fn live_coins_spent_by_another_tx_drop_the_draft_and_say_so() { } /// Draft A signed over one coin, draft B spending the same coin mined, then -/// A sent: hsd answers A's txid and keeps it out of its mempool. Returns the -/// app and A's id. +/// A sent: hsd answers A's txid and keeps it out of its mempool, and the +/// wallet says so (honest-broadcast R2): not sent, `broadcast_pending`. +/// Returns the app and A's id. async fn double_spent_draft( url: &str, key: &str, @@ -5961,10 +5881,203 @@ async fn double_spent_draft( .expect("build B"); execute(&app, &cl, &addr, b.id).await; - let res = broadcast_tx_draft(app.state(), a.id.clone()) + let err = broadcast_tx_draft(app.state(), a.id.clone()) .await - .expect("hsd answers a txid for a tx its mempool turns away"); - assert_eq!(res.status, "broadcasted"); + .expect_err("hsd answers a txid for a tx its mempool turns away"); + assert!(err.to_string().contains("did not take"), "{err}"); + let row = draft_status(&app, &a.id); + assert_eq!(row.status, "broadcast_pending"); + assert_eq!( + row.error_message.as_deref(), + Some(crate::noncustodial::tx_evidence::NOT_TAKEN) + ); cl.generate_to_address(1, &addr).await.expect("mine"); (app, a.id) } + +/// A send judged `dropped` that is mined after all is confirmed by the next +/// poll. The verdict is forced here while the send sits in the mempool, as a +/// node that lost its mempool on a restart would have reached it, with its +/// coins released; the block then mines it. +async fn dropped_send_mined_after_all_is_confirmed(url: &str, key: &str) { + let conn = seeded_conn_regtest(url, key); + let app = app_with(conn); + let cl = client(url, key); + let (addr, _, _) = leaf00(); + fund(&cl, &addr, 103).await; + sync_wallet_state(app.state(), None).await.expect("sync"); + let d = build_send_hns_draft(app.state(), addr.clone(), 100_000, Some(1), None) + .await + .expect("build"); + broadcast_only(&app, &d.id).await; + let txid = draft_status(&app, &d.id).txid.expect("txid"); + wait_until_node_has(&cl, &txid).await; + { + let state = app.state::(); + let c = state.db.lock().unwrap(); + db::queries::update_tx_draft_status(&c, &d.id, "dropped", Some("judged dropped"), None) + .unwrap(); + db::queries::release_reserved_utxos_for_draft(&c, &d.id).unwrap(); + } + cl.generate_to_address(1, &addr).await.expect("mine"); + let mined_at = cl.get_blockchain_info().await.expect("info").blocks; + + let r = refresh_tx_confirmations(app.state(), None) + .await + .expect("refresh"); + let row = draft_status(&app, &d.id); + assert_eq!(row.status, "confirmed", "{r} {:?}", row.error_message); + assert_eq!(row.confirmation_height, Some(mined_at)); + assert_eq!(row.txid.as_deref(), Some(txid.as_str())); + assert_eq!(r["revived"], 1, "{r}"); +} + +#[tokio::test] +async fn live_dropped_send_mined_after_all_is_confirmed() { + let Some((url, key)) = it_env() else { + eprintln!("skip live_dropped_send_mined_after_all_is_confirmed: set HNS_IT_NODE_URL"); + return; + }; + dropped_send_mined_after_all_is_confirmed(&url, &key).await; +} + +/// The same on a node without a transaction index: the mined send is found +/// by its outputs. +#[tokio::test] +async fn live_noindex_dropped_send_mined_after_all_is_confirmed() { + let Some((url, key)) = noindex_env() else { + eprintln!( + "skip live_noindex_dropped_send_mined_after_all_is_confirmed: set HNS_IT_NOINDEX_NODE_URL" + ); + return; + }; + dropped_send_mined_after_all_is_confirmed(&url, &key).await; +} + +/// The fee rate is the one figure of a purchase consensus does not pin: an +/// excess is simply paid to the miner. A purchase and its FINALIZE built at a +/// rate above the 5 doos/byte floor pay that rate on hsd's virtual size, +/// the seller's input and signature included (spec R4); hsd's `GET /tx/:hash` +/// works the rate out the same way. +#[tokio::test] +async fn shakedex_purchase_and_finalize_pay_their_fee_rate_on_vsize() { + let Some((url, key, cli)) = + shakedex_env("shakedex_purchase_and_finalize_pay_their_fee_rate_on_vsize") + else { + return; + }; + let per_byte = 20; + let listing = cli.sell_fixed(3); + let b = ShakedexBuyer::new(&url, &key).await; + advance_mtp_past(&b.cl, &b.addr, listing.lock_time(0)).await; + sync_wallet_state(b.app.state(), None).await.expect("sync"); + + let draft = crate::commands::shakedex::shakedex_build_purchase_draft( + b.app.state(), + listing.json.clone(), + None, + false, + Some(per_byte), + ) + .await + .expect("build purchase"); + broadcast_only(&b.app, &draft.id).await; + settle(&b.app, &b.cl, &b.addr, &draft.id).await; + assert_pays_rate(&b, &draft, per_byte).await; + + b.cl.generate_to_address(NET.name_params().transfer_lockup, &b.addr) + .await + .expect("mine lockup"); + b.refresh().await; + let p = b.purchase(&listing.name); + let fin = crate::commands::shakedex::shakedex_build_purchase_finalize_draft( + b.app.state(), + p.id.clone(), + Some(per_byte), + ) + .await + .expect("build finalize"); + broadcast_only(&b.app, &fin.id).await; + settle(&b.app, &b.cl, &b.addr, &fin.id).await; + assert_pays_rate(&b, &fin, per_byte).await; +} + +/// `draft`, mined, paid the fee its summary shows, at `per_byte` doos per +/// virtual byte as hsd works it out (doos per 1000 virtual bytes), within the +/// byte or two a signature's length can vary. +async fn assert_pays_rate( + b: &ShakedexBuyer, + draft: &crate::noncustodial::types::TxDraftSummary, + per_byte: u64, +) { + let txid = draft_status(&b.app, &draft.id).txid.expect("txid"); + let tx = b.cl.get_tx_by_hash(&txid).await.expect("tx"); + assert_eq!(tx["fee"], draft.summary["feeDoos"], "{tx}"); + let rate = tx["rate"].as_u64().expect("rate"); + let asked = per_byte * 1000; + assert!( + (asked..=asked * 102 / 100).contains(&rate), + "asked {asked} doos/kvB, hsd reports {rate}: {tx}" + ); +} + +/// R13 and R10 on a live node: a purchase taken out of the chain by a reorg +/// is missing, and its one rebroadcast is checked like any broadcast. A +/// cheaper step of the reverse auction became valid meanwhile, so the old +/// price is not sent again: the purchase is lost with nothing paid, its coins +/// released, and the node never sees it again. +#[tokio::test] +async fn shakedex_rebroadcast_after_a_price_drop_is_not_sent() { + let Some((url, key, cli)) = shakedex_env("shakedex_rebroadcast_after_a_price_drop_is_not_sent") + else { + return; + }; + let listing = cli.sell_auction(10, 5); + let name = listing.name.clone(); + let b = ShakedexBuyer::new(&url, &key).await; + advance_mtp_past(&b.cl, &b.addr, listing.lock_time(0)).await; + sync_wallet_state(b.app.state(), None).await.expect("sync"); + + let draft = b.sign_purchase(&listing).await; + assert_eq!(draft.summary["priceDoos"], listing.price(0)); + let bc = broadcast_tx_draft(b.app.state(), draft.id.clone()) + .await + .expect("broadcast"); + settle(&b.app, &b.cl, &b.addr, &draft.id).await; + b.refresh().await; + let bought_at = b.purchase(&name).purchase_height.expect("purchase height"); + + // `invalidateblock` empties the mempool: the purchase is nowhere. + let block = b.cl.get_block_hash(bought_at).await.expect("blockhash"); + b.cl.invalidate_block(&block).await.expect("invalidate"); + b.refresh().await; + assert_eq!( + b.purchase(&name).state, + db::queries::PurchaseState::Unconfirmed + ); + + // Step 1 becomes valid; the 11 blocks this mines are more than the six + // missing blocks that call for the rebroadcast. + advance_mtp_past(&b.cl, &b.addr, listing.lock_time(1)).await; + b.refresh().await; + let p = b.purchase(&name); + assert_eq!(p.state, db::queries::PurchaseState::Lost); + assert!( + p.lost_reason + .as_deref() + .is_some_and(|r| r.contains("not sent again")), + "{:?}", + p.lost_reason + ); + assert_eq!(p.rebroadcast_count, 0, "nothing was sent"); + assert_eq!(b.reserved_by(&draft.id), 0, "its coins are free"); + b.cl.generate_to_address(1, &b.addr).await.expect("mine"); + assert!( + b.cl.get_tx_by_hash(&bc.txid) + .await + .expect("tx lookup") + .is_null(), + "the old price never reached the node again" + ); + b.cl.reconsider_block(&block).await.expect("reconsider"); +} diff --git a/src-tauri/src/tests/mock_node_rpc.rs b/src-tauri/src/tests/mock_node_rpc.rs index c434b36..0df22ba 100644 --- a/src-tauri/src/tests/mock_node_rpc.rs +++ b/src-tauri/src/tests/mock_node_rpc.rs @@ -232,6 +232,16 @@ impl MockNodeRpc { self } + /// `getrawtransaction` answered by `f`, called once per look-up: a test + /// whose answer changes between look-ups keeps its own count. + pub fn with_raw_transaction_fn( + mut self, + f: impl Fn() -> Result + Send + Sync + 'static, + ) -> Self { + self.raw_transaction = Box::new(f); + self + } + /// hsd's own `getrawtransaction` answer for a transaction it does not /// find: `Transaction not found.`, code -1, as `call` returns it. pub fn with_raw_transaction_not_found(mut self) -> Self { diff --git a/src-tauri/src/tests/node_rpc_injected_tests.rs b/src-tauri/src/tests/node_rpc_injected_tests.rs index d6c2c65..ca0780e 100644 --- a/src-tauri/src/tests/node_rpc_injected_tests.rs +++ b/src-tauri/src/tests/node_rpc_injected_tests.rs @@ -735,14 +735,141 @@ async fn resolve_ownership_propagates_getnameinfo_error() { // ------- classify_broadcast_outcome_with_client --------------------------- +/// hsd's `getrawtransaction` verbose reply for a transaction in its mempool, +/// the fields the look-up reads (`txToJSON`: no block, 0 confirmations). +fn hsd_mempool_tx() -> serde_json::Value { + json!({ "txid": "abc123", "blockhash": null, "confirmations": 0 }) +} + +fn hsd_not_found() -> AppError { + AppError::NodeRefused { + message: "Transaction not found.".into(), + code: -1, + } +} + +/// Spec honest-broadcast R1: hsd answered the txid and has the transaction. #[tokio::test] -async fn broadcast_classify_success_returns_txid() { - let mock = MockNodeRpc::new().with_send_raw_transaction("abc123".to_string()); +async fn broadcast_taken_by_the_node_is_success() { + let mock = MockNodeRpc::new() + .with_send_raw_transaction("abc123".to_string()) + .with_raw_transaction(hsd_mempool_tx()); let out = crate::commands::tx::classify_broadcast_outcome_with_client(&mock, "deadbeef").await; match out { crate::commands::tx::BroadcastOutcome::Success(txid) => assert_eq!(txid, "abc123"), other => panic!("expected Success, got {other:?}"), } + assert!(mock + .calls() + .contains(&crate::tests::mock_node_rpc::RpcCall::RawTransaction( + "abc123".into() + ))); +} + +/// R1: hsd adds a transaction to its mempool after answering, so a later +/// check that finds it ends the window as a success. +#[tokio::test] +async fn broadcast_taken_on_a_later_check_is_success() { + let n = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let seen = n.clone(); + let mock = MockNodeRpc::new() + .with_send_raw_transaction("abc123".to_string()) + .with_raw_transaction_fn(move || { + if seen.fetch_add(1, std::sync::atomic::Ordering::SeqCst) < 2 { + Err(hsd_not_found()) + } else { + Ok(hsd_mempool_tx()) + } + }); + let out = crate::commands::tx::classify_broadcast_outcome_with_client(&mock, "deadbeef").await; + assert!( + matches!(out, crate::commands::tx::BroadcastOutcome::Success(_)), + "{out:?}" + ); + assert_eq!(n.load(std::sync::atomic::Ordering::SeqCst), 3); +} + +/// R1 and R2: hsd's not-found on every check, with every coin the +/// transaction spends unspent, is not taken. Each check asks hsd. +#[tokio::test] +async fn broadcast_not_taken_by_the_node_is_pending() { + let (d, txid) = evidence_draft(); + let own = txid.clone(); + let reply = txid.clone(); + let mock = MockNodeRpc::new() + .with_send_raw_transaction(reply) + .with_raw_transaction_not_found() + .with_get_coin(move |h, i| Ok((h != own).then(|| hsd_coin(h, i, 10)))); + let out = + crate::commands::tx::classify_broadcast_outcome_with_client(&mock, &d.unsigned_tx_hex) + .await; + match out { + crate::commands::tx::BroadcastOutcome::NotTaken(t) => assert_eq!(t, txid), + other => panic!("expected NotTaken, got {other:?}"), + } + let checks = mock + .calls() + .into_iter() + .filter(|c| matches!(c, crate::tests::mock_node_rpc::RpcCall::RawTransaction(_))) + .count(); + assert_eq!( + checks, + crate::noncustodial::tx_evidence::TAKEN_CHECKS as usize + ); +} + +/// R1: on a node without a transaction index a send already mined reads as +/// not found; its output mined is the node having it. +#[tokio::test] +async fn broadcast_mined_without_tx_index_is_success() { + let (d, txid) = evidence_draft(); + let own = txid.clone(); + let mock = MockNodeRpc::new() + .with_send_raw_transaction(txid.clone()) + .with_raw_transaction_not_found() + .with_get_coin(move |h, i| Ok((h == own && i == 0).then(|| hsd_coin(h, i, 77)))); + let out = + crate::commands::tx::classify_broadcast_outcome_with_client(&mock, &d.unsigned_tx_hex) + .await; + match out { + crate::commands::tx::BroadcastOutcome::Mined(t, h) => { + assert_eq!((t.as_str(), h), (txid.as_str(), 77)) + } + other => panic!("expected Mined, got {other:?}"), + } +} + +/// R3: a check without hsd's answer ends the window with no verdict: the +/// node returned a txid, so the send stands. +#[tokio::test] +async fn broadcast_check_without_hsds_answer_is_success() { + let mock = MockNodeRpc::new() + .with_send_raw_transaction("abc123".to_string()) + .with_raw_transaction_err("node returned non-JSON body (status 502 Bad Gateway)"); + let out = crate::commands::tx::classify_broadcast_outcome_with_client(&mock, "deadbeef").await; + assert!( + matches!(out, crate::commands::tx::BroadcastOutcome::Success(_)), + "{out:?}" + ); + assert_eq!( + mock.calls().len(), + 2, + "one send, one check: {:?}", + mock.calls() + ); +} + +#[tokio::test] +async fn taken_reads_unknown_evidence_as_no_verdict() { + // Not found on every check, and the coin look-ups fail: no verdict. + let (d, txid) = evidence_draft(); + let mock = MockNodeRpc::new() + .with_raw_transaction_not_found() + .with_get_coin(|_, _| Err(AppError::Rpc("proxy page".into()))); + assert_eq!( + taken_by_node_with_client(&mock, &d.unsigned_tx_hex, &txid).await, + Taken::Unknown + ); } #[tokio::test] @@ -1580,10 +1707,11 @@ async fn broadcast_gates_pass_a_local_node_and_an_approved_remote_on_our_chain() .unwrap(); } -// ------- chain_evidence_with_client / node_has_tx_index_with_client (tx.rs) -- +// ------- noncustodial::tx_evidence -------------------------------------------- -use crate::commands::tx::{ - chain_evidence_with_client, node_has_tx_index_with_client, ChainEvidence, +use crate::noncustodial::tx_evidence::{ + chain_evidence_with_client, node_has_tx_index_with_client, taken_by_node_with_client, + ChainEvidence, Taken, }; /// A draft spending `[0x11;32]:0` and `[0x22;32]:1` into two outputs, and its @@ -1650,7 +1778,7 @@ async fn evidence_an_output_mined_is_mined_at_its_height() { let mock = MockNodeRpc::new() .with_get_coin(move |h, i| Ok((h == own && i == 1).then(|| hsd_coin(h, i, 812)))); assert_eq!( - chain_evidence_with_client(&mock, &d, &txid).await, + chain_evidence_with_client(&mock, &d.unsigned_tx_hex, &txid).await, ChainEvidence::Mined(812) ); } @@ -1662,7 +1790,7 @@ async fn evidence_inputs_all_unspent() { let mock = MockNodeRpc::new().with_get_coin(move |h, i| Ok((h != own).then(|| hsd_coin(h, i, 10)))); assert_eq!( - chain_evidence_with_client(&mock, &d, &txid).await, + chain_evidence_with_client(&mock, &d.unsigned_tx_hex, &txid).await, ChainEvidence::InputsUnspent ); let asked: Vec<_> = mock @@ -1685,7 +1813,7 @@ async fn evidence_one_input_spent() { let mock = MockNodeRpc::new().with_get_coin(|h, i| Ok((h == PREV_A).then(|| hsd_coin(h, i, 10)))); assert_eq!( - chain_evidence_with_client(&mock, &d, &txid).await, + chain_evidence_with_client(&mock, &d.unsigned_tx_hex, &txid).await, ChainEvidence::InputsSpent ); } @@ -1698,7 +1826,7 @@ async fn evidence_without_a_deciding_answer_is_unknown() { let mempool = MockNodeRpc::new().with_get_coin(move |h, i| Ok((h == own).then(|| hsd_coin(h, i, -1)))); assert_eq!( - chain_evidence_with_client(&mempool, &d, &txid).await, + chain_evidence_with_client(&mempool, &d.unsigned_tx_hex, &txid).await, ChainEvidence::Unknown ); // A failed lookup of an input. @@ -1711,13 +1839,13 @@ async fn evidence_without_a_deciding_answer_is_unknown() { } }); assert_eq!( - chain_evidence_with_client(&failing, &d, &txid).await, + chain_evidence_with_client(&failing, &d.unsigned_tx_hex, &txid).await, ChainEvidence::Unknown ); // A draft whose bytes are not this txid's. let any = MockNodeRpc::new().with_get_coin(|h, i| Ok(Some(hsd_coin(h, i, 5)))); assert_eq!( - chain_evidence_with_client(&any, &d, "ab").await, + chain_evidence_with_client(&any, &d.unsigned_tx_hex, "ab").await, ChainEvidence::Unknown ); assert!( diff --git a/src-tauri/src/tests/shakedex_purchase_state_tests.rs b/src-tauri/src/tests/shakedex_purchase_state_tests.rs index 39ca972..595e3c0 100644 --- a/src-tauri/src/tests/shakedex_purchase_state_tests.rs +++ b/src-tauri/src/tests/shakedex_purchase_state_tests.rs @@ -1095,6 +1095,77 @@ async fn rebroadcast_refused_by_hsd_is_lost() { assert_eq!(reservation(&conn), None, "funding coin released"); } +/// Spec honest-broadcast R5: the one rebroadcast is checked like any send. +/// hsd answered the txid but never had the purchase (its not-found on every +/// check, the coin it spends still unspent): it was not taken, so nothing was +/// paid, and the purchase is lost with the refusal's reason. +#[tokio::test] +async fn rebroadcast_not_taken_is_lost() { + use crate::noncustodial::tx::{Covenant, Input, Outpoint, Output, OutputAddress, Transaction}; + let mut tx = Transaction::new(); + tx.inputs.push(Input::new(Outpoint { + hash: [0x44; 32], + index: 0, + })); + tx.outputs.push(Output { + value: 1, + address: OutputAddress { + version: 0, + hash: vec![0xaa; 20], + }, + covenant: Covenant::default(), + }); + let (signed, txid) = (tx.to_hex(), tx.txid()); + let conn = seeded("broadcasted", "unconfirmed"); + conn.execute( + "UPDATE wallet_tx_drafts SET signed_tx_hex = ?1, txid = ?2", + params![signed, txid], + ) + .unwrap(); + conn.execute( + "UPDATE shakedex_purchases SET purchase_txid = ?1, missing_since_height = 1000", + params![txid], + ) + .unwrap(); + let funding = hex::encode([0x44u8; 32]); + let own = txid.clone(); + let rpc = MockNodeRpc::new() + .with_blockchain_info(tip(1006)) + .with_tx_by_hash(Value::Null) + .with_name_info(foreign_owner()) + .with_send_raw_transaction(txid.clone()) + .with_raw_transaction_not_found() + .with_get_coin(move |h, vout| { + if h == own { + Ok(None) + } else if h == funding { + Ok(Some(coin( + h, + vout, + &elsewhere(), + json!({"type": 0, "items": []}), + ))) + } else { + untraced(h, vout, Some(lock_coin())) + } + }); + + run(&conn, &rpc).await; + + let p = row(&conn); + assert_eq!( + rpc.count_matching(|c| matches!(c, RpcCall::SendRawTransaction(_))), + 1 + ); + assert_eq!(p.state, crate::db::queries::PurchaseState::Lost); + assert_eq!( + p.lost_reason.as_deref(), + Some("the node refused to take the purchase, so nothing was paid") + ); + assert_eq!(p.rebroadcast_count, 1); + assert_eq!(reservation(&conn), None, "funding coin released"); +} + #[tokio::test] async fn dropped_purchase_is_never_rebroadcast() { let conn = seeded("dropped", "unconfirmed"); diff --git a/src-tauri/src/tests/tx_lifecycle_tests.rs b/src-tauri/src/tests/tx_lifecycle_tests.rs index 60b4114..3f4a8a3 100644 --- a/src-tauri/src/tests/tx_lifecycle_tests.rs +++ b/src-tauri/src/tests/tx_lifecycle_tests.rs @@ -184,18 +184,33 @@ async fn full_lifecycle_build_sign_broadcast_succeeds() { let mut server = mockito::Server::new_async().await; // hsd returns the txid string as the JSON-RPC result. let node_txid = "abc0000000000000000000000000000000000000000000000000000000000def"; - // Two POSTs now: the chain-identity probe `broadcast_tx_draft` runs before - // it hands over a signed transaction, then `sendrawtransaction` itself. The - // probe gets this same txid-shaped body, fails to decode as blockchain info, - // and — a probe failure being no evidence of a mismatch — lets the broadcast - // proceed. + // Three POSTs: the chain-identity probe `broadcast_tx_draft` runs before + // it hands over a signed transaction, `sendrawtransaction` itself, then + // `getrawtransaction` to see that the node took it (hsd answers the txid + // whatever its mempool does). The probe gets the txid-shaped body, fails to + // decode as blockchain info, and — a probe failure being no evidence of a + // mismatch — lets the broadcast proceed. let m = server .mock("POST", "/") + .match_body(mockito::Matcher::Regex( + "getblockchaininfo|sendrawtransaction".into(), + )) .with_header("content-type", "application/json") .with_body(format!(r#"{{"result":"{node_txid}","error":null,"id":1}}"#)) .expect(2) .create_async() .await; + let taken = server + .mock("POST", "/") + .match_body(mockito::Matcher::Regex(format!( + "getrawtransaction.*{node_txid}" + ))) + .with_body(format!( + r#"{{"result":{{"txid":"{node_txid}","blockhash":null,"confirmations":0}},"error":null,"id":1}}"# + )) + .expect(1) + .create_async() + .await; let conn = seeded_conn(&server.url(), 2_000_000); let app = app_with(conn); @@ -238,6 +253,7 @@ async fn full_lifecycle_build_sign_broadcast_succeeds() { assert_eq!(result.status, "broadcasted"); assert_eq!(result.txid, node_txid); m.assert_async().await; + taken.assert_async().await; // The draft row reflects the broadcast. let row = draft_row(&app, &draft_id); @@ -3124,3 +3140,142 @@ async fn resolve_fee_rate_uses_per_profile_node_override() { "fee estimate must come from the override node, not the default fallback" ); } + +// --- honest broadcast: the node must have what it answered a txid for ----- + +/// Spec honest-broadcast R2: hsd answered `sendrawtransaction` with the txid +/// but never had the transaction (its not-found on every check, its coin +/// still unspent). The send is not sent: an error the dialog shows, the +/// draft `broadcast_pending` with the note, and its coins held. +#[tokio::test] +async fn broadcast_not_taken_keeps_its_coins_and_says_why() { + let mut server = mockito::Server::new_async().await; + let conn = seeded_conn(&server.url(), 2_000_000); + let app = app_with(conn); + let draft = build_send_hns_draft(app.state(), recv_addr(), 500_000, Some(1), None) + .await + .unwrap(); + unlock(&app, PROFILE); + sign_tx_draft_inner(&app.state(), &draft.id).await.unwrap(); + let txid = summary_of(&draft_row(&app, &draft.id)).txid.unwrap(); + + let _probe = server + .mock("POST", "/") + .match_body(mockito::Matcher::Regex("getblockchaininfo".into())) + .with_body(r#"{"result":{"blocks":500,"headers":500},"error":null,"id":1}"#) + .create_async() + .await; + let _send = server + .mock("POST", "/") + .match_body(mockito::Matcher::Regex("sendrawtransaction".into())) + .with_body(format!(r#"{{"result":"{txid}","error":null,"id":1}}"#)) + .create_async() + .await; + let checks = server + .mock("POST", "/") + .match_body(mockito::Matcher::Regex("getrawtransaction".into())) + .with_body(HSD_TX_NOT_FOUND) + .expect(crate::noncustodial::tx_evidence::TAKEN_CHECKS as usize) + .create_async() + .await; + mock_unmoved_coins(&mut server).await; + for vout in 0..2 { + server + .mock("GET", format!("/coin/{txid}/{vout}").as_str()) + .with_status(404) + .create_async() + .await; + } + + let err = broadcast_tx_draft(app.state(), draft.id.clone()) + .await + .expect_err("not taken is not sent"); + assert!(err.to_string().contains("did not take"), "{err}"); + checks.assert_async().await; + let row = draft_row(&app, &draft.id); + assert_eq!(row.status, "broadcast_pending"); + assert_eq!( + row.error_message.as_deref(), + Some(crate::noncustodial::tx_evidence::NOT_TAKEN) + ); + assert_eq!( + reserved_txids_for(&app, &draft.id), + vec![COIN_TXID.to_string()] + ); +} + +/// A draft already sent is not signed or sent again, and a `dropped` or +/// `failed` one, whose coins were released and may fund another draft by now, +/// is not either: nothing reaches the node, and the draft keeps its status. +#[tokio::test] +async fn sign_and_broadcast_refuse_a_sent_or_released_draft() { + let mut server = mockito::Server::new_async().await; + let none = server.mock("POST", "/").expect(0).create_async().await; + let conn = seeded_conn(&server.url(), 2_000_000); + for status in ["broadcasted", "confirmed", "dropped", "failed"] { + db::queries::insert_tx_draft(&conn, status, PROFILE, "send_hns", &draft_hex(), "{}", "{}") + .unwrap(); + conn.execute( + "UPDATE wallet_tx_drafts SET status = ?2, signed_tx_hex = ?3 WHERE id = ?1", + params![status, status, draft_hex()], + ) + .unwrap(); + } + let app = app_with(conn); + for status in ["broadcasted", "confirmed", "dropped", "failed"] { + let err = broadcast_tx_draft(app.state(), status.to_string()) + .await + .expect_err(status); + let want = if matches!(status, "broadcasted" | "confirmed") { + "already sent" + } else { + "coins were released" + }; + assert!(err.to_string().contains(want), "{status}: {err}"); + // Signing would flip it back to `signed` and open the same door. + unlock(&app, PROFILE); + let err = sign_tx_draft_inner(&app.state(), status) + .await + .expect_err(status); + assert!(err.to_string().contains(want), "sign {status}: {err}"); + assert_eq!(draft_row(&app, status).status, status); + } + none.assert_async().await; +} + +/// A `dropped` draft whose transaction is mined after all (another node held +/// it) is confirmed by the next poll, never left reading "not sent"; one +/// released longer ago than hsd keeps a mempool entry is no longer looked at. +#[tokio::test] +async fn refresh_confirms_a_dropped_draft_mined_after_all() { + let mut server = mockito::Server::new_async().await; + let (_info, _tx) = mock_node( + &mut server, + 500, + r#"{"result":{"confirmations":2,"blockhash":"abababababababababababababababababababababababababababababababab"},"error":null,"id":1}"#, + ) + .await; + let conn = seeded_conn(&server.url(), 2_000_000); + for id in ["late", "old"] { + seed_broadcasted_draft(&conn, id); + conn.execute( + "UPDATE wallet_tx_drafts SET status = 'dropped', signed_tx_hex = unsigned_tx_hex WHERE id = ?1", + params![id], + ) + .unwrap(); + } + conn.execute( + "UPDATE wallet_tx_drafts SET updated_at = datetime('now', '-73 hours') WHERE id = 'old'", + [], + ) + .unwrap(); + let app = app_with(conn); + + let res = refresh_tx_confirmations(app.state(), None).await.unwrap(); + assert_eq!(res["revived"], 1, "{res}"); + let row = draft_row(&app, "late"); + assert_eq!(row.status, "confirmed"); + assert_eq!(row.confirmation_height, Some(499)); + assert_eq!(row.error_message, None); + assert_eq!(draft_row(&app, "old").status, "dropped"); +} From 8c44af01de76142b7075290dbb440503ec9b36ae Mon Sep 17 00:00:00 2001 From: Dmitriy Zhavoronkov Date: Thu, 8 Oct 2026 19:09:19 +0200 Subject: [PATCH 2/2] fix(names): refuse a renew, finalize or redeem hsd's consensus would reject The name builders built transactions hsd refuses: a RENEW within tree_interval of the name's last renewal (bad-renewal-premature), a FINALIZE before the transfer lockup is over (bad-finalize-maturity; the Finalize button was gated, the builders were not, and the gate allowed it when the transfer height was unknown), and a REDEEM of the winning reveal, which is the name's owner coin until REGISTER (bad-redeem-owner; the old filter read the wallet's database and missed a name the wallet never tracked). Each now asks the node (getnameinfo and the tip, judged at tip + 1 as hsd does) and refuses before a draft is persisted, saying how many blocks are left; single and batch renew, finalize, finalize-with-payment and redeem all go through the same checks (commands/draft_ctx.rs). The UI withholds Renew the same way ("can be renewed in N blocks"). Seven live regtest tests built exactly these transactions and passed only because the broadcast never said hsd refused them. They now run valid scenarios and assert the effect on chain: a rival bidder outbids for redeem, renew and finalize mine the required blocks first and check info.renewal / the new owner, and the batch transfer test syncs between its two finalizes (both had spent one funding coin, which hsd held as an orphan). New live test: live_redeem_of_the_winning_reveal_is_refused. Tests: unit 2959 passed, each guard and wrapper call checked to fail its tests when removed; full live suite on a fresh regtest chain, Shakedex and the node without a transaction index included, 80 passed. clippy -D warnings, fmt and prettier clean. --- CHANGELOG.md | 1 + docs/REGTEST_TESTING.md | 21 +- docs/USER_MANUAL.md | 2 +- src-tauri/src/commands/draft_ctx.rs | 129 +++++ src-tauri/src/commands/names.rs | 162 ++++-- src-tauri/src/noncustodial/network.rs | 8 + src-tauri/src/tests/draft_ctx_tests.rs | 204 +++++++ src-tauri/src/tests/live_node_it.rs | 523 ++++++++++++++---- .../src/tests/name_capabilities_tests.rs | 1 + src-tauri/src/tests/names_cmd_tests.rs | 234 +++++++- src-tauri/src/tests/network_tests.rs | 13 + 11 files changed, 1154 insertions(+), 144 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 487ac11..fb20757 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ ### Fixed - **A send the node did not take says "Not sent".** hsd answers `sendrawtransaction` with the txid whatever its mempool does with the transaction, so a send whose coins were already spent, or that the mempool turned away for any other reason, read as sent and was quietly marked dropped ten minutes later. The wallet now looks the transaction up right after sending (up to five times over two seconds, since hsd adds it to its mempool after answering). When the node does not have it, the dialog says "The node did not take the transaction", the draft waits with its coins held, and the confirmation check resolves it later: dropped, saying another transaction spent its coins, when that is what the chain shows. A send already mined by the time it is looked up (a node without a transaction index finds it only by its outputs) reads as confirmed. A Shakedex purchase follows the same rule, and its one automatic rebroadcast too: a rebroadcast the node does not take loses the purchase with nothing paid. +- **A renewal, finalize or redeem the network would refuse is refused before it is signed.** hsd answers `sendrawtransaction` with the txid even when its consensus rules turn the transaction away, so three such name actions read as sent. A renewal right after the name was registered, renewed or finalized (hsd accepts one only a tree interval later, 36 blocks on mainnet) now says how many blocks are left, and Renew waits that long before it is offered. A finalize inside the transfer lockup says how long the lockup still runs. A redeem never spends the winning reveal, which the network refuses to redeem because until REGISTER it owns the name: the wallet now asks the node which coin owns the name instead of relying on its own records, which knew nothing of a name it never tracked, and when that reveal is all there is it says the bid won and the name should be registered. Each check reads the name and the chain tip from the node at the moment the draft is built. A batch redeem also takes every losing reveal on a name, as the single redeem does, instead of only the newest bid's. - **A send already sent, dropped or failed is not signed or sent again.** Signing or broadcasting such a draft again would turn a confirmed one back into "signed", or spend coins a dropped or failed one had released, which another draft may hold by now. Both are refused before anything reaches the node; build a new send instead. - **A send marked dropped or failed that is mined after all turns confirmed.** Dropped and failed were final, so a transaction another node still held, or that this node forgot when restarted, kept reading "not sent" once mined, inviting a second payment. The confirmation check now keeps looking for such a transaction for 72 hours (hsd's mempool expiry) and confirms it once it is mined. - **A sent transaction is no longer reported as "not sent" because the node keeps no transaction index.** hsd finds a transaction by its txid only in its mempool and, once mined, in its transaction index (`--index-tx`). On a remote node started with the address index alone, which is enough to send from, every mined transaction read as unknown: ten minutes later the wallet marked the send dropped (or failed) with "The coins were not moved" and released its coins, inviting you to pay again, although the payment had gone through. When hsd does not find a transaction, the wallet now asks the chain about the coins instead, which needs no transaction index: one of the transaction's own outputs mined is the block it is in, and it is confirmed at that height; every coin it spends still unspent is what "dropped, the coins were not moved" now rests on. When another transaction spent its coins, a node with the index drops it and says so ("Another transaction spent this transaction's coins … Nothing was paid by this transaction"); a node without the index cannot tell that apart from a mined transaction whose outputs were spent since, and the wallet leaves it as sent. diff --git a/docs/REGTEST_TESTING.md b/docs/REGTEST_TESTING.md index ff2e235..adfb6f6 100644 --- a/docs/REGTEST_TESTING.md +++ b/docs/REGTEST_TESTING.md @@ -217,8 +217,9 @@ The following scenarios must be validated against a running regtest node to conf |------|--------|----------------| | 1 | Register a name (from Scenario 1) | Name is owned | | 2 | Transfer the name to another regtest address | TRANSFER tx broadcasted | -| 3 | Finalize the transfer | FINALIZE tx broadcasted; name moves to recipient | -| 4 | Verify original wallet no longer `ownsName` | `capabilities.ownsName` is `false` | +| 3 | Finalize the transfer right away | Refused: "the transfer of '…' is still locked for N more blocks" | +| 4 | Mine the transfer lockup (10 blocks on regtest), then finalize | FINALIZE tx broadcasted; name moves to recipient | +| 5 | Verify original wallet no longer `ownsName` | `capabilities.ownsName` is `false` | ### Scenario 4: Missed Reveal (simulated) @@ -263,9 +264,9 @@ This runs the following tests: | Test | Lifecycle | |------|-----------| | `live_auction_open_bid_reveal_register` | Winner: OPEN → BID → REVEAL → REGISTER | -| `live_auction_open_bid_reveal_redeem` | Loser redeem: OPEN → BID → REVEAL → REDEEM | -| `live_auction_register_transfer_finalize` | Post-win: REGISTER → TRANSFER → FINALIZE | -| `live_batch_transfer_two_names` | Batch: acquire 2 → BATCH TRANSFER → FINALIZE each | +| `live_auction_open_bid_reveal_redeem` | Loser redeem: OPEN → BID, a rival wallet on the same node bids higher → both REVEAL → REDEEM; the redeem spends exactly the losing reveal, returns its value, and the rival's reveal stays the owner | +| `live_auction_register_transfer_finalize` | Post-win: REGISTER → TRANSFER → FINALIZE refused inside the lockup → mine the rest of it → FINALIZE; the owner coin is at the recipient and `transfer` is 0 | +| `live_batch_transfer_two_names` | Batch: acquire 2 → BATCH TRANSFER → lockup → FINALIZE each, syncing in between; both owner coins are at the recipient | ### Send-path money invariants (Group A) @@ -331,7 +332,7 @@ HNS_IT_NOINDEX_NODE_URL=http://127.0.0.1:24037 HNS_IT_NOINDEX_NODE_API_KEY=test | Test | Asserts | |------|---------| | `live_update_records` | UPDATE writes records; name stays CLOSED, owner value preserved | -| `live_renew_extends_lease` | Guards commit `78bba67`: RENEW uses `getblockhash` UNREVERSED (no `bad-register-renewal`) | +| `live_renew_extends_lease` | RENEW straight after REGISTER is refused (`bad-renewal-premature`: not before `renewal + tree_interval`); after that many blocks it lands, `renewal` moves to the block it was mined in and the value is kept. Guards commit `78bba67`: RENEW uses `getblockhash` UNREVERSED (no `bad-register-renewal`) | | `live_cancel_reverts_transfer` | CANCEL clears a pending transfer; name still owned | | `live_revoke_burns_control` | REVOKE → name state REVOKED | @@ -341,18 +342,20 @@ HNS_IT_NOINDEX_NODE_URL=http://127.0.0.1:24037 HNS_IT_NOINDEX_NODE_API_KEY=test |------|---------| | `live_batch_bid_two_names` | Shared-lockup batch bid → both BIDs land, commitments persisted | | `live_batch_reveal_two_names` | Batch reveal after batch bid → both reveals accepted | -| `live_batch_redeem_two_names` | Losing batch → batch redeem reclaims both lockups | -| `live_batch_renew_two_names` | Shared `renewal_block` → both renewed | +| `live_batch_redeem_two_names` | A rival wallet outbids this one on both names; one batch REDEEM spends both losing reveals, one REDEEM output per name, and the rival's reveals stay the owners | +| `live_batch_renew_two_names` | Batch refused while the last-registered name is too recently renewed; after the tree interval, shared `renewal_block` → both names' `renewal` moves to the block it was mined in | +| `live_batch_large_covenant_count` | 20 names renewed in one transaction once the last one may be renewed; every name's `renewal` moves to that block | | `live_batch_finalize_two_names` | Single-tx batch finalize after batch transfer + lockup | ### Covenant invariant + timing negatives (Group F) | Test | Asserts | |------|---------| -| `live_premature_finalize_rejected` | Finalize before `transfer_lockup` (10) → rejected; after → accepted | +| `live_premature_finalize_rejected` | Finalize before `transfer_lockup` (10) → the builder refuses it ("still locked for 9 more blocks") and persists nothing, as hsd would refuse it (`bad-finalize-maturity`); after → accepted and the name is at the recipient | | `live_bid_lockup_invariant_and_reveal_value` | On-chain: BID value == lockup; REVEAL value == true bid | | `live_register_value_is_clearing_price` | REGISTER output value == `getnameinfo.info.value` | | `live_redeem_when_won_rejected` | Winner cannot `build_redeem_draft` (no losing reveal to reclaim) | +| `live_redeem_of_the_winning_reveal_is_refused` | A lone bidder's reveal, before REGISTER and with the name never tracked, is the owner coin the node reports: `build_redeem_draft` refuses it ("won the auction"), as hsd would (`bad-redeem-owner`), and the coin stays unspent | | `live_double_open_and_double_bid_guarded` | Second OPEN/BID while first pending → command-level rejection | ### Atomic swap + signing + capability (Group G) diff --git a/docs/USER_MANUAL.md b/docs/USER_MANUAL.md index e474773..c56fb98 100644 --- a/docs/USER_MANUAL.md +++ b/docs/USER_MANUAL.md @@ -360,7 +360,7 @@ In the Name Actions modal for an owned name, click **Show all actions**: | Action | What it does | |--------|--------------| | **Update** | Replace the on-chain DNS records (uses the DNS editor). | -| **Renew** | Extend the name's expiry. | +| **Renew** | Extend the name's expiry. Not right after the name was registered, renewed or finalized: the network accepts a renewal only 36 blocks (about 6 hours) later on mainnet and testnet, 5 on regtest, and until then the button says how many blocks are left. | | **Transfer** | Start a transfer to another Handshake address. Enters a `TRANSFER` covenant. | | **Finalize** | Complete a transfer after the lockup period (mainnet: ~2 days). | | **Cancel** | Revert a pending transfer before it's finalized. | diff --git a/src-tauri/src/commands/draft_ctx.rs b/src-tauri/src/commands/draft_ctx.rs index 3db5a8b..486a529 100644 --- a/src-tauri/src/commands/draft_ctx.rs +++ b/src-tauri/src/commands/draft_ctx.rs @@ -232,3 +232,132 @@ pub(crate) async fn renewal_block( h.copy_from_slice(&bytes); Ok(h) } + +// --- consensus guards ------------------------------------------------------- +// +// A draft hsd's consensus would refuse is refused here, before it is persisted +// and signed: hsd 8.0.0's `sendrawtransaction` hands back the txid even when +// its mempool turns the transaction away, so a broadcast cannot be trusted to +// say so. Each guard asks the node itself — `getnameinfo` and the tip — what +// `chain.js` will judge the transaction against, at the next block's height +// (`tip + 1`), never the wallet's cached copy of either. + +/// The `info` object of hsd's `getnameinfo` reply for `name`. `null` is a name +/// with no state; a reply without the key is not hsd's answer (it always sends +/// one). +async fn node_name_info(client: &dyn NodeRpc, name: &str) -> Result { + let mut reply = client.get_name_info(name).await?; + match reply.get_mut("info").map(serde_json::Value::take) { + None => Err(AppError::Rpc(format!( + "node did not report the name's info for '{name}'" + ))), + Some(serde_json::Value::Null) => Err(AppError::InvalidInput(format!( + "name '{name}' has no on-chain state" + ))), + Some(info) => Ok(info), + } +} + +/// A block height field of `getnameinfo.info` (`renewal`, `transfer`), read +/// exactly as the node reports it: a reply without it gives no verdict. +fn info_height(info: &serde_json::Value, name: &str, key: &str) -> Result { + info.get(key) + .and_then(|v| v.as_u64()) + .and_then(|v| u32::try_from(v).ok()) + .map(i64::from) + .ok_or_else(|| AppError::Rpc(format!("node did not report the name's {key} for '{name}'"))) +} + +fn blocks_word(n: i64) -> &'static str { + if n == 1 { + "block" + } else { + "blocks" + } +} + +/// Refuses a RENEW of `name` that hsd would reject as `bad-renewal-premature`: +/// a name may be renewed only once `tree_interval` blocks have passed since it +/// was last registered, renewed or finalized (`ns.renewal`). +pub(crate) async fn ensure_renew_not_premature( + client: &dyn NodeRpc, + network: Network, + name: &str, +) -> Result<(), AppError> { + let info = node_name_info(client, name).await?; + let renewal = info_height(&info, name, "renewal")?; + let tip = client.get_blockchain_info().await?.blocks; + let blocks = network.name_params().blocks_until_renew(renewal, tip); + if blocks > 0 { + return Err(AppError::InvalidInput(format!( + "'{name}' was renewed too recently: it can be renewed in {blocks} {}", + blocks_word(blocks) + ))); + } + Ok(()) +} + +/// Refuses a FINALIZE of `name` that hsd would reject: with no transfer +/// recorded on the name, or as `bad-finalize-maturity` while the transfer +/// lockup is not over (the same [`NameParams::blocks_until_finalize`] the +/// Finalize button is offered by). +/// +/// [`NameParams::blocks_until_finalize`]: crate::noncustodial::network::NameParams::blocks_until_finalize +pub(crate) async fn ensure_finalize_matured( + client: &dyn NodeRpc, + network: Network, + name: &str, +) -> Result<(), AppError> { + let info = node_name_info(client, name).await?; + let transfer = info_height(&info, name, "transfer")?; + if transfer == 0 { + return Err(AppError::InvalidInput(format!( + "the node reports no transfer of '{name}': nothing to finalize" + ))); + } + let tip = client.get_blockchain_info().await?.blocks; + let blocks = network.name_params().blocks_until_finalize(transfer, tip); + if blocks > 0 { + return Err(AppError::InvalidInput(format!( + "the transfer of '{name}' is still locked for {blocks} more {}", + blocks_word(blocks) + ))); + } + Ok(()) +} + +/// The reveal coins in `coins` that a REDEEM may spend: every one but the +/// name's owner coin as the node reports it (`getnameinfo.info.owner`). Until +/// REGISTER spends it, the winning reveal IS the owner coin, and hsd rejects +/// redeeming it (`bad-redeem-owner`), taking the whole transaction down with +/// it. Refused when `coins` held nothing but the owner coin; an empty `coins` +/// is returned as it is, for the caller to say there is nothing to redeem. +pub(crate) async fn exclude_owner_reveal( + client: &dyn NodeRpc, + name: &str, + coins: Vec, +) -> Result, AppError> { + if coins.is_empty() { + return Ok(coins); + } + let info = node_name_info(client, name).await?; + let owner = info.get("owner"); + let hash = owner.and_then(|o| o.get("hash")).and_then(|h| h.as_str()); + let index = owner.and_then(|o| o.get("index")).and_then(|i| i.as_u64()); + let (Some(hash), Some(index)) = (hash, index) else { + return Err(AppError::Rpc(format!( + "node did not report the name's owner for '{name}'" + ))); + }; + let losing: Vec<_> = coins + .into_iter() + .filter(|c| !(c.txid.eq_ignore_ascii_case(hash) && u64::from(c.vout) == index)) + .collect(); + if losing.is_empty() { + return Err(AppError::InvalidInput(format!( + "your reveal for '{name}' won the auction: it is the name's owner coin and cannot \ + be redeemed (register the name instead)" + ))); + } + Ok(losing) +} diff --git a/src-tauri/src/commands/names.rs b/src-tauri/src/commands/names.rs index e25a8f0..7881fbc 100644 --- a/src-tauri/src/commands/names.rs +++ b/src-tauri/src/commands/names.rs @@ -25,6 +25,7 @@ use crate::noncustodial::types::TxDraftSummary; use crate::noncustodial::{address, bids, covenants, names, resource}; use crate::AppState; +use super::draft_ctx::{ensure_finalize_matured, ensure_renew_not_premature, exclude_owner_reveal}; // Re-exported: the `*_inner` builders take these, and their tests name them // through this module. pub(crate) use super::draft_ctx::{ @@ -360,6 +361,11 @@ pub(crate) struct NameActionContext { /// the node would accept must not be refused on a guess. pub transfer_height: Option, pub current_height: Option, + /// The block the name was last registered, renewed or finalized in + /// (`getnameinfo.renewal`). hsd refuses a RENEW until `tree_interval` + /// blocks have passed since (`bad-renewal-premature`); `None` means we + /// cannot tell, and Renew is not refused on a guess. + pub renewal_height: Option, /// The `reveal_txid` stamped on the bid commitment row (if any). pub reveal_txid: Option, /// Status of the local tx_draft matching `reveal_txid` (if one exists). @@ -574,6 +580,10 @@ pub(crate) fn find_name_action_context( .as_ref() .and_then(|t| t.transfer_height) .filter(|h| *h > 0); + let renewal_height = tracked_row + .as_ref() + .and_then(|t| t.renewal_height) + .filter(|h| *h > 0); // The same call is already propagated further down this file; a failure // here read as "height unknown", which quietly widens every gate that // compares a height against the tip. @@ -590,6 +600,7 @@ pub(crate) fn find_name_action_context( transfer_has_items: transfer, transfer_height, current_height, + renewal_height, existing_bid_count, has_pending_open, pending_broadcast_action, @@ -1053,10 +1064,28 @@ pub(crate) fn build_name_action_capabilities( // just as UPDATE does (`chain.js`), so extending the registration ends a // transfer in flight without saying so. Cancel the transfer first and the // renewal is one click away; the reverse order loses the transfer silently. + // + // Nor is Renew offered before hsd would take it: the same rule + // `build_renew_draft` refuses a premature RENEW by, read + // here from the tracked renewal height instead of a fresh node answer. + let blocks_until_renew = action_ctx + .renewal_height + .zip(action_ctx.current_height) + .map(|(renewal, tip)| network.name_params().blocks_until_renew(renewal, tip)) + .filter(|b| *b > 0); let can_renew = NameActionCapability { - allowed: can_spend_as_owner && !transfer_pending, + allowed: can_spend_as_owner && !transfer_pending && blocks_until_renew.is_none(), reason: owner_gate().or_else(|| { - transfer_pending.then(|| "a transfer is pending — renewing would cancel it".into()) + if transfer_pending { + Some("a transfer is pending — renewing would cancel it".into()) + } else { + blocks_until_renew.map(|blocks| { + format!( + "the name was renewed too recently — it can be renewed in {blocks} block{}", + if blocks == 1 { "" } else { "s" } + ) + }) + } }), }; @@ -2161,25 +2190,19 @@ pub async fn build_redeem_draft( let coins = { let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; let nh_hex = hex::encode(names::hash_name(&name)?); - let all = queries::find_unspent_covenant_utxos_by_name_hash( + queries::find_unspent_covenant_utxos_by_name_hash( &conn, &ctx.profile_id, sync::COV_REVEAL as i64, &nh_hex, - )?; - // The winning reveal IS the name's owner coin until REGISTER spends it, - // and consensus rejects redeeming it (`bad-redeem-owner`) — which would - // take the whole transaction down with it. - let owner = queries::get_name_coin(&conn, &ctx.profile_id, &name)?; - all.into_iter() - .filter(|c| { - owner - .as_ref() - .map(|o| !(o.txid == c.txid && o.vout == c.vout)) - .unwrap_or(true) - }) - .collect::>() + )? }; + // The winning reveal IS the name's owner coin until REGISTER spends it, + // and consensus rejects redeeming it (`bad-redeem-owner`). Which coin that + // is comes from the node: the wallet's own record of the owner is missing + // whenever the name was never tracked, which is exactly when a reveal that + // won looks like one that lost. + let coins = exclude_owner_reveal(&client, &name, coins).await?; if coins.is_empty() { return Err(AppError::NotFound(format!( "no unspent losing reveal coin for '{name}' (sync first?)" @@ -2386,6 +2409,7 @@ pub async fn build_renew_draft( let ctx = load_ctx(&state)?; let (coin, ns) = owner_coin_and_state(&state, &ctx, &name).await?; let client = ctx.node.clone(); + ensure_renew_not_premature(&client, ctx.network, &name).await?; let rblock = renewal_block(&client, ctx.network).await?; let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; build_renew_draft_inner(&conn, &ctx, &name, fee_rate, &ns, &coin, &rblock) @@ -2500,6 +2524,7 @@ pub async fn build_finalize_draft( let ctx = load_ctx(&state)?; let (coin, ns) = owner_coin_and_state(&state, &ctx, &name).await?; let client = ctx.node.clone(); + ensure_finalize_matured(&client, ctx.network, &name).await?; let rblock = renewal_block(&client, ctx.network).await?; let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; build_finalize_draft_inner(&conn, &ctx, &name, fee_rate, &ns, &coin, &rblock) @@ -2687,6 +2712,7 @@ pub async fn build_batch_renew_draft( for name in &names { let nh = names::hash_name(name)?; let (coin, ns) = owner_coin_and_state(&state, &ctx, name).await?; + ensure_renew_not_premature(&client, ctx.network, name).await?; per_name.push((name.clone(), nh, coin, ns)); } @@ -3022,27 +3048,33 @@ pub async fn build_batch_redeem_draft( for name in &names { let nh = names::hash_name(name)?; let ns = fetch_name_state(&client, name).await?; - let (bid, coin) = { + // Every reveal the wallet holds for the name, as the single redeem + // takes them: a wallet that outbid itself has several, each on its own + // address, so the newest commitment's address finds only one of them. + let (bid, coins) = { let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; let bid = queries::get_bid_commitment(&conn, &ctx.profile_id, name)? .ok_or_else(|| AppError::NotFound(format!("no bid for '{}'", name)))?; - let coin = queries::find_unspent_covenant_utxo( + let coins = queries::find_unspent_covenant_utxos_by_name_hash( &conn, &ctx.profile_id, - &bid.address, sync::COV_REVEAL as i64, - name, &hex::encode(nh), - )? - .ok_or_else(|| { - AppError::NotFound(format!( - "no unspent losing reveal coin for '{}' (sync first?)", - name - )) - })?; - (bid, coin) + )?; + (bid, coins) }; - per_name.push((name.clone(), nh, bid, coin, ns)); + // The winning reveal is the name's owner coin until REGISTER spends + // it, and hsd refuses to redeem it (`bad-redeem-owner`). + let coins = exclude_owner_reveal(&client, name, coins).await?; + if coins.is_empty() { + return Err(AppError::NotFound(format!( + "no unspent losing reveal coin for '{}' (sync first?)", + name + ))); + } + for coin in coins { + per_name.push((name.clone(), nh, bid.clone(), coin, ns.clone())); + } } let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; @@ -3053,8 +3085,9 @@ pub async fn build_batch_redeem_draft( /// `State`. The caller must hold the DB mutex for the full duration. /// /// `per_name` is `(name, name_hash, bid_commitment, reveal_coin, -/// on_chain_state)` for every name in the batch. The wrapper's per-name RPC + -/// DB prefetch resolves all of these; the inner builds the REDEEM covenant +/// on_chain_state)` for every losing reveal coin in the batch — several rows +/// for a name the wallet outbid itself on. The wrapper's per-name RPC + DB +/// prefetch resolves all of these; the inner builds the REDEEM covenant /// outputs and persists the batch draft. pub(crate) fn build_batch_redeem_draft_inner( conn: &rusqlite::Connection, @@ -3074,7 +3107,11 @@ pub(crate) fn build_batch_redeem_draft_inner( address: coin.address.clone(), covenant: cov, }); - batch_names.push(name); + // One row per reveal coin: a name with several losing reveals is + // still one name in the batch. + if !batch_names.contains(&name) { + batch_names.push(name); + } } let res = actions::build_batch_plan( @@ -3126,6 +3163,7 @@ pub async fn build_batch_finalize_draft( let nh = names::hash_name(name)?; let raw = names::raw_name(name)?; let (coin, ns) = owner_coin_and_state(&state, &ctx, name).await?; + ensure_finalize_matured(&client, ctx.network, name).await?; per_name.push((name.clone(), nh, raw, coin, ns)); } @@ -3145,7 +3183,8 @@ pub(crate) type PerNameOwner = Vec<(String, [u8; 32], queries::NameCoin, NameSta /// Per-name prefetch for a batch that spends a bid: the name, its hash, the /// commitment row the bid was made from, the coin it created, and the on-chain -/// state. Used by reveal and by redeem, which ignores the commitment. +/// state. Used by reveal and by redeem, which ignores the commitment and has +/// one row per losing reveal coin. pub(crate) type PerNameBid = Vec<( String, [u8; 32], @@ -3275,6 +3314,7 @@ pub async fn build_finalize_with_payment_draft( let rate = self::fee_rate(&ctx, fee_rate); let (coin, ns) = owner_coin_and_state(&state, &ctx, &name).await?; let client = ctx.node.clone(); + ensure_finalize_matured(&client, ctx.network, &name).await?; let rblock = renewal_block(&client, ctx.network).await?; let conn = state.db.lock().map_err(|e| AppError::Lock(e.to_string()))?; @@ -3414,6 +3454,7 @@ mod tests { owner_spend_in_flight: false, transfer_height: None, current_height: None, + renewal_height: None, reveal_txid: None, reveal_draft_status: None, bid_value_doos: None, @@ -4611,6 +4652,61 @@ mod tests { assert!(caps.can_finalize.allowed, "the next block may carry it"); } + /// hsd refuses a RENEW until `tree_interval` blocks have passed since the + /// name was last registered, renewed or finalized (`bad-renewal-premature`, + /// chain.js) — the rule `build_renew_draft` refuses one by. Offering the + /// button meanwhile sends the user at that refusal. + #[test] + fn renew_waits_a_tree_interval_after_the_last_renewal() { + // Regtest tree interval is 5. Renewed at 800, a renew is valid in + // block 805 — i.e. once the tip reaches 804. + let just_renewed = NameActionContext { + has_owner_coin: true, + owner_covenant_type: Some(COV_REGISTER as i64), + transfer_has_items: Some(false), + renewal_height: Some(800), + current_height: Some(802), + ..ctx_default() + }; + let caps = |ctx: &NameActionContext| { + build_name_action_capabilities( + "n".into(), + "CLOSED".into(), + "CLOSED", + None, + ctx, + true, + false, + None, + Network::Regtest, + ) + }; + let early = caps(&just_renewed); + assert!(!early.can_renew.allowed, "renewed too recently"); + let reason = early.can_renew.reason.unwrap_or_default(); + assert!( + reason.contains("in 2 blocks"), + "the reason must say how many blocks are left, got {reason:?}" + ); + + let ready = NameActionContext { + current_height: Some(804), + ..just_renewed + }; + assert!( + caps(&ready).can_renew.allowed, + "the next block may carry it" + ); + + // Without a renewal height we cannot say: Renew stays offered. + let unknown = NameActionContext { + renewal_height: None, + current_height: Some(801), + ..ready + }; + assert!(caps(&unknown).can_renew.allowed); + } + /// Without heights we cannot say, and refusing an action the node would /// accept is its own kind of wrong. A transfer with no recorded height /// stays offered. diff --git a/src-tauri/src/noncustodial/network.rs b/src-tauri/src/noncustodial/network.rs index 03bb186..fb35ac9 100644 --- a/src-tauri/src/noncustodial/network.rs +++ b/src-tauri/src/noncustodial/network.rs @@ -225,6 +225,14 @@ impl NameParams { (transfer_height + i64::from(self.transfer_lockup) - (tip + 1)).max(0) } + /// Blocks until a RENEW of a name last renewed (registered, renewed or + /// finalized) at `renewal_height` is accepted, 0 once it is. hsd refuses + /// one while `height < renewal + tree_interval` (`bad-renewal-premature`), + /// and a transaction built now is judged at `tip + 1`. + pub fn blocks_until_renew(self, renewal_height: i64, tip: i64) -> i64 { + (renewal_height + i64::from(self.tree_interval) - (tip + 1)).max(0) + } + /// The height at which a name last renewed at `renewal` expires: the end /// of its renewal window, or for a `claimed` (reserved) name not before /// the claim period is over. hsd `NameState.isExpired`: "Claimed names can diff --git a/src-tauri/src/tests/draft_ctx_tests.rs b/src-tauri/src/tests/draft_ctx_tests.rs index a8e30da..1414199 100644 --- a/src-tauri/src/tests/draft_ctx_tests.rs +++ b/src-tauri/src/tests/draft_ctx_tests.rs @@ -115,3 +115,207 @@ async fn strict_name_state_without_info_key_is_an_rpc_error() { let err = strict(json!({"info": null})).await.unwrap_err(); assert!(matches!(err, AppError::InvalidInput(_)), "{err:?}"); } + +// --- consensus guards: RENEW, FINALIZE, REDEEM ------------------------------- + +use crate::commands::draft_ctx::{ + ensure_finalize_matured, ensure_renew_not_premature, exclude_owner_reveal, +}; +use crate::db::queries::NameCoin; +use crate::noncustodial::network::Network; +use crate::noncustodial::rpc::BlockchainInfo; +use crate::tests::mock_node_rpc::RpcCall; + +const OWNER_TXID: &str = "80cfddda07cf7baedbafc078fd2a013deb2a323db389898da771f2d1041fcfdd"; + +/// A regtest `getnameinfo` reply, copied from the live regtest node for a +/// registered name with a transfer pending: renewed at 9735, transferred at +/// 9838, owner coin `OWNER_TXID:0`. +fn regtest_name_info() -> Value { + json!({ + "start": {"reserved": false, "week": 17, "start": 34}, + "info": { + "name": "shkreg2205014174", + "nameHash": "03a3cdeb7bfad6b0b1a2dd6ac670d5a00445955122a750335a2e8772e24e8d59", + "state": "CLOSED", + "height": 9701, + "renewal": 9735, + "owner": {"hash": OWNER_TXID, "index": 0}, + "value": 500000, + "highest": 1000000, + "data": "000601117368616b656465782d636c692d73656c6c", + "transfer": 9838, + "revoked": 0, + "claimed": 0, + "renewals": 1, + "registered": true, + "expired": false, + "weak": false, + "stats": { + "renewalPeriodStart": 9735, + "renewalPeriodEnd": 14735, + "blocksUntilExpire": 839, + "daysUntilExpire": 5.83, + "transferLockupStart": 9838, + "transferLockupEnd": 9848, + "blocksUntilValidFinalize": -4048, + "hoursUntilValidFinalize": -674.67 + } + } + }) +} + +fn node_at(tip: i64, reply: Value) -> MockNodeRpc { + MockNodeRpc::new() + .with_name_info(reply) + .with_blockchain_info(BlockchainInfo { + blocks: tip, + ..Default::default() + }) +} + +/// hsd refuses a RENEW while `height < renewal + treeInterval` (regtest 5), +/// judged at `tip + 1`: renewed at 9735, the first block it may be renewed in +/// is 9740, built on tip 9739. +#[tokio::test] +async fn renew_before_a_tree_interval_since_the_last_renewal_is_refused() { + let node = node_at(9738, regtest_name_info()); + let err = ensure_renew_not_premature(&node, Network::Regtest, "shkreg2205014174") + .await + .unwrap_err(); + assert!( + matches!(&err, AppError::InvalidInput(m) if m.contains("renewed in 1 block")), + "{err:?}" + ); + assert!(node + .calls() + .contains(&RpcCall::NameInfo("shkreg2205014174".into()))); + + let node = node_at(9739, regtest_name_info()); + ensure_renew_not_premature(&node, Network::Regtest, "shkreg2205014174") + .await + .expect("renewable at tip + 1 = renewal + tree interval"); +} + +/// Without the renewal height there is no verdict, and no renewal on a guess. +#[tokio::test] +async fn renew_guard_without_the_renewal_height_is_an_rpc_error() { + let mut reply = regtest_name_info(); + reply["info"].as_object_mut().unwrap().remove("renewal"); + let err = ensure_renew_not_premature(&node_at(20_000, reply), Network::Regtest, "x") + .await + .unwrap_err(); + assert!( + matches!(&err, AppError::Rpc(m) if m.contains("renewal")), + "{err:?}" + ); +} + +/// hsd refuses a FINALIZE while `height < transfer + transferLockup` (regtest +/// 10): transferred at 9838, finalizable in block 9848, built on tip 9847. +#[tokio::test] +async fn finalize_before_the_transfer_lockup_is_over_is_refused() { + let node = node_at(9846, regtest_name_info()); + let err = ensure_finalize_matured(&node, Network::Regtest, "shkreg2205014174") + .await + .unwrap_err(); + assert!( + matches!(&err, AppError::InvalidInput(m) if m.contains("still locked for 1 more block")), + "{err:?}" + ); + + let node = node_at(9847, regtest_name_info()); + ensure_finalize_matured(&node, Network::Regtest, "shkreg2205014174") + .await + .expect("finalizable once the lockup is over"); +} + +/// `transfer: 0` is hsd saying no transfer is pending: a FINALIZE of it is +/// refused by the node (it asserts `ns.transfer !== 0`). +#[tokio::test] +async fn finalize_of_a_name_with_no_transfer_is_refused() { + let mut reply = regtest_name_info(); + reply["info"]["transfer"] = json!(0); + let err = ensure_finalize_matured(&node_at(20_000, reply), Network::Regtest, "x") + .await + .unwrap_err(); + assert!( + matches!(&err, AppError::InvalidInput(m) if m.contains("no transfer")), + "{err:?}" + ); +} + +fn reveal(txid: &str, vout: u32) -> NameCoin { + NameCoin { + txid: txid.into(), + vout, + value: 1_000_000, + address: "rs1qexample".into(), + branch: 0, + child_index: 0, + covenant_type: crate::noncustodial::sync::COV_REVEAL as i64, + covenant_json: None, + name_height: Some(9701), + } +} + +/// hsd refuses to REDEEM the name's owner coin (`bad-redeem-owner`): the +/// winning reveal is dropped, a losing one on another outpoint of the same +/// transaction is kept. +#[tokio::test] +async fn redeem_never_spends_the_owner_coin_the_node_reports() { + let node = node_at(20_000, regtest_name_info()); + let kept = exclude_owner_reveal( + &node, + "shkreg2205014174", + vec![reveal(OWNER_TXID, 0), reveal(OWNER_TXID, 1)], + ) + .await + .unwrap(); + assert_eq!( + kept.iter() + .map(|c| (c.txid.as_str(), c.vout)) + .collect::>(), + vec![(OWNER_TXID, 1)] + ); +} + +/// A wallet whose only reveal won has nothing to redeem: refused, rather than +/// a transaction the node rejects. +#[tokio::test] +async fn redeem_of_nothing_but_the_winning_reveal_is_refused() { + let node = node_at(20_000, regtest_name_info()); + let err = exclude_owner_reveal(&node, "shkreg2205014174", vec![reveal(OWNER_TXID, 0)]) + .await + .unwrap_err(); + assert!( + matches!(&err, AppError::InvalidInput(m) if m.contains("won the auction")), + "{err:?}" + ); +} + +/// Without the node's owner there is no telling the winner from a loser. +#[tokio::test] +async fn redeem_guard_without_the_owner_is_an_rpc_error() { + let mut reply = regtest_name_info(); + reply["info"].as_object_mut().unwrap().remove("owner"); + let err = exclude_owner_reveal(&node_at(20_000, reply), "x", vec![reveal(OWNER_TXID, 1)]) + .await + .unwrap_err(); + assert!( + matches!(&err, AppError::Rpc(m) if m.contains("owner")), + "{err:?}" + ); +} + +/// No coins, no question for the node: the caller says there is nothing to +/// redeem. +#[tokio::test] +async fn redeem_guard_with_no_coins_asks_nothing() { + let node = MockNodeRpc::new(); + assert!(exclude_owner_reveal(&node, "x", vec![]) + .await + .unwrap() + .is_empty()); + assert!(node.calls().is_empty()); +} diff --git a/src-tauri/src/tests/live_node_it.rs b/src-tauri/src/tests/live_node_it.rs index 510f50a..b84dda4 100644 --- a/src-tauri/src/tests/live_node_it.rs +++ b/src-tauri/src/tests/live_node_it.rs @@ -439,6 +439,10 @@ async fn live_auction_open_bid_reveal_register() { assert_eq!(node_state(&cl, &name).await.as_deref(), Some("CLOSED")); } +/// A genuine loser redeems: a rival wallet on the same node outbids this one, +/// so the rival's reveal becomes the name's owner and this wallet's reveal is +/// a losing one. REDEEM spends exactly that losing reveal and returns its +/// value; the rival's winning reveal stays the owner. #[tokio::test] async fn live_auction_open_bid_reveal_redeem() { let Some((url, key)) = it_env() else { @@ -449,10 +453,18 @@ async fn live_auction_open_bid_reveal_redeem() { let app = app_with(conn); let cl = client(&url, &key); let (addr, _, _) = leaf00(); + let (rival, rival_addr) = rival_wallet(&url, &key); - // Fund the wallet. + // Fund both wallets. cl.generate_to_address(101, &addr).await.expect("fund"); + // A few coinbases are plenty for one bid (regtest coinbase maturity is 2). + cl.generate_to_address(5, &rival_addr) + .await + .expect("fund rival"); sync_wallet_state(app.state(), None).await.expect("sync"); + sync_wallet_state(rival.state(), None) + .await + .expect("sync rival"); // A per-run-unique name (avoid collisions with names already on the node). let tip = cl.get_blockchain_info().await.expect("info").blocks; @@ -469,43 +481,131 @@ async fn live_auction_open_bid_reveal_redeem() { node_state(&cl, &name).await ); - // BID → advance to REVEAL. + // BID from both wallets; the rival bids higher → advance to REVEAL. sync_wallet_state(app.state(), None).await.expect("sync"); let bid = build_bid_draft(app.state(), name.clone(), 500_000, 1_000_000, Some(1)) .await .expect("build bid"); execute(&app, &cl, &addr, bid.id).await; + sync_wallet_state(rival.state(), None) + .await + .expect("sync rival"); + let rival_bid = build_bid_draft(rival.state(), name.clone(), 800_000, 1_600_000, Some(1)) + .await + .expect("build rival bid"); + execute(&rival, &cl, &addr, rival_bid.id).await; assert!( mine_until(&cl, &name, "REVEAL", &addr, 30).await, "name {name} did not reach REVEAL; state={:?}", node_state(&cl, &name).await ); - // REVEAL → advance to CLOSED. + // REVEAL from both → advance to CLOSED. sync_wallet_state(app.state(), None).await.expect("sync"); let reveal = build_reveal_draft(app.state(), name.clone(), Some(1)) .await .expect("build reveal"); - execute(&app, &cl, &addr, reveal.id).await; + execute(&app, &cl, &addr, reveal.id.clone()).await; + sync_wallet_state(rival.state(), None) + .await + .expect("sync rival"); + let rival_reveal = build_reveal_draft(rival.state(), name.clone(), Some(1)) + .await + .expect("build rival reveal"); + execute(&rival, &cl, &addr, rival_reveal.id.clone()).await; assert!( mine_until(&cl, &name, "CLOSED", &addr, 40).await, "name {name} did not reach CLOSED; state={:?}", node_state(&cl, &name).await ); - // Do NOT insert tracked_name_states for this name. The sync therefore - // won't pick up the owner coin → wallet sees no owner → can redeem. - sync_wallet_state(app.state(), None).await.expect("sync"); + // The node names the rival's reveal as the owner: ours lost. + let rival_reveal_txid = draft_status(&rival, &rival_reveal.id) + .txid + .expect("rival reveal txid"); + assert_eq!( + name_owner(&cl, &name).await.0, + rival_reveal_txid, + "the higher bid must own the name" + ); + let ours = covenant_outpoints( + &cl, + &draft_status(&app, &reveal.id).txid.expect("reveal txid"), + COV_TYPE_REVEAL, + ) + .await; + assert_eq!(ours.len(), 1, "one reveal output: {ours:?}"); - // REDEEM: there is an unspent reveal coin at the bid commitment address. - // The wallet can reclaim it without needing the name's owner coin. + // REDEEM the losing reveal. + sync_wallet_state(app.state(), None).await.expect("sync"); let redeem = build_redeem_draft(app.state(), name.clone(), Some(1)) .await .expect("build redeem"); - execute(&app, &cl, &addr, redeem.id).await; + execute(&app, &cl, &addr, redeem.id.clone()).await; - // After redeem, the name stays CLOSED on-chain. - assert_eq!(node_state(&cl, &name).await.as_deref(), Some("CLOSED")); + // The redeem spent exactly our losing reveal, which is gone from the UTXO + // set, and paid its value (the true bid) back; the owner is unchanged. + let redeem_txid = draft_status(&app, &redeem.id).txid.expect("redeem txid"); + assert_spends_reveals(&cl, &redeem_txid, &ours).await; + assert_eq!( + output_value_by_covenant_type(&cl, &redeem_txid, COV_TYPE_REDEEM).await, + Some(500_000), + "REDEEM returns the losing reveal's value" + ); + assert_eq!(name_owner(&cl, &name).await.0, rival_reveal_txid); +} + +/// The guard behind REDEEM on a real node: a lone bidder's reveal won, and +/// until REGISTER spends it it is the name's owner coin, which hsd refuses to +/// redeem (`bad-redeem-owner`). The name is never tracked, so the wallet has +/// no record of the owner of its own — only the node's answer can refuse it. +#[tokio::test] +async fn live_redeem_of_the_winning_reveal_is_refused() { + let Some((url, key)) = it_env() else { + eprintln!("skip live_redeem_of_the_winning_reveal_is_refused: set HNS_IT_NODE_URL"); + return; + }; + let conn = seeded_conn_regtest(&url, &key); + let app = app_with(conn); + let cl = client(&url, &key); + let (addr, _, _) = leaf00(); + cl.generate_to_address(101, &addr).await.expect("fund"); + sync_wallet_state(app.state(), None).await.expect("sync"); + let tip = cl.get_blockchain_info().await.expect("info").blocks; + let name = format!("wonrv{tip}"); + + let open = build_open_draft(app.state(), name.clone(), Some(1)) + .await + .expect("build open"); + execute(&app, &cl, &addr, open.id).await; + assert!(mine_until(&cl, &name, "BIDDING", &addr, 30).await); + sync_wallet_state(app.state(), None).await.expect("sync"); + let bid = build_bid_draft(app.state(), name.clone(), 500_000, 1_000_000, Some(1)) + .await + .expect("build bid"); + execute(&app, &cl, &addr, bid.id).await; + assert!(mine_until(&cl, &name, "REVEAL", &addr, 30).await); + sync_wallet_state(app.state(), None).await.expect("sync"); + let reveal = build_reveal_draft(app.state(), name.clone(), Some(1)) + .await + .expect("build reveal"); + execute(&app, &cl, &addr, reveal.id.clone()).await; + assert!(mine_until(&cl, &name, "CLOSED", &addr, 40).await); + sync_wallet_state(app.state(), None).await.expect("sync"); + + let reveal_txid = draft_status(&app, &reveal.id).txid.expect("reveal txid"); + assert_eq!( + name_owner(&cl, &name).await.0, + reveal_txid, + "the lone reveal owns the name" + ); + expect_err( + build_redeem_draft(app.state(), name.clone(), Some(1)).await, + "won the auction", + ); + // Nothing spent it: it is still the owner coin, waiting for REGISTER. + let (hash, index) = name_owner(&cl, &name).await; + assert!(cl.get_coin(&hash, index).await.expect("coin").is_some()); } #[tokio::test] @@ -582,14 +682,37 @@ async fn live_auction_register_transfer_finalize() { execute(&app, &cl, &addr, transfer.id).await; sync_wallet_state(app.state(), None).await.expect("sync"); - // After TRANSFER, the name stays on-chain in some state still allowing finalize. + // hsd refuses a FINALIZE until the transfer lockup is over + // (`bad-finalize-maturity`), so the builder refuses it too. use crate::commands::names::build_finalize_draft; + expect_err( + build_finalize_draft(app.state(), name.clone(), Some(1)).await, + "still locked", + ); + + // Mine exactly the rest of the lockup, then finalize. + let transfer_height = name_info_height(&cl, &name, "transfer").await; + let tip = cl.get_blockchain_info().await.expect("info").blocks; + let left = NET + .name_params() + .blocks_until_finalize(transfer_height, tip); + assert!(left > 0, "the lockup was not over a moment ago"); + cl.generate_to_address(left as u32, &addr) + .await + .expect("lockup"); + sync_wallet_state(app.state(), None).await.expect("sync"); let finalize = build_finalize_draft(app.state(), name.clone(), Some(1)) .await .expect("build finalize"); execute(&app, &cl, &addr, finalize.id).await; - // After finalize, the name is at the new address. Check state. + // The name's owner coin is now at the recipient, and no transfer is + // pending. + assert_eq!( + owner_coin_address(&cl, &name).await.as_deref(), + Some(addr2.as_str()) + ); + assert_eq!(name_info_height(&cl, &name, "transfer").await, 0); assert_eq!(node_state(&cl, &name).await.as_deref(), Some("CLOSED")); } @@ -715,8 +838,8 @@ async fn live_batch_transfer_two_names() { ); } - // Finalize each name after the transfer lockup elapses. FINALIZE is per-name - // (there is no batch finalize command); advance past the regtest transfer + // Finalize each name on its own after the transfer lockup elapses (the + // batch finalize has its own test); advance past the regtest transfer // lockup (10 blocks) so each finalize is valid. sync_wallet_state(app.state(), None).await.expect("sync"); cl.generate_to_address(11, &addr).await.expect("lockup"); @@ -727,10 +850,20 @@ async fn live_batch_transfer_two_names() { .await .expect("build finalize"); execute(&app, &cl, &addr, finalize.id).await; + // The first finalize spent a funding coin for its fee. Without a sync + // the wallet still holds that coin as unspent, and the second finalize + // spent it again: hsd took that transaction as an orphan, its input + // missing. In the app a sync runs between any two actions. + sync_wallet_state(app.state(), None).await.expect("sync"); } // After finalize, both names are at the recipient address and CLOSED. for name in [&name_a, &name_b] { + assert_eq!( + owner_coin_address(&cl, name).await.as_deref(), + Some(recipient.as_str()), + "name {name} did not move to the recipient" + ); assert_eq!( node_state(&cl, name).await.as_deref(), Some("CLOSED"), @@ -911,6 +1044,102 @@ fn expect_err(res: Result, needle } } +/// hsd covenant type codes (`lib/covenants/rules.js` `types`). +const COV_TYPE_REVEAL: u64 = 4; +const COV_TYPE_REDEEM: u64 = 5; + +/// A second wallet on the same node: its own in-memory database and a BIP44 +/// account nothing else uses, so it bids against this test's wallet as a +/// stranger would. Returns the app and its funding address. +fn rival_wallet(url: &str, key: &str) -> (tauri::App, String) { + let acct = unique_acct(); + let app = app_with(seeded_conn_acct(url, key, acct)); + (app, leaf00_at(acct).0) +} + +/// A height field of the node's `getnameinfo` → `info` (`renewal`, +/// `transfer`). +async fn name_info_height(cl: &NodeRpcClient, name: &str, key: &str) -> i64 { + let info = cl.get_name_info(name).await.expect("name info"); + info["info"][key] + .as_i64() + .unwrap_or_else(|| panic!("no {key} for {name}: {info}")) +} + +/// Mine exactly as many blocks as the latest-renewed of `names` needs before +/// hsd accepts a RENEW of it (`renewal + tree_interval`, judged at `tip + 1`). +async fn mine_until_renewable(cl: &NodeRpcClient, names: &[&str], addr: &str) { + let tip = cl.get_blockchain_info().await.expect("info").blocks; + let mut left = 0; + for name in names { + let renewal = name_info_height(cl, name, "renewal").await; + left = left.max(NET.name_params().blocks_until_renew(renewal, tip)); + } + if left > 0 { + cl.generate_to_address(left as u32, addr) + .await + .expect("mine to renewable"); + } +} + +/// The name's owner outpoint as the node reports it (`getnameinfo` → +/// `info.owner`). +async fn name_owner(cl: &NodeRpcClient, name: &str) -> (String, u32) { + let info = cl.get_name_info(name).await.expect("name info"); + let owner = &info["info"]["owner"]; + ( + owner["hash"].as_str().expect("owner hash").to_string(), + owner["index"].as_u64().expect("owner index") as u32, + ) +} + +/// The outpoints of `txid`'s outputs carrying covenant `want_type`. +async fn covenant_outpoints(cl: &NodeRpcClient, txid: &str, want_type: u64) -> Vec<(String, u32)> { + let tx = cl.get_tx_by_hash(txid).await.expect("tx lookup"); + tx["outputs"] + .as_array() + .expect("outputs") + .iter() + .enumerate() + .filter(|(_, o)| o["covenant"]["type"].as_u64() == Some(want_type)) + .map(|(i, _)| (txid.to_string(), i as u32)) + .collect() +} + +/// The mined transaction `txid` spends every outpoint in `reveals`, and the +/// node no longer has any of them as a coin. +async fn assert_spends_reveals(cl: &NodeRpcClient, txid: &str, reveals: &[(String, u32)]) { + let tx = cl.get_tx_by_hash(txid).await.expect("tx lookup"); + assert!( + tx["height"].as_i64().unwrap_or(-1) >= 0, + "{txid} is not mined" + ); + let spent: Vec<(String, u32)> = tx["inputs"] + .as_array() + .expect("inputs") + .iter() + .map(|i| { + ( + i["prevout"]["hash"] + .as_str() + .expect("prevout hash") + .to_string(), + i["prevout"]["index"].as_u64().expect("prevout index") as u32, + ) + }) + .collect(); + for (hash, index) in reveals { + assert!( + spent.contains(&(hash.clone(), *index)), + "{txid} does not spend {hash}:{index}; inputs {spent:?}" + ); + assert!( + cl.get_coin(hash, *index).await.expect("coin").is_none(), + "{hash}:{index} is still unspent" + ); + } +} + /// Acquire two names end-to-end so the wallet owns both. Convenience wrapper /// around `acquire_name` used by the E-series batch tests. async fn acquire_two_names( @@ -1974,6 +2203,11 @@ async fn live_update_records() { /// D2. Regression guard for commit 78bba67: RENEW references the renewal /// block decoded in hsd internal (UNREVERSED) byte order, so the node accepts /// it (no bad-register-renewal). The name stays owned and its value is kept. +/// +/// A RENEW right after REGISTER is refused by hsd (`bad-renewal-premature`: +/// not before `renewal + tree_interval`), so the builder refuses it first; +/// once that many blocks are mined the renewal lands and moves `renewal` to +/// the block it was mined in. #[tokio::test] async fn live_renew_extends_lease() { let Some((url, key)) = it_env() else { @@ -1996,15 +2230,29 @@ async fn live_renew_extends_lease() { .and_then(|i| i.get("value")) .and_then(|v| v.as_u64()); + // Just registered: too early to renew. + expect_err( + crate::commands::names::build_renew_draft(app.state(), name.clone(), Some(1)).await, + "renewed too recently", + ); + let renewal_before = name_info_height(&cl, &name, "renewal").await; + mine_until_renewable(&cl, &[&name], &addr).await; + // build_renew_draft calls renewal_block() -> getblockhash decoded // UNREVERSED. If the byte order regressed, the node would reject the // covenant with bad-register-renewal on broadcast; execute() asserts the - // broadcast succeeds. + // node took it. let renew = crate::commands::names::build_renew_draft(app.state(), name.clone(), Some(1)) .await .expect("build renew"); execute(&app, &cl, &addr, renew.id).await; + // The renewal is on chain: `renewal` is now the block it was mined in. + let mined_at = cl.get_blockchain_info().await.expect("info").blocks; + let renewal_after = name_info_height(&cl, &name, "renewal").await; + assert!(renewal_after > renewal_before, "renewal did not advance"); + assert_eq!(renewal_after, mined_at, "renewed in the block just mined"); + assert_eq!(node_state(&cl, &name).await.as_deref(), Some("CLOSED")); let after = cl.get_name_info(&name).await.expect("info"); let value_after = after @@ -2252,8 +2500,10 @@ async fn live_batch_reveal_two_names() { } } -/// E3. A losing batch bid can be reclaimed with build_batch_redeem_draft after -/// the auction closes; both lockups are returned in one tx. +/// E3. Losing batch bids are reclaimed with build_batch_redeem_draft after the +/// auctions close: a rival wallet on the same node outbids this one on both +/// names, and one REDEEM transaction spends both of this wallet's losing +/// reveals while the rival's reveals stay the owners. #[tokio::test] async fn live_batch_redeem_two_names() { let Some((url, key)) = it_env() else { @@ -2264,17 +2514,20 @@ async fn live_batch_redeem_two_names() { let app = app_with(conn); let cl = client(&url, &key); let (addr, _, _) = leaf00(); + let (rival, rival_addr) = rival_wallet(&url, &key); fund(&cl, &addr, 101).await; + fund(&cl, &rival_addr, 5).await; sync_wallet_state(app.state(), None).await.expect("sync"); let tip = cl.get_blockchain_info().await.expect("info").blocks; let name_a = format!("brdm{tip}a"); let name_b = format!("brdm{tip}b"); + let names = vec![name_a.clone(), name_b.clone()]; open_two_to_bidding(&app, &cl, &addr, &name_a, &name_b).await; sync_wallet_state(app.state(), None).await.expect("sync"); let bid = crate::commands::names::build_batch_bid_draft( app.state(), - vec![name_a.clone(), name_b.clone()], + names.clone(), 500_000, 1_000_000, Some(1), @@ -2282,6 +2535,19 @@ async fn live_batch_redeem_two_names() { .await .expect("build batch bid"); execute(&app, &cl, &addr, bid.id).await; + sync_wallet_state(rival.state(), None) + .await + .expect("sync rival"); + let rival_bid = crate::commands::names::build_batch_bid_draft( + rival.state(), + names.clone(), + 800_000, + 1_600_000, + Some(1), + ) + .await + .expect("build rival batch bid"); + execute(&rival, &cl, &addr, rival_bid.id).await; for name in [&name_a, &name_b] { assert!( mine_until(&cl, name, "REVEAL", &addr, 30).await, @@ -2290,14 +2556,19 @@ async fn live_batch_redeem_two_names() { } sync_wallet_state(app.state(), None).await.expect("sync"); - let reveal = crate::commands::names::build_batch_reveal_draft( - app.state(), - vec![name_a.clone(), name_b.clone()], - Some(1), - ) - .await - .expect("build batch reveal"); - execute(&app, &cl, &addr, reveal.id).await; + let reveal = + crate::commands::names::build_batch_reveal_draft(app.state(), names.clone(), Some(1)) + .await + .expect("build batch reveal"); + execute(&app, &cl, &addr, reveal.id.clone()).await; + sync_wallet_state(rival.state(), None) + .await + .expect("sync rival"); + let rival_reveal = + crate::commands::names::build_batch_reveal_draft(rival.state(), names.clone(), Some(1)) + .await + .expect("build rival batch reveal"); + execute(&rival, &cl, &addr, rival_reveal.id.clone()).await; for name in [&name_a, &name_b] { assert!( mine_until(&cl, name, "CLOSED", &addr, 40).await, @@ -2305,18 +2576,40 @@ async fn live_batch_redeem_two_names() { ); } - // Do NOT track the names → sync won't record an owner coin → the wallet - // sees only its unspent reveal coins and can redeem both. - sync_wallet_state(app.state(), None).await.expect("sync"); - let redeem = crate::commands::names::build_batch_redeem_draft( - app.state(), - vec![name_a.clone(), name_b.clone()], - Some(1), + // The rival's reveals own both names; both of ours lost. + let rival_reveal_txid = draft_status(&rival, &rival_reveal.id) + .txid + .expect("rival reveal txid"); + for name in [&name_a, &name_b] { + assert_eq!(name_owner(&cl, name).await.0, rival_reveal_txid, "{name}"); + } + let ours = covenant_outpoints( + &cl, + &draft_status(&app, &reveal.id).txid.expect("reveal txid"), + COV_TYPE_REVEAL, ) - .await - .expect("build batch redeem"); - execute(&app, &cl, &addr, redeem.id).await; + .await; + assert_eq!(ours.len(), 2, "one reveal output per name: {ours:?}"); + + sync_wallet_state(app.state(), None).await.expect("sync"); + let redeem = + crate::commands::names::build_batch_redeem_draft(app.state(), names.clone(), Some(1)) + .await + .expect("build batch redeem"); + execute(&app, &cl, &addr, redeem.id.clone()).await; + + // One transaction spent both losing reveals; the owners are unchanged. + let redeem_txid = draft_status(&app, &redeem.id).txid.expect("redeem txid"); + assert_spends_reveals(&cl, &redeem_txid, &ours).await; + assert_eq!( + covenant_outpoints(&cl, &redeem_txid, COV_TYPE_REDEEM) + .await + .len(), + 2, + "one REDEEM output per name" + ); for name in [&name_a, &name_b] { + assert_eq!(name_owner(&cl, name).await.0, rival_reveal_txid, "{name}"); assert_eq!(node_state(&cl, name).await.as_deref(), Some("CLOSED")); } } @@ -2339,16 +2632,31 @@ async fn live_batch_renew_two_names() { let name_a = format!("brnw{tip}a"); let name_b = format!("brnw{tip}b"); acquire_two_names(&app, &cl, &addr, &name_a, &name_b).await; + let names = vec![name_a.clone(), name_b.clone()]; + + // `name_b` was registered in the last block: the batch would carry a + // RENEW hsd refuses (`bad-renewal-premature`), so the builder refuses the + // whole batch. + expect_err( + crate::commands::names::build_batch_renew_draft(app.state(), names.clone(), Some(1)).await, + "renewed too recently", + ); + let before = [ + name_info_height(&cl, &name_a, "renewal").await, + name_info_height(&cl, &name_b, "renewal").await, + ]; + mine_until_renewable(&cl, &[&name_a, &name_b], &addr).await; - let renew = crate::commands::names::build_batch_renew_draft( - app.state(), - vec![name_a.clone(), name_b.clone()], - Some(1), - ) - .await - .expect("build batch renew"); + let renew = + crate::commands::names::build_batch_renew_draft(app.state(), names.clone(), Some(1)) + .await + .expect("build batch renew"); execute(&app, &cl, &addr, renew.id).await; - for name in [&name_a, &name_b] { + let mined_at = cl.get_blockchain_info().await.expect("info").blocks; + for (name, before) in names.iter().zip(before) { + let after = name_info_height(&cl, name, "renewal").await; + assert!(after > before, "{name}: renewal did not advance"); + assert_eq!(after, mined_at, "{name} renewed in the block just mined"); assert_eq!(node_state(&cl, name).await.as_deref(), Some("CLOSED")); } } @@ -2439,8 +2747,9 @@ async fn output_value_by_covenant_type( None } -/// F1. Finalize BEFORE transfer_lockup (10 blocks on regtest) is rejected by -/// the node; after the lockup, it is accepted. +/// F1. Finalize BEFORE transfer_lockup (10 blocks on regtest) is refused by +/// the builder, as hsd would refuse it; after the lockup, it is accepted and +/// the name moves to the recipient. #[tokio::test] async fn live_premature_finalize_rejected() { let Some((url, key)) = it_env() else { @@ -2459,59 +2768,48 @@ async fn live_premature_finalize_rejected() { // Transfer to leaf 0/1. let recipient = recv_leaf_01(); - let transfer = - crate::commands::names::build_transfer_draft(app.state(), name.clone(), recipient, Some(1)) - .await - .expect("build transfer"); + let transfer = crate::commands::names::build_transfer_draft( + app.state(), + name.clone(), + recipient.clone(), + Some(1), + ) + .await + .expect("build transfer"); execute(&app, &cl, &addr, transfer.id).await; sync_wallet_state(app.state(), None).await.expect("sync"); // Try to finalize IMMEDIATELY — before the transfer lockup elapses. hsd - // ACCEPTS a premature FINALIZE into the mempool (`sendrawtransaction` - // returns success) but silently DROPS it when assembling the next block: - // the covenant's finalize-height rule is enforced at block-connect, not at - // mempool admission. So the correct invariant is not "broadcast fails" but - // "the finalize never CONFIRMS while the lockup is unmet" — the name's - // TRANSFER stays pending across a mined block. - let fin = crate::commands::names::build_finalize_draft(app.state(), name.clone(), Some(1)) - .await - .expect("build finalize"); - unlock(&app); - sign_tx_draft_inner(&app.state(), &fin.id) - .await - .expect("sign"); - // Broadcast may succeed (mempool) or fail (older hsd) — either is fine. - let _ = broadcast_tx_draft(app.state(), fin.id.clone()).await; - let fin_txid = draft_status(&app, &fin.id).txid; - - // Mine a block. A valid finalize would confirm here; the premature one is - // dropped from the block template. - cl.generate_to_address(1, &addr).await.expect("mine"); - // The finalize tx must NOT be on-chain, and the name's transfer must still - // be pending (not yet finalized). - if let Some(txid) = fin_txid { - let on_chain = cl.get_tx_by_hash(&txid).await.ok(); - assert!( - matches!(on_chain, None | Some(serde_json::Value::Null)), - "premature finalize must NOT confirm before the transfer lockup" - ); - } - let info = cl.get_name_info(&name).await.expect("nameinfo"); - let still_pending = info - .get("info") - .and_then(|i| i.get("transfer")) - .map(|t| !t.is_null()) - .unwrap_or(false); - assert!( - still_pending, - "name's TRANSFER must still be pending after a premature finalize attempt: {info}" + // refuses that FINALIZE (`bad-finalize-maturity`), but its + // `sendrawtransaction` hands back the txid all the same, so the wallet + // cannot leave the refusal to the broadcast: the builder refuses it, says + // how long the lockup still runs, and persists nothing. + let drafts_before = { + let state = app.state::(); + let c = state.db.lock().unwrap(); + c.query_row("SELECT COUNT(*) FROM wallet_tx_drafts", [], |r| { + r.get::<_, i64>(0) + }) + .unwrap() + }; + expect_err( + crate::commands::names::build_finalize_draft(app.state(), name.clone(), Some(1)).await, + "still locked for 9 more blocks", ); - - // Release the draft's reservation so the next finalize can pick up the - // owner coin. - crate::commands::tx::release_tx_draft_reservation(app.state(), fin.id) - .await - .ok(); + let drafts_after = { + let state = app.state::(); + let c = state.db.lock().unwrap(); + c.query_row("SELECT COUNT(*) FROM wallet_tx_drafts", [], |r| { + r.get::<_, i64>(0) + }) + .unwrap() + }; + assert_eq!( + drafts_before, drafts_after, + "a refused finalize persists nothing" + ); + let transfer_h = name_info_height(&cl, &name, "transfer").await; + assert!(transfer_h > 0, "the transfer is still pending"); // Advance past the transfer lockup (10 blocks), sync, and retry — accepted. cl.generate_to_address(11, &addr).await.expect("lockup"); @@ -2520,6 +2818,10 @@ async fn live_premature_finalize_rejected() { .await .expect("build finalize 2"); execute(&app, &cl, &addr, fin2.id).await; + assert_eq!( + owner_coin_address(&cl, &name).await.as_deref(), + Some(recipient.as_str()) + ); assert_eq!(node_state(&cl, &name).await.as_deref(), Some("CLOSED")); } @@ -3439,6 +3741,15 @@ async fn live_batch_large_covenant_count() { names.push(name); } + // The last name was registered in the last block; hsd accepts a RENEW + // only a tree interval after that (`bad-renewal-premature`). + let name_refs: Vec<&str> = names.iter().map(|n| n.as_str()).collect(); + mine_until_renewable(&cl, &name_refs, &addr).await; + let mut before = Vec::new(); + for name in &names { + before.push(name_info_height(&cl, name, "renewal").await); + } + // Batch renew all 20 names in one tx. let batch = crate::commands::names::build_batch_renew_draft(app.state(), names.clone(), Some(1)) @@ -3448,14 +3759,19 @@ async fn live_batch_large_covenant_count() { // Broadcast the batch. execute(&app, &cl, &addr, batch.id.clone()).await; - // Verify the batch was accepted: draft status shows broadcasted/confirmed. + // The node took it and mined it: every name's renewal moved to that block. let row = draft_status(&app, &batch.id); assert!( matches!(row.status.as_str(), "broadcasted" | "confirmed"), - "large batch (20 names) must broadcast successfully, got status: {}", + "large batch (20 names) must be sent, got status: {}", row.status ); - eprintln!("✓ Large batch (20 covenants) assembled and broadcast successfully"); + let mined_at = cl.get_blockchain_info().await.expect("info").blocks; + for (name, before) in names.iter().zip(before) { + let after = name_info_height(&cl, name, "renewal").await; + assert!(after > before, "{name}: renewal did not advance"); + assert_eq!(after, mined_at, "{name} renewed in the block just mined"); + } } /// End-to-end proof for the chain scanner against a real node: OPEN a name, @@ -4115,10 +4431,17 @@ async fn live_multi_bid_lifecycle_leaves_no_coin_stranded() { for (what, cap) in [ ("update", &caps.can_update), ("transfer", &caps.can_transfer), - ("renew", &caps.can_renew), ] { assert!(cap.allowed, "{what} must be offered: {:?}", cap.reason); } + // Renew is held back for the plain reason hsd gives, not a transfer: a + // name registered in the last block may be renewed only a tree interval + // later (`bad-renewal-premature`). + let renew_reason = caps.can_renew.reason.clone().unwrap_or_default(); + assert!( + !caps.can_renew.allowed && renew_reason.contains("renewed too recently"), + "renew must wait out the tree interval: {renew_reason:?}" + ); assert!(!caps.can_finalize.allowed, "nothing to finalize"); // The guided panel is what tells a user what to do next. Registered, with // its own losing lockups still out there, the wallet must point at them — diff --git a/src-tauri/src/tests/name_capabilities_tests.rs b/src-tauri/src/tests/name_capabilities_tests.rs index cb46576..a8a9742 100644 --- a/src-tauri/src/tests/name_capabilities_tests.rs +++ b/src-tauri/src/tests/name_capabilities_tests.rs @@ -50,6 +50,7 @@ fn ctx( owner_spend_in_flight: false, transfer_height: None, current_height: None, + renewal_height: None, reveal_txid, reveal_draft_status, bid_value_doos, diff --git a/src-tauri/src/tests/names_cmd_tests.rs b/src-tauri/src/tests/names_cmd_tests.rs index 93a9c3f..84d5409 100644 --- a/src-tauri/src/tests/names_cmd_tests.rs +++ b/src-tauri/src/tests/names_cmd_tests.rs @@ -1096,11 +1096,53 @@ fn seed_bid_commitment(conn: &rusqlite::Connection, profile_id: &str, name: &str /// `build_reveal_draft` need: getnameinfo, getblockchaininfo, getblockhash. pub(crate) async fn mock_names_rpc( server: &mut mockito::Server, +) -> (mockito::Mock, mockito::Mock, mockito::Mock) { + mock_names_rpc_with_info(server, names_rpc_info()).await +} + +/// The `getnameinfo` `info` [`mock_names_rpc`] answers with, in hsd's +/// `NameState.getJSON` shape (`lib/covenants/namestate.js`): a registered name +/// last renewed at 120 and transferred at 900, both far enough behind the +/// mocked tip (1000) for a RENEW or FINALIZE to be accepted, whose owner coin +/// is none a test seeds — so a reveal coin a test seeds is a losing one. +pub(crate) fn names_rpc_info() -> serde_json::Value { + serde_json::json!({ + "name": "alpha", + "nameHash": "00".repeat(32), + "state": "CLOSED", + "height": 100, + "renewal": 120, + "owner": {"hash": "0f".repeat(32), "index": 0}, + "value": 50000, + "highest": 60000, + "data": "", + "transfer": 900, + "revoked": 0, + "claimed": 1, + "renewals": 2, + "registered": true, + "expired": false, + "weak": false, + "stats": null + }) +} + +/// [`mock_names_rpc`] answering `getnameinfo` with `info`. +pub(crate) async fn mock_names_rpc_with_info( + server: &mut mockito::Server, + info: serde_json::Value, ) -> (mockito::Mock, mockito::Mock, mockito::Mock) { let name_info = server .mock("POST", "/") .match_body(mockito::Matcher::Regex("getnameinfo".into())) - .with_body(r#"{"result":{"info":{"height":100,"value":50000,"renewals":2,"claimed":1,"weak":false}},"error":null,"id":1}"#) + .with_body( + serde_json::json!({ + "result": {"start": {"reserved": false, "week": 17, "start": 34}, "info": info}, + "error": null, + "id": 1 + }) + .to_string(), + ) .expect_at_least(1) .create_async() .await; @@ -3868,3 +3910,193 @@ async fn capabilities_uses_per_profile_node_override() { ); o_name.assert_async().await; } + +// --- consensus guards: the wrappers refuse what hsd would reject ------------ +// +// hsd's `sendrawtransaction` returns the txid even when its mempool refuses +// the transaction, so a RENEW, FINALIZE or REDEEM the chain rejects has to be +// refused while it is built. Each test answers `getnameinfo` with a name the +// node would refuse the action for, at the mocked tip of 1000. + +fn drafts_of(app: &tauri::App) -> i64 { + let state = app.state::(); + let conn = state.db.lock().unwrap(); + conn.query_row("SELECT COUNT(*) FROM wallet_tx_drafts", [], |r| r.get(0)) + .unwrap() +} + +fn with_info(changes: serde_json::Value) -> serde_json::Value { + let mut info = names_rpc_info(); + for (k, v) in changes.as_object().unwrap() { + info[k] = v.clone(); + } + info +} + +/// Renewed at 997: hsd accepts a RENEW from block 1002 (`renewal + +/// treeInterval`, regtest 5), and a draft built on tip 1000 is judged at 1001. +#[tokio::test] +async fn renew_wrappers_refuse_a_premature_renewal() { + let mut server = mockito::Server::new_async().await; + let _mocks = + mock_names_rpc_with_info(&mut server, with_info(serde_json::json!({"renewal": 997}))).await; + let state = create_full_test_state(); + { + let conn = state.db.lock().unwrap(); + let id = insert_valid_profile(&conn, "regtest"); + set_node_rpc_url(&conn, &server.url()); + insert_extra_funding(&conn, &id, &"23".repeat(32)); + seed_owner_coin_at_derived(&conn, &id, "alpha", &"a1".repeat(32), 0, None); + seed_owner_coin_at_derived(&conn, &id, "bravo", &"b1".repeat(32), 0, None); + } + let app = mock_app_with(state); + let err = names::build_renew_draft(app.state(), "alpha".into(), None) + .await + .expect_err("a premature RENEW must be refused"); + assert!( + format!("{err}").contains("can be renewed in 1 block"), + "{err}" + ); + let err = + names::build_batch_renew_draft(app.state(), vec!["alpha".into(), "bravo".into()], None) + .await + .expect_err("a premature batch RENEW must be refused"); + assert!(format!("{err}").contains("renewed too recently"), "{err}"); + assert_eq!(drafts_of(&app), 0, "nothing is persisted"); +} + +/// Transferred at 995: hsd accepts a FINALIZE from block 1005 (`transfer + +/// transferLockup`, regtest 10); a draft built on tip 1000 is judged at 1001. +#[tokio::test] +async fn finalize_wrappers_refuse_a_finalize_inside_the_transfer_lockup() { + let mut server = mockito::Server::new_async().await; + let _mocks = + mock_names_rpc_with_info(&mut server, with_info(serde_json::json!({"transfer": 995}))) + .await; + let state = create_full_test_state(); + let payment_addr = { + let conn = state.db.lock().unwrap(); + let id = insert_valid_profile(&conn, "regtest"); + set_node_rpc_url(&conn, &server.url()); + insert_extra_funding(&conn, &id, &"56".repeat(32)); + for (name, txid) in [("alpha", "a4".repeat(32)), ("bravo", "b4".repeat(32))] { + let cov = transfer_covenant_json_for(name); + seed_owner_coin_at_derived( + &conn, + &id, + name, + &txid, + crate::noncustodial::sync::COV_TRANSFER as i64, + Some(&cov), + ); + } + first_derived_address(&conn, &id) + }; + let app = mock_app_with(state); + let err = names::build_finalize_draft(app.state(), "alpha".into(), None) + .await + .expect_err("a FINALIZE inside the lockup must be refused"); + assert!( + format!("{err}").contains("still locked for 4 more blocks"), + "{err}" + ); + let err = + names::build_batch_finalize_draft(app.state(), vec!["alpha".into(), "bravo".into()], None) + .await + .expect_err("a batch FINALIZE inside the lockup must be refused"); + assert!(format!("{err}").contains("still locked"), "{err}"); + let err = names::build_finalize_with_payment_draft( + app.state(), + "alpha".into(), + payment_addr, + 3_000_000, + None, + ) + .await + .expect_err("a paid FINALIZE inside the lockup must be refused"); + assert!(format!("{err}").contains("still locked"), "{err}"); + assert_eq!(drafts_of(&app), 0, "nothing is persisted"); +} + +/// The node names the wallet's only reveal as the owner: it won, and hsd +/// refuses to redeem it (`bad-redeem-owner`). The wallet has no record of the +/// owner (the name was never tracked), so only the node can tell. +#[tokio::test] +async fn redeem_wrappers_refuse_to_redeem_the_winning_reveal() { + let mut server = mockito::Server::new_async().await; + let _mocks = mock_names_rpc_with_info( + &mut server, + with_info(serde_json::json!({"owner": {"hash": "ac".repeat(32), "index": 0}})), + ) + .await; + let state = create_full_test_state(); + { + let conn = state.db.lock().unwrap(); + let id = insert_valid_profile(&conn, "regtest"); + set_node_rpc_url(&conn, &server.url()); + insert_extra_funding(&conn, &id, &"78".repeat(32)); + let addr = first_derived_address(&conn, &id); + let cov = covenant_json_for("alpha", crate::noncustodial::sync::COV_REVEAL, "REVEAL"); + seed_covenant_coin( + &conn, + &id, + &"ac".repeat(32), + &addr, + crate::noncustodial::sync::COV_REVEAL, + 1000, + Some(&cov), + ); + seed_bid_commitment(&conn, &id, "alpha", &addr); + } + let app = mock_app_with(state); + let err = names::build_redeem_draft(app.state(), "alpha".into(), None) + .await + .expect_err("redeeming the owner coin must be refused"); + assert!(format!("{err}").contains("won the auction"), "{err}"); + let err = names::build_batch_redeem_draft(app.state(), vec!["alpha".into()], None) + .await + .expect_err("batch-redeeming the owner coin must be refused"); + assert!(format!("{err}").contains("won the auction"), "{err}"); + assert_eq!(drafts_of(&app), 0, "nothing is persisted"); +} + +/// A wallet that outbid itself holds the winning reveal and a losing one: the +/// batch redeem spends the loser and leaves the owner coin alone. +#[tokio::test] +async fn batch_redeem_spends_every_losing_reveal_but_not_the_owner() { + let mut server = mockito::Server::new_async().await; + let _mocks = mock_names_rpc_with_info( + &mut server, + with_info(serde_json::json!({"owner": {"hash": "ad".repeat(32), "index": 0}})), + ) + .await; + let state = create_full_test_state(); + { + let conn = state.db.lock().unwrap(); + let id = insert_valid_profile(&conn, "regtest"); + set_node_rpc_url(&conn, &server.url()); + insert_extra_funding(&conn, &id, &"79".repeat(32)); + let addr = first_derived_address(&conn, &id); + let cov = covenant_json_for("alpha", crate::noncustodial::sync::COV_REVEAL, "REVEAL"); + for txid in ["ad".repeat(32), "ae".repeat(32), "af".repeat(32)] { + seed_covenant_coin( + &conn, + &id, + &txid, + &addr, + crate::noncustodial::sync::COV_REVEAL, + 1000, + Some(&cov), + ); + } + seed_bid_commitment(&conn, &id, "alpha", &addr); + } + let app = mock_app_with(state); + let draft = names::build_batch_redeem_draft(app.state(), vec!["alpha".into()], None) + .await + .expect("the losing reveals are redeemable"); + let inputs = draft_signing_inputs(&app, &draft.id); + assert!(!inputs.contains(&"ad".repeat(32)), "owner spent: {inputs}"); + assert!(inputs.contains(&"ae".repeat(32)), "loser left: {inputs}"); + assert!(inputs.contains(&"af".repeat(32)), "loser left: {inputs}"); +} diff --git a/src-tauri/src/tests/network_tests.rs b/src-tauri/src/tests/network_tests.rs index f2d0c0c..1ef0f86 100644 --- a/src-tauri/src/tests/network_tests.rs +++ b/src-tauri/src/tests/network_tests.rs @@ -235,6 +235,19 @@ fn finalize_is_accepted_from_the_block_where_the_lockup_ends() { assert_eq!(p.blocks_until_finalize(100, 500), 0); } +// ── blocks_until_renew ─────────────────────────────────────────────── + +#[test] +fn renew_is_accepted_from_the_block_a_tree_interval_after_the_last_renewal() { + // Regtest tree interval is 5: a name renewed (or registered) at 100 may + // be renewed again in block 105, i.e. built while the tip is 104. + let p = Network::Regtest.name_params(); + assert_eq!(p.blocks_until_renew(100, 100), 4); + assert_eq!(p.blocks_until_renew(100, 103), 1); + assert_eq!(p.blocks_until_renew(100, 104), 0); + assert_eq!(p.blocks_until_renew(100, 500), 0); +} + // ── roundtrip as_str / from_str_opt ────────────────────────────────── #[test]