From 2bb9fe812cf51bd7ac905edb0cec89d0ea7301c4 Mon Sep 17 00:00:00 2001 From: bitcoinafterlife Date: Wed, 7 Oct 2026 11:20:04 +0200 Subject: [PATCH] i18n review fixes (#63) and invalidation/Check fixes (#64-#67) #63: Truman's i18n review (release gate runs i18n/CLI tests, Babel check, ruff known-first-party, no translated text stored in the wallet). #64: invalidation pays the current network fee (2-block estimate). #65: Check KeyError on a stale father link. #66: invalidation cascade with list-shaped children. #67: deselected will-executor (delete allowed if never received; broadcast skips it). Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 5 +- CHANGELOG.md | 233 +++++++++++++++++++++ bal/cli/controller.py | 10 +- bal/core/will.py | 155 +++++++++++++- bal/core/willexecutors.py | 38 +++- bal/gui/qt/lists.py | 24 ++- bal/gui/qt/window.py | 14 +- make-release.sh | 10 +- pyproject.toml | 6 + tests/test_core_deselected_executor.py | 82 ++++++++ tests/test_core_invalidation_fee.py | 231 ++++++++++++++++++++ tests/test_core_status_language_neutral.py | 136 ++++++++++++ tests/test_core_willtree_father.py | 145 +++++++++++++ tests/test_gui_will_menu.py | 25 +++ 14 files changed, 1089 insertions(+), 25 deletions(-) create mode 100644 tests/test_core_deselected_executor.py create mode 100644 tests/test_core_invalidation_fee.py create mode 100644 tests/test_core_status_language_neutral.py create mode 100644 tests/test_core_willtree_father.py diff --git a/AGENTS.md b/AGENTS.md index f4b0a93..a05d3aa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,7 +117,10 @@ python3 build_zip.py # -> bal-electrum-plugin.zip (deterministic, prints sha25 ``` - `build_zip.py` needs **Babel** (`pip install babel`): it compiles the - translation catalogs into the zip and stops if Babel is missing. + translation catalogs into the zip and stops if Babel is missing. The i18n + tests need it too. Install it in the **runtime** env used for releases + (`source "$BAL_HOME/electrum/env/bin/activate" && pip install babel`); + `make-release.sh` checks it before running anything. - `make-release.sh` requires gpg and Gitea credentials (`~/.git-credentials` or `GITEA_USER`/`GITEA_TOKEN`). It bumps `bal/manifest.json` — bump the version there, never invent a new source of truth. diff --git a/CHANGELOG.md b/CHANGELOG.md index 6215885..7b6c673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3549,3 +3549,236 @@ delivery; when it is not, the plugin raises an explicit error (see below). catalogs refreshed (removed format strings marked obsolete). **Outcome:** DONE. + +--- + +## 63. i18n review fixes (Truman's review of the translation PR) + +**Date:** 2026-10-07 + +**What changed:** + +- `make-release.sh` + - Step 3 now also runs `tests/test_i18n.py`, `tests/test_translations.py`, + `tests/test_cli_*.py` and `tests/test_import_will_details.py` (the i18n + tests were never run by the release gate; the CLI and will-import tests + were not either). + - Checks up front that Babel is importable by `python3` and stops with + "pip install babel" otherwise: the i18n tests (step 3) and `build_zip.py` + (step 5) both need it, so a release no longer stops half-way. +- `AGENTS.md`: Babel must be installed in the runtime env used for releases. +- `pyproject.toml`: `[tool.ruff.lint.isort] known-first-party = ["bal"]`. + WHY: the reported I001 in `tests/test_translations.py` is not reproducible + from the repository root (ruff 0.15.11 and 0.6.9: clean), but it appears as + soon as ruff classifies `bal` as third-party (then 20 files report I001). + Removing the blank line would break the check where `bal` is first-party; + declaring it makes the result the same on every machine. +- `bal/core/willexecutors.py` - `get_willexecutor_transactions` returns COPIES + of the executor records. It used to write `txs`, `txsids` and the + translated `broadcast_status` into the will item's own `we` dict, which is + saved in the wallet (stored data depended on the UI language). +- `bal/core/will.py` - `WillItem` drops those transient fields when loading a + will saved by an older version, and maps a translated leading "New" + ("Nuovo", "Nuevo" - saved by i18n test builds) back to "New". +- `bal/gui/qt/window.py`, `bal/cli/controller.py`: the status history of a new + will item is saved as the literal "New" (was `_("New")`). +- `tests/test_core_status_language_neutral.py` (new, 7 tests; 5 fail on + `main`): English "New" stored, translated "New" mapped back, the mapping + covers every catalog's translation of "New", the push helper does not + modify the will item, old transient fields dropped on load. + +**Verification:** extended release test list (as above) plus the new test +files: 489 passed, 1 failed (pre-existing: `test_group_e_mock_karen7::test_e5` +needs `tests/karen7`, not in the repository; `main` 827a1d1 gives 456 passed, +same failure). `ruff check bal/ tests/`: the same 3 pre-existing I001 as on +`main` (`dialogs.py`, `test_core_animated_qr.py`, `test_core_qr_transfer.py`), +none new. + +**Outcome:** DONE (pending the owner's test of the ZIP and Truman's review). + +--- + +## 64. Invalidation pays the current network fee (2 blocks) instead of `baltx_fees` + +**Date:** 2026-09-26 (ported to `main` 827a1d1 on 2026-10-07) + +**Goal (owner request):** the will's `baltx_fees` (default 20 sat/vbyte) is +deliberately high because the inheritance transactions may be broadcast by the +will-executors years from now, when network fees may be much higher. The +on-chain INVALIDATION, instead, is sent TODAY: it spends the will's coins back +to the wallet so the old pre-signed inheritance transactions become unusable. +It must therefore pay the current network fee, not the inheritance rate. After +the invalidation the plugin rebuilds the new inheritance with `baltx_fees` as +before (unchanged). + +**What changed:** + +- `bal/core/will.py` + - New constant `INVALIDATION_FEE_ETA_BLOCKS = 2` (owner decision: confirm + within 2 blocks, Electrum's own default target). + - New `Will.get_invalidation_fee_rate(wallet, fallback_fee_per_byte)`: + returns Electrum's live estimate for 2 blocks + (`network.fee_estimates.eta_target_to_fee`, sat/kvbyte → sat/vbyte), or + the fallback (the will's `baltx_fees`, i.e. the previous behaviour) when + there is no usable estimate: offline wallet, estimates not received yet, + network error or an invalid value. Never raises. + - `Will.invalidate_will`: uses that rate; the total fee is rounded UP to + whole satoshis (network rates can be fractional); logs the chosen rate and + its source (`network` / `fallback`). The `fees_per_byte` argument is now + only the fallback, so every caller (Tools → Invalidate, wizard + postpone/expired paths, automatic invalidation, CLI `bal_will_invalidate`) + gets the new behaviour without being changed. Docstrings added. +- `tests/test_core_invalidation_fee.py` (new): rate selection (2-block target, + network value used as is also above `baltx_fees`, fractional rates, fallback + when offline / no estimate / error / invalid values) and the fee actually + deducted by `invalidate_will` (network vs fallback, rounding up). + +**Why no special case for an already-expired will:** discussed with the owner +and intentionally left out. Before its locktime the old inheritance tx cannot +enter a block or the mempool, so the invalidation has no competitor. RBF stays +enabled on the invalidation, so its fee can be raised from Electrum's History +("Increase fee") if it ever gets stuck. + +**Verification:** + +- `tests/test_core_invalidation_fee.py`: 12 passed (pytest and standalone). +- `make-release.sh` test list + `tests/test_cli_*.py`: 435 passed, 1 failed. + The failure is pre-existing and unrelated: `test_group_e_mock_karen7::test_e5` + needs `tests/karen7`, a wallet file that is not in the repository. + `tests/test_import_will_details.py`: 17 passed. +- `ruff check` clean on the touched files. +- Live, Electrum 4.8.2 testnet, offline daemon: `bal_will_invalidate` → + "invalidation fee: 20 sat/vbyte (fallback), total 2200 sat" (unchanged + behaviour when offline). +- Real Electrum wallet + real `FeeTimeEstimates` (1 input, 110 vbytes): + market 3 sat/vbyte → 330 sat; 1.5 → 165 sat; 0.4 → 110 sat (Electrum clamps + to its 1 sat/vbyte minimum relay fee); 45 → 4950 sat; no 2-block estimate → + fallback 20 sat/vbyte (2200 sat). +- API checked in Electrum 4.7.2, 4.8.0 and 4.8.2 + (`FeeTimeEstimates.eta_target_to_fee`, `network.fee_estimates`). + +**Notes:** on regtest Electrum returns a fixed 150 sat/vbyte when it has no +estimate, so regtest invalidations will pay that rate. + +**Outcome:** DONE (pending the owner's test of the ZIP). + +--- + +## 65. Check crashed with `KeyError: ''` (stale "father" link in the will tree) + +**Date:** 2026-09-27 + +**Symptom (owner's mainnet wallet, 23 will items, Electrum 4.8.2):** the Check +wizard stopped at "Checking your will" with `Error: 'a65a682f…'`, and the +window refresh logged "error while updating window" with the same txid: + +``` +File "bal/core/will.py", line 657, in check_invalidated + or willtree[w.father].get_status("CONFIRMED") +KeyError: 'a65a682f2975f0dffd9eccce9392833f227badf617fc67815674736d594d4115' +``` + +**Root cause:** every will item stores its parent will tx in `father`, which is +persisted in the wallet (`"Father"` in `WillItem.to_dict`). `Will.add_willtree` +only ever SET it (`if not father: father = ...`) and never cleared it, so when +the parent tx was later removed from the will (rebuild, merge, replacement) the +child kept pointing to a txid that no longer exists. `check_invalidated` then +indexed the will with it. Reproduced on unmodified `main` (not related to #64). + +**What changed:** + +- `bal/core/will.py` + - `Will.add_willtree`: resets `father` on every item before rebuilding the + links, so the tree is always recomputed from the transactions themselves + (a stale value loaded from the wallet is discarded). Docstring added. + - `Will.check_invalidated`: defensive lookup - a father that is not (or no + longer) in the will is treated as "no father" instead of raising. +- `tests/test_core_willtree_father.py` (new): stale father discarded / replaced + by the real parent, no crash in `check_invalidated` (with and without the + tree rebuild), real parent/child link built, child of an unconfirmed parent + still skipped. 4 of the 6 tests fail on `main` and pass with the fix; the 2 + "normal behaviour" tests pass on both. + +**Verification:** make-release test list + `tests/test_cli_*.py` + +`tests/test_import_will_details.py`: 458 passed, 1 failed (pre-existing, +unrelated: `test_group_e_mock_karen7::test_e5` needs the missing `tests/karen7` +wallet file). `ruff check` clean on the touched files. + +**Found while writing the tests, NOT fixed here (reported to the owner):** +`Will.set_invalidate` iterates `children.items()`, but `add_willtree` builds +`children` as a LIST, so invalidating an item that has child will txs raises +`AttributeError: 'list' object has no attribute 'items'` (reproduced on +`main`). The existing tests in `test_core_will_invalidate.py` set `children` to +dicts by hand, so they do not see it. + +**Outcome:** DONE (pending the owner's test of the ZIP). + +--- + +## 66. Invalidation cascade crashed on real will trees (`children` is a list) + +**Date:** 2026-09-27 + +**Problem (found while fixing #65, reproduced on `main`):** +`Will.set_invalidate` iterated `children.items()`, but `Will.add_willtree` +builds `children` as a LIST of `[child_id, input_idx, prevout_idx]`, so +invalidating a will tx that has child will txs raised +`AttributeError: 'list' object has no attribute 'items'`. The existing tests +set `children` to dicts by hand, so they never saw it. + +**What changed:** `bal/core/will.py` - `set_invalidate` accepts both shapes +(list from `add_willtree`, dict from older data/tests) and only recurses into +children that are still in the will. `tests/test_core_willtree_father.py`: new +test `test_set_invalidate_cascades_to_children_built_by_add_willtree` (fails +on `main`, passes with the fix). + +**Verification:** 459 passed, 1 failed (pre-existing missing `tests/karen7`); +`ruff check` clean. + +**Outcome:** DONE (included in the owner's test ZIP of 2026-09-27). + +--- + +## 67. Deselected will-executor: its tx can be deleted and is no longer pushed + +**Date:** 2026-09-27 + +**Problem (owner, mainnet wallet):** the will had a signed tx for +`we.divinaly.support` whose push had failed ("Push failed", "Not on server"). +After deselecting that executor, Check still reported the will as valid, the +row stayed, and "delete" in the context menu was disabled (valid + signed +txs were never deletable). The broadcast also still targeted the deselected +executor, because each will item keeps its own build-time copy of the +executor record (`selected` = True). + +**What changed (owner chose option "B"):** + +- `bal/gui/qt/lists.py` - `_can_delete`: a valid, signed tx is now deletable + when its will-executor NEVER received it (not PUSHED and not CHECKED). It + stays protected when the executor holds it (deleting it locally would not + cancel it on the server) and when it has no executor (backup tx). +- `bal/core/willexecutors.py` - `get_willexecutor_transactions(..., + current_willexecutors=None)`: when the current executor list is given, an + executor that is no longer selected (or no longer listed) is skipped. +- `bal/gui/qt/window.py` (manual broadcast) and `bal/cli/controller.py` + (`bal_will_broadcast`) pass the current list read from the config. The + wizard already filtered by the current selection. +- Tests: `tests/test_gui_will_menu.py` (+3: deletable when never received, + protected when pushed/checked, protected backup tx) and new + `tests/test_core_deselected_executor.py` (4). + +**Not changed (proposal for Truman, option "A"):** Check still does not treat +a tx of a deselected executor as "not coherent" (no automatic rebuild). + +**Verification:** 483 passed, 1 failed (pre-existing missing `tests/karen7`); +`ruff check` clean on the touched files. Live (Electrum 4.8.2 testnet, CLI): +will built for 2 executors, one deselected → `bal_will_broadcast` targets only +the selected one (before the fix it targeted both). + +**Outcome:** DONE (pending the owner's test of the ZIP). + +**Porting note (entries 64-67):** first written on `main` 4a33116 as +58-61 (numbers then taken by the i18n and Check Alive work), re-applied on +`main` 827a1d1 on 2026-10-07 together with #63: extended release test list +489 passed, 1 pre-existing failure (missing `tests/karen7`); `ruff` no new +errors. diff --git a/bal/cli/controller.py b/bal/cli/controller.py index 5157ee8..aaeb23d 100644 --- a/bal/cli/controller.py +++ b/bal/cli/controller.py @@ -385,7 +385,9 @@ class BalController: tx["heirsvalue"] = txs[txid].heirsvalue tx["description"] = txs[txid].description tx["willexecutor"] = copy_structure(txs[txid].willexecutor) - tx["status"] = _("New") + # Stored history: always English (translated only for display by + # format_status_history); never _() here. + tx["status"] = "New" tx["baltx_fees"] = txs[txid].tx_fees tx["time"] = creation_time tx["heirs"] = copy_structure(txs[txid].heirs) @@ -765,7 +767,11 @@ class BalController: if not willitems: raise UserFacingException(_("No transaction matches the given txids")) - willexecutors = Willexecutors.get_willexecutor_transactions(willitems, force=force) + # Skip executors the user deselected after building the will (the + # controller's list is read from the config for every command). + willexecutors = Willexecutors.get_willexecutor_transactions( + willitems, force=force, current_willexecutors=self.willexecutors + ) if not willexecutors: return {} diff --git a/bal/core/will.py b/bal/core/will.py index 46a00d8..323e820 100644 --- a/bal/core/will.py +++ b/bal/core/will.py @@ -26,6 +26,7 @@ The status flags themselves (the source of truth) stay here; only the mapping "status -> colour" now lives in the GUI layer. No behaviour changed. """ +import math from datetime import datetime, timezone from electrum.address_synchronizer import TX_HEIGHT_FUTURE, TX_HEIGHT_LOCAL @@ -50,6 +51,13 @@ from .willexecutors import Willexecutors MIN_LOCKTIME = 1 MIN_BLOCK = 1 +# Confirmation target (in blocks) for the on-chain INVALIDATION transaction. +# WHY: the invalidation is sent TODAY (it spends the will's coins so the old +# pre-signed inheritance txs become unusable), so it must pay the CURRENT +# network fee for a quick confirmation - not ``baltx_fees``, which is a +# deliberately high rate reserved for inheritance txs that may be broadcast +# years from now. 2 blocks is Electrum's own default target (owner decision). +INVALIDATION_FEE_ETA_BLOCKS = 2 _logger = get_logger(__name__) @@ -68,6 +76,22 @@ class Will: # build a tree with parent transactions @staticmethod def add_willtree(will): + """(Re)build the parent/child links between the will transactions. + + Every item gets ``children`` (the will txs spending its outputs) and, + when it spends the output of another will tx, a ``father``. + + WHY the reset below: ``father`` is persisted in the wallet (the + ``"Father"`` field of ``WillItem.to_dict``) and was only ever set, + never cleared. When the father tx was later removed from the will + (rebuild, merge, replacement) the child kept pointing to a txid that + no longer exists, and ``check_invalidated`` crashed with a + ``KeyError`` on that txid (reported on a real wallet: the Check + wizard and the window refresh both failed). The tree is therefore + always recomputed from scratch from the transactions themselves. + """ + for willid in will: + will[willid].father = None for willid in will: will[willid].children = Will.get_children(will, willid) for child in will[willid].children: @@ -438,9 +462,68 @@ class Will: out[inp.prevout.to_str()] = inp return out + @staticmethod + def get_invalidation_fee_rate(wallet, fallback_fee_per_byte): + """Return ``(fee_rate, source)`` for the on-chain invalidation tx. + + ``fee_rate`` is in sat/vbyte. ``source`` is ``"network"`` when the rate + comes from Electrum's live fee estimate, ``"fallback"`` otherwise. + + WHY: the invalidation is broadcast NOW, so it pays the current network + fee for a confirmation within ``INVALIDATION_FEE_ETA_BLOCKS`` blocks. + The will's ``baltx_fees`` is kept only as a fallback, used when no + estimate is available (offline wallet, estimates not received yet, + server error), which is exactly the behaviour of previous versions. + + The estimate is read from ``network.fee_estimates.eta_target_to_fee`` + (Electrum 4.7.2 / 4.8.x), which returns sat/kvbyte and already clamps + the value between Electrum's conservative minimum relay fee (1 + sat/vbyte) and its maximum dynamic fee. This method never raises. + + Args: + wallet: The Electrum wallet (its ``network`` may be ``None``). + fallback_fee_per_byte: Rate (sat/vbyte) used when there is no + usable estimate - normally the will's ``baltx_fees``. + """ + network = getattr(wallet, "network", None) if wallet else None + fee_per_kb = None + if network is not None: + try: + fee_per_kb = network.fee_estimates.eta_target_to_fee( + INVALIDATION_FEE_ETA_BLOCKS + ) + except Exception as e: + _logger.warning(f"invalidation fee: no network estimate ({e!r})") + fee_per_kb = None + # Accept only a real positive integer (Electrum's type for sat/kvbyte); + # ``bool`` is excluded explicitly because it is a subclass of ``int``. + if ( + isinstance(fee_per_kb, int) + and not isinstance(fee_per_kb, bool) + and fee_per_kb > 0 + ): + return fee_per_kb / 1000, "network" + return fallback_fee_per_byte, "fallback" + @staticmethod def invalidate_will(will, wallet, fees_per_byte, history_label=None, will_locktime=None): + """Build the (unsigned) on-chain invalidation transaction. + + The transaction spends every coin used by the valid will items back to + one of the wallet's own change addresses, so the pre-signed + inheritance transactions held by the will-executors become unusable. + + Fee: the CURRENT network fee for a confirmation within + ``INVALIDATION_FEE_ETA_BLOCKS`` blocks (see + :meth:`get_invalidation_fee_rate`). ``fees_per_byte`` (the will's + ``baltx_fees``) is only the FALLBACK used when no network estimate is + available. RBF stays enabled, so the fee can still be raised from + Electrum's History ("Increase fee") if the transaction gets stuck. + + Returns the ``PartialTransaction``, or ``None`` when there is nothing + to invalidate or the balance does not cover the fee. + """ _logger.debug("invalidate tx in will module") will_only_valid = Will.only_valid_list(will) inputs = Will.get_all_inputs(will_only_valid) @@ -477,7 +560,17 @@ class Will: utxo_to_spend, [out], locktime=locktime, version=2 ) tx.set_rbf(True) - fee = tx.estimated_size() * fees_per_byte + fee_rate, fee_source = Will.get_invalidation_fee_rate( + wallet, fees_per_byte + ) + # The network rate can be fractional (e.g. 1.5 sat/vbyte): round + # the total fee UP to whole satoshis so the output value is an + # integer and the rate is never below the one requested. + fee = int(math.ceil(tx.estimated_size() * fee_rate)) + _logger.info( + f"invalidation fee: {fee_rate} sat/vbyte ({fee_source}), " + f"total {fee} sat" + ) if balance - fee > 0: out = PartialTxOutput.from_address_and_value( change_addresses[0], balance - fee @@ -562,9 +655,15 @@ class Will: def set_invalidate(wid, will=None): will = will if will is not None else {} will[wid].set_status("INVALIDATED", True) - if will[wid].children: - for c in will[wid].children.items(): - Will.set_invalidate(c[0], will) + children = will[wid].children or [] + # add_willtree builds a LIST of [child_id, input_idx, prevout_idx]; + # a dict keyed by child id is still accepted (older data / tests). + child_ids = children.keys() if isinstance(children, dict) else [ + c[0] for c in children + ] + for child_id in child_ids: + if child_id in will: + Will.set_invalidate(child_id, will) @staticmethod def check_tx_height(tx, wallet): @@ -575,10 +674,15 @@ class Will: @staticmethod def check_invalidated(willtree, utxos_list, wallet): for wid, w in willtree.items(): + # Defensive: a father that is not (or no longer) part of the will + # is treated as "no father" instead of raising KeyError, so the + # item's inputs are checked against the wallet UTXOs like any + # root transaction. See add_willtree for the root cause. + father = willtree.get(w.father) if w.father else None if ( - not w.father - or willtree[w.father].get_status("CONFIRMED") - or willtree[w.father].get_status("MEMPOOL") + father is None + or father.get_status("CONFIRMED") + or father.get_status("MEMPOOL") ): for inp in w.tx.inputs(): inp_str = Util.utxo_to_str(inp) @@ -1364,6 +1468,37 @@ class WillItem(Logger): # format_status_history() still translates them. LEGACY_STATUS_LABELS = (N_("New"),) + # Translations of "New" that test builds of the i18n work saved INTO the + # status history (window.py / the CLI used ``_("New")`` when building). + # They are mapped back to "New" on load so the history is language + # neutral again. Keep in sync with the "New" msgstr of every catalog in + # bal/locale (tests/test_core_status_language_neutral.py checks it). + TRANSLATED_NEW_LABELS = ("Nuovo", "Nuevo") + + # Fields that Willexecutors.get_willexecutor_transactions / + # push_transactions_to_willexecutor used to write into the item's own + # executor record (older versions); they are transient push data. + TRANSIENT_PUSH_FIELDS = ("txs", "txsids", "broadcast_status") + + @staticmethod + def _untranslate_new_status(status): + """Map a translated leading "New" of a saved history back to "New".""" + head, sep, rest = status.partition(".") + if head in WillItem.TRANSLATED_NEW_LABELS: + return "New" + sep + rest + return status + + @staticmethod + def _without_transient_push_fields(we): + """Return ``we`` without the transient push fields (a copy if needed).""" + if not isinstance(we, dict) or not any( + k in we for k in WillItem.TRANSIENT_PUSH_FIELDS + ): + return we + return { + k: v for k, v in we.items() if k not in WillItem.TRANSIENT_PUSH_FIELDS + } + def set_status(self, status, value=True): """Set a status flag and apply the related side effects. @@ -1445,8 +1580,10 @@ class WillItem(Logger): w = data self.tx = Will.get_tx_from_any(w["tx"]) self.heirs = w.get("heirs", None) - self.we = w.get("willexecutor", None) - self.status = w.get("status") or "" + self.we = WillItem._without_transient_push_fields( + w.get("willexecutor", None) + ) + self.status = WillItem._untranslate_new_status(w.get("status") or "") self.description = w.get("description", None) self.time = w.get("time", None) self.change = w.get("change", None) diff --git a/bal/core/willexecutors.py b/bal/core/willexecutors.py index 54bbec0..38bb4c0 100644 --- a/bal/core/willexecutors.py +++ b/bal/core/willexecutors.py @@ -230,7 +230,21 @@ class Willexecutors: return True @staticmethod - def get_willexecutor_transactions(will, force=False): + def get_willexecutor_transactions(will, force=False, current_willexecutors=None): + """Group the valid, signed, not-yet-pushed will txs by will-executor. + + Returns ``{url: executor_record}`` where each record gets ``txs`` (the + raw txs, one per line) and ``txsids``. + + ``current_willexecutors`` is the CURRENT executor list from the config + (``{url: record}``). When given, an executor that is no longer in it or + no longer selected is skipped. WHY: every will item keeps its own copy + of the executor record taken at build time (``selected`` = True), so + without this check a deselected executor would still receive the will. + ``None`` keeps the previous behaviour (only the item's own copy). + + The returned records are COPIES: the will items are never modified. + """ willexecutors = {} for wid, willitem in will.items(): if willitem.get_status("VALID"): @@ -238,12 +252,26 @@ class Willexecutors: if not willitem.get_status("PUSHED") or force: if willexecutor := willitem.we: url = willexecutor["url"] + if current_willexecutors is not None and not ( + Willexecutors.is_selected( + current_willexecutors.get(url) + ) + ): + continue if willexecutor and Willexecutors.is_selected(willexecutor): if url not in willexecutors: - willexecutor["txs"] = "" - willexecutor["txsids"] = [] - willexecutor["broadcast_status"] = _("Waiting...") - willexecutors[url] = willexecutor + # Work on a COPY of the record: the item's + # own ``we`` dict is saved in the wallet, so + # the transient push fields below (raw txs, + # txids and the - translated - broadcast + # status shown in the push dialog) must + # never end up in it (stored data must not + # depend on the UI language, see bal/i18n.py). + record = dict(willexecutor) + record["txs"] = "" + record["txsids"] = [] + record["broadcast_status"] = _("Waiting...") + willexecutors[url] = record willexecutors[url]["txs"] += str(willitem.tx) + "\n" willexecutors[url]["txsids"].append(wid) diff --git a/bal/gui/qt/lists.py b/bal/gui/qt/lists.py index 9f15edb..70a240b 100644 --- a/bal/gui/qt/lists.py +++ b/bal/gui/qt/lists.py @@ -93,13 +93,27 @@ def _can_broadcast(will_item): def _can_delete(will_item): - """True if a will transaction may be deleted (invalid or unsigned). + """True if a will transaction may be deleted. - Valid AND fully-signed transactions are never deletable: removing them - would silently drop a committed inheritance. + Deletable: + * invalid or not fully signed transactions (unchanged rule); + * valid AND fully signed transactions that their will-executor NEVER + received: not PUSHED and not CHECKED (e.g. "Push failed" / + "Not on server", or never sent). + + Not deletable: a valid, signed transaction that the will-executor holds + (PUSHED or CHECKED). WHY: deleting it only on this PC would not remove it + from the executor's server, so the user would wrongly believe that + inheritance was cancelled; that case needs an on-chain invalidation. + Signed transactions without a will-executor (backup txs) also stay + protected, as before. """ - return bool(will_item) and ( - not will_item.get_status("VALID") or not will_item.get_status("COMPLETE") + if not will_item: + return False + if not will_item.get_status("VALID") or not will_item.get_status("COMPLETE"): + return True + return bool(getattr(will_item, "we", None)) and not ( + will_item.get_status("PUSHED") or will_item.get_status("CHECKED") ) diff --git a/bal/gui/qt/window.py b/bal/gui/qt/window.py index be021e9..734e1c6 100644 --- a/bal/gui/qt/window.py +++ b/bal/gui/qt/window.py @@ -551,7 +551,9 @@ class BalWindow: tx["heirsvalue"] = txs[txid].heirsvalue tx["description"] = txs[txid].description tx["willexecutor"] = copy_structure(txs[txid].willexecutor) - tx["status"] = _("New") + # Stored history: always English (translated only for display by + # format_status_history); never _() here. + tx["status"] = "New" tx["baltx_fees"] = txs[txid].tx_fees tx["time"] = creation_time tx["heirs"] = copy_structure(txs[txid].heirs) @@ -1630,7 +1632,15 @@ class BalWindow: willitems = { t: willitems[t] for t in txids if t in willitems } - willexecutors = Willexecutors.get_willexecutor_transactions(willitems, force=force) + # Read the CURRENT executor list from the config, so an executor the + # user deselected after building the will is not contacted anymore. + willexecutors = Willexecutors.get_willexecutor_transactions( + willitems, + force=force, + current_willexecutors=Willexecutors.get_willexecutors( + self.bal_plugin, update=False, task=False + ), + ) def getMsg(willexecutors): msg = _("Broadcasting Transactions to Will-Executors:") + "\n" diff --git a/make-release.sh b/make-release.sh index a2d7cbf..b713e29 100755 --- a/make-release.sh +++ b/make-release.sh @@ -5,7 +5,7 @@ # ./make-release.sh # read version from bal/manifest.json # ./make-release.sh v0.6.2 # bump manifest to 0.6.2, then release # -# Requires: git, gpg, curl, python3, sha256sum +# Requires: git, gpg, curl, python3, sha256sum, Babel for python3 (pip install babel) # Optional: ruff (lint skipped if not installed) # Credentials: ~/.git-credentials or GITEA_USER / GITEA_TOKEN env vars @@ -33,6 +33,12 @@ else NEW_VER="" fi +# Babel is needed by the i18n tests (step 3) and by build_zip.py, which +# compiles the translation catalogs into the zip (step 5). Check it up front +# so a release never stops half-way. +python3 -c "import babel" 2>/dev/null \ + || die "Babel is missing for $(command -v python3): pip install babel" + echo "=== Release ${TAG} ===" echo "Current manifest version: ${CURRENT_VER}" [ -n "$NEW_VER" ] && echo "New version (will bump): ${NEW_VER}" @@ -68,6 +74,8 @@ if QT_QPA_PLATFORM=offscreen PYTHONPATH=electrum-src python3 -m pytest \ tests/test_group_d_alarms.py tests/test_group_e_mock_karen7.py \ tests/test_group_f_heir_change_rebuild.py tests/test_group_g_basic_calendar.py \ tests/test_group_h_v048.py \ + tests/test_i18n.py tests/test_translations.py \ + tests/test_cli_*.py tests/test_import_will_details.py \ -q 2>&1; then echo "All tests passed." else diff --git a/pyproject.toml b/pyproject.toml index 879f935..18aca70 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,6 +6,12 @@ target-version = "py312" select =["E", "W", "F", "I", "N", "B", "INT"] # INT: gettext rules (see bal/i18n.py) ignore = ["E501"] +[tool.ruff.lint.isort] +# Classify the plugin package explicitly: otherwise ruff decides from where it +# is run / how the project root is detected, and the same import block is +# "sorted" on one machine and I001 on another. +known-first-party = ["bal"] + [tool.ruff.lint.pep8-naming] classmethod-decorators = ["classmethod", "classproperty"] # electrum.util.classproperty uses cls diff --git a/tests/test_core_deselected_executor.py b/tests/test_core_deselected_executor.py new file mode 100644 index 0000000..b41acc0 --- /dev/null +++ b/tests/test_core_deselected_executor.py @@ -0,0 +1,82 @@ +""" +Tests for ``Willexecutors.get_willexecutor_transactions`` with the CURRENT +executor selection (``current_willexecutors``). + +Every will item keeps its own copy of the executor record taken at build time +(``selected`` = True). Without the current selection, an executor the user +deselected after building the will would still receive the will. + +Run (standalone or with pytest): + QT_QPA_PLATFORM=offscreen PYTHONPATH=electrum-src \ + python3 tests/test_core_deselected_executor.py +""" + +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir)) + +from bal.core.willexecutors import Willexecutors + + +class FakeItem: + """Minimal will item: statuses, a tx and the build-time executor copy.""" + + def __init__(self, url, **flags): + self._status = {"VALID": True, "COMPLETE": True} + self._status.update(flags) + self.we = {"url": url, "selected": True} + self.tx = f"rawtx-{url}" + + def get_status(self, name): + return self._status.get(name, False) + + +A = "https://we.a.example" +B = "https://we.b.example" + + +def _will(): + return {"tx_a": FakeItem(A), "tx_b": FakeItem(B)} + + +def test_without_current_list_previous_behaviour(): + assert set(Willexecutors.get_willexecutor_transactions(_will())) == {A, B} + + +def test_deselected_executor_is_skipped(): + current = {A: {"selected": True}, B: {"selected": False}} + out = Willexecutors.get_willexecutor_transactions( + _will(), current_willexecutors=current + ) + assert set(out) == {A} + assert out[A]["txsids"] == ["tx_a"] + + +def test_executor_removed_from_list_is_skipped(): + current = {A: {"selected": True}} + out = Willexecutors.get_willexecutor_transactions( + _will(), current_willexecutors=current + ) + assert set(out) == {A} + + +def test_already_pushed_still_excluded_unless_forced(): + will = {"tx_a": FakeItem(A, PUSHED=True)} + current = {A: {"selected": True}} + assert Willexecutors.get_willexecutor_transactions( + will, current_willexecutors=current + ) == {} + assert set( + Willexecutors.get_willexecutor_transactions( + will, force=True, current_willexecutors=current + ) + ) == {A} + + +if __name__ == "__main__": + for name in sorted(dir()): + if name.startswith("test_"): + globals()[name]() + print(f" [OK] {name}") + print("[OK] All deselected-executor tests passed") diff --git a/tests/test_core_invalidation_fee.py b/tests/test_core_invalidation_fee.py new file mode 100644 index 0000000..c1cc4c7 --- /dev/null +++ b/tests/test_core_invalidation_fee.py @@ -0,0 +1,231 @@ +""" +Tests for the fee of the on-chain INVALIDATION transaction. + +The invalidation is broadcast TODAY (it spends the will's coins so the old +pre-signed inheritance transactions become unusable), therefore it must pay the +CURRENT network fee for a confirmation within +``INVALIDATION_FEE_ETA_BLOCKS`` (2) blocks. The will's ``baltx_fees`` (a +deliberately high rate meant for inheritance transactions broadcast years from +now) is only the FALLBACK used when no network estimate is available. + +Covers: +* Will.get_invalidation_fee_rate() - rate selection and fallback rules +* Will.invalidate_will() - the chosen rate is really used for the fee + +Run (standalone or with pytest): + QT_QPA_PLATFORM=offscreen PYTHONPATH=electrum-src \ + python3 tests/test_core_invalidation_fee.py + QT_QPA_PLATFORM=offscreen PYTHONPATH=electrum-src \ + python3 -m pytest tests/test_core_invalidation_fee.py -q +""" + +import os +import sys +from unittest.mock import MagicMock, patch + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir)) + +from electrum.transaction import Transaction + +from bal.core.will import INVALIDATION_FEE_ETA_BLOCKS, Will, WillItem + +# WillItem parses a raw tx and calls add_info_from_wallet; no live wallet here. +_patcher = patch.object(Transaction, "add_info_from_wallet") +_patcher.start() + +# A valid serialized transaction (1 input, 1 output) reused by the core tests. +_VALID_TX_HEX = ( + "01000000012a5c9a94fcde98f5581cd00162c60a13936ceb75389ea65b" + "f38633b424eb4031000000006c493046022100a82bbc57a0136751e543" + "3f41cf000b3f1a99c6744775e76ec764fb78c54ee100022100f9e80b7d" + "e89de861dc6fb0c1429d5da72c2b6b2ee2406bc9bfb1beedd729d98501" + "2102e61d176da16edd1d258a200ad9759ef63adf8e14cd97f53227bae3" + "5cdb84d2f6ffffffff0140420f00000000001976a914230ac37834073a" + "42146f11ef8414ae929feaafc388ac00000000" +) +# The outpoint spent by _VALID_TX_HEX (input 0). +_PREVOUT_STR = "3140eb24b43386f35ba69e3875eb6c93130ac66201d01c58f598defc949a5c2a:0" +_CHANGE_ADDR = "14CHYaaByjJZpx4oHBpfDMdqhTyXnZ3kVs" +_BALANCE = 1000000 +# The will's baltx_fees used as the fallback rate in every test (sat/vbyte). +_BALTX_FEES = 20 + + +# ------------------------------------------------------------------ # +# Helpers +# ------------------------------------------------------------------ # + +def _wallet_with_estimate(fee_per_kb): + """Wallet whose network returns ``fee_per_kb`` (sat/kvbyte) for any ETA. + + ``fee_per_kb`` may also be an Exception instance: the estimate call then + raises it, to simulate a broken network object. + """ + wallet = MagicMock() + wallet.get_change_addresses_for_new_transaction.return_value = [_CHANGE_ADDR] + network = MagicMock() + if isinstance(fee_per_kb, Exception): + network.fee_estimates.eta_target_to_fee.side_effect = fee_per_kb + else: + network.fee_estimates.eta_target_to_fee.return_value = fee_per_kb + wallet.network = network + return wallet + + +def _make_will(): + """One VALID will item spending _PREVOUT_STR with a known input value.""" + item = WillItem({ + "tx": _VALID_TX_HEX, + "heirs": {"alice": ["addr_alice", 5000, "30d"]}, + "willexecutor": None, + "status": "", + "description": "", + "time": 0, + "change": "", + "baltx_fees": _BALTX_FEES, + }) + item.STATUS = WillItem.copy_status_table(WillItem.STATUS_DEFAULT) + item.tx.inputs()[0]._TxInput__value_sats = _BALANCE + return {"willtxid1": item} + + +def _make_utxo(): + """Wallet-side UTXO matching the will input.""" + utxo = MagicMock() + utxo.prevout.to_str.return_value = _PREVOUT_STR + utxo.is_coinbase_output.return_value = False + utxo.block_height = 1 + utxo.value_sats.return_value = _BALANCE + return utxo + + +def _invalidation_output_value(wallet, estimated_size=200): + """Run Will.invalidate_will and return the value of the final output. + + Electrum's tx building is mocked (as in test_core_will_invalidate.py): + ``estimated_size`` is the vsize the fee is computed from. + """ + wallet.get_utxos.return_value = [_make_utxo()] + mock_output = MagicMock() + mock_tx = MagicMock() + mock_tx.estimated_size.return_value = estimated_size + with patch("bal.core.will.Util.get_current_height", return_value=800000), \ + patch("electrum.transaction.PartialTxOutput.from_address_and_value", + return_value=mock_output) as mock_out, \ + patch("electrum.transaction.PartialTransaction.from_io", + return_value=mock_tx): + result = Will.invalidate_will(_make_will(), wallet, _BALTX_FEES) + assert result is mock_tx + # 1st call: full balance (size estimation); 2nd call: balance - fee. + return mock_out.call_args_list[1][0][1] + + +# ------------------------------------------------------------------ # +# Will.get_invalidation_fee_rate +# ------------------------------------------------------------------ # + +def test_target_is_two_blocks(): + """The owner's decision: confirm within 2 blocks.""" + assert INVALIDATION_FEE_ETA_BLOCKS == 2 + wallet = _wallet_with_estimate(3000) + Will.get_invalidation_fee_rate(wallet, _BALTX_FEES) + wallet.network.fee_estimates.eta_target_to_fee.assert_called_once_with(2) + + +def test_network_estimate_is_used(): + """A live estimate (sat/kvbyte) becomes the rate in sat/vbyte.""" + wallet = _wallet_with_estimate(3000) + assert Will.get_invalidation_fee_rate(wallet, _BALTX_FEES) == (3.0, "network") + + +def test_network_estimate_higher_than_baltx_fees_is_used_as_is(): + """No cap: if the market is above baltx_fees, the market rate is used.""" + wallet = _wallet_with_estimate(45000) + assert Will.get_invalidation_fee_rate(wallet, _BALTX_FEES) == (45.0, "network") + + +def test_fractional_estimate_is_kept(): + """Electrum estimates may be fractional (e.g. 1.5 sat/vbyte).""" + wallet = _wallet_with_estimate(1500) + assert Will.get_invalidation_fee_rate(wallet, _BALTX_FEES) == (1.5, "network") + + +def test_offline_wallet_falls_back_to_baltx_fees(): + """No network object (offline): previous behaviour, baltx_fees.""" + wallet = MagicMock() + wallet.network = None + assert Will.get_invalidation_fee_rate(wallet, _BALTX_FEES) == ( + _BALTX_FEES, + "fallback", + ) + + +def test_missing_estimate_falls_back(): + """Estimates not received yet: Electrum returns None.""" + wallet = _wallet_with_estimate(None) + assert Will.get_invalidation_fee_rate(wallet, _BALTX_FEES) == ( + _BALTX_FEES, + "fallback", + ) + + +def test_estimate_error_falls_back_and_does_not_raise(): + """A broken network object must never break the invalidation.""" + wallet = _wallet_with_estimate(RuntimeError("server gone")) + assert Will.get_invalidation_fee_rate(wallet, _BALTX_FEES) == ( + _BALTX_FEES, + "fallback", + ) + + +def test_non_integer_or_invalid_estimates_fall_back(): + """Only a positive int (Electrum's sat/kvbyte type) is trusted.""" + for bad in (0, -1000, True, "3000", 3.5, MagicMock()): + wallet = _wallet_with_estimate(bad) + assert Will.get_invalidation_fee_rate(wallet, _BALTX_FEES) == ( + _BALTX_FEES, + "fallback", + ), bad + + +def test_no_wallet_falls_back(): + assert Will.get_invalidation_fee_rate(None, _BALTX_FEES) == ( + _BALTX_FEES, + "fallback", + ) + + +# ------------------------------------------------------------------ # +# Will.invalidate_will uses the chosen rate +# ------------------------------------------------------------------ # + +def test_invalidation_pays_network_fee_not_baltx_fees(): + """3 sat/vbyte on 200 vbytes = 600 sat (not 200 * 20 = 4000 sat).""" + wallet = _wallet_with_estimate(3000) + assert _invalidation_output_value(wallet) == _BALANCE - 600 + + +def test_invalidation_offline_pays_baltx_fees(): + """Offline: unchanged behaviour, 200 * 20 = 4000 sat.""" + wallet = _wallet_with_estimate(None) + assert _invalidation_output_value(wallet) == _BALANCE - 4000 + + +def test_fractional_fee_is_rounded_up_to_whole_satoshis(): + """1.5 sat/vbyte on 201 vbytes = 301.5 -> 302 sat (never below the rate).""" + wallet = _wallet_with_estimate(1500) + value = _invalidation_output_value(wallet, estimated_size=201) + assert value == _BALANCE - 302 + assert isinstance(value, int) + + +# ------------------------------------------------------------------ # +# Main +# ------------------------------------------------------------------ # + +if __name__ == "__main__": + for name in sorted(dir()): + if name.startswith("test_"): + globals()[name]() + print(f" [OK] {name}") + print("[OK] All invalidation fee tests passed") diff --git a/tests/test_core_status_language_neutral.py b/tests/test_core_status_language_neutral.py new file mode 100644 index 0000000..856c05e --- /dev/null +++ b/tests/test_core_status_language_neutral.py @@ -0,0 +1,136 @@ +""" +Stored will data must not depend on the GUI language (rule in bal/i18n.py). + +Covers the two findings of the i18n review: + +* the status history of a new will item is saved as English "New" (not + ``_("New")``), and histories saved by test builds as "Nuovo"/"Nuevo" are + mapped back to "New" on load; +* the push helper no longer writes transient fields (``txs``, ``txsids`` and + the translated ``broadcast_status``) into the will item's executor record, + and such fields saved by older versions are dropped on load. + +Run (standalone or with pytest): + QT_QPA_PLATFORM=offscreen PYTHONPATH=electrum-src \ + python3 tests/test_core_status_language_neutral.py +""" + +import os +import re +import sys +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir)) + +from electrum.transaction import Transaction + +from bal.core.will import WillItem +from bal.core.willexecutors import Willexecutors + +# WillItem parses a raw tx and calls add_info_from_wallet; no live wallet here. +_patcher = patch.object(Transaction, "add_info_from_wallet") +_patcher.start() + +ROOT = Path(__file__).resolve().parent.parent + +_VALID_TX_HEX = ( + "01000000012a5c9a94fcde98f5581cd00162c60a13936ceb75389ea65b" + "f38633b424eb4031000000006c493046022100a82bbc57a0136751e543" + "3f41cf000b3f1a99c6744775e76ec764fb78c54ee100022100f9e80b7d" + "e89de861dc6fb0c1429d5da72c2b6b2ee2406bc9bfb1beedd729d98501" + "2102e61d176da16edd1d258a200ad9759ef63adf8e14cd97f53227bae3" + "5cdb84d2f6ffffffff0140420f00000000001976a914230ac37834073a" + "42146f11ef8414ae929feaafc388ac00000000" +) +URL = "https://we.example" + + +def _item(status="", we=None): + return WillItem( + {"tx": _VALID_TX_HEX, "heirs": {}, "willexecutor": we, "status": status} + ) + + +# ------------------------------------------------------------------ # +# Status history "New" +# ------------------------------------------------------------------ # + +def test_builders_store_english_new(): + """window.py and the CLI controller must not translate the stored status.""" + for rel in ("bal/gui/qt/window.py", "bal/cli/controller.py"): + src = (ROOT / rel).read_text(encoding="utf-8") + assert re.search(r'tx\["status"\]\s*=\s*"New"', src), rel + assert not re.search(r'tx\["status"\]\s*=\s*_\(', src), rel + + +def test_translated_new_is_mapped_back_on_load(): + assert _item("Nuovo.Signed.Pushed").status == "New.Signed.Pushed" + assert _item("Nuevo").status == "New" + assert _item("New.Signed").status == "New.Signed" + + +def test_only_the_leading_new_is_touched(): + assert _item(".Nuovo").status == ".Nuovo" + assert _item("Signed.Nuovo").status == "Signed.Nuovo" + + +def test_translated_new_labels_match_the_catalogs(): + """Every catalog's translation of "New" must be recognised on load.""" + found = set() + for po in (ROOT / "bal" / "locale").glob("*/LC_MESSAGES/bal.po"): + text = po.read_text(encoding="utf-8") + m = re.search(r'^msgid "New"\nmsgstr "(.*)"$', text, re.MULTILINE) + if m and m.group(1): + found.add(m.group(1)) + assert found, "no catalog translates 'New'" + assert found <= set(WillItem.TRANSLATED_NEW_LABELS), found + + +# ------------------------------------------------------------------ # +# Transient push fields +# ------------------------------------------------------------------ # + +class FakeItem: + def __init__(self): + self.we = {"url": URL, "selected": True, "address": "a", "base_fee": 1} + self.tx = "rawtx" + + def get_status(self, name): + return name in ("VALID", "COMPLETE") + + +def test_push_helper_does_not_modify_the_will_item(): + item = FakeItem() + before = dict(item.we) + out = Willexecutors.get_willexecutor_transactions({"t1": item}) + assert out[URL]["txsids"] == ["t1"] + assert "broadcast_status" in out[URL] + assert item.we == before + assert out[URL] is not item.we + + +def test_transient_fields_saved_by_older_versions_are_dropped_on_load(): + we = { + "url": URL, + "selected": True, + "txs": "raw\n", + "txsids": ["t1"], + "broadcast_status": "Riuscito", + } + item = _item("New", we) + assert item.we == {"url": URL, "selected": True} + assert "broadcast_status" not in item.to_dict()["willexecutor"] + + +def test_clean_executor_record_is_kept_as_is(): + we = {"url": URL, "selected": True} + assert _item("New", we).we is we + + +if __name__ == "__main__": + for name in sorted(dir()): + if name.startswith("test_"): + globals()[name]() + print(f" [OK] {name}") + print("[OK] All language-neutral status tests passed") diff --git a/tests/test_core_willtree_father.py b/tests/test_core_willtree_father.py new file mode 100644 index 0000000..e3fcad1 --- /dev/null +++ b/tests/test_core_willtree_father.py @@ -0,0 +1,145 @@ +""" +Tests for the parent/child tree of the will transactions (``father`` links). + +Bug reproduced from a real wallet (Electrum log, Check wizard): + + File "bal/core/will.py", in check_invalidated + or willtree[w.father].get_status("CONFIRMED") + KeyError: 'a65a682f…' + +``father`` is persisted in the wallet (``"Father"``) and was never cleared, so +after the father tx was removed from the will the child pointed to a txid that +no longer exists. ``Will.add_willtree`` now rebuilds the links from scratch and +``Will.check_invalidated`` treats a missing father as "no father". + +Run (standalone or with pytest): + QT_QPA_PLATFORM=offscreen PYTHONPATH=electrum-src \ + python3 tests/test_core_willtree_father.py +""" + +import os +import sys +from unittest.mock import MagicMock, patch + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir)) + +from electrum.transaction import Transaction + +from bal.core.will import Will, WillItem + +# WillItem parses a raw tx and calls add_info_from_wallet; no live wallet here. +_patcher = patch.object(Transaction, "add_info_from_wallet") +_patcher.start() + +# A valid serialized transaction (1 input, 1 output) reused by the core tests. +_VALID_TX_HEX = ( + "01000000012a5c9a94fcde98f5581cd00162c60a13936ceb75389ea65b" + "f38633b424eb4031000000006c493046022100a82bbc57a0136751e543" + "3f41cf000b3f1a99c6744775e76ec764fb78c54ee100022100f9e80b7d" + "e89de861dc6fb0c1429d5da72c2b6b2ee2406bc9bfb1beedd729d98501" + "2102e61d176da16edd1d258a200ad9759ef63adf8e14cd97f53227bae3" + "5cdb84d2f6ffffffff0140420f00000000001976a914230ac37834073a" + "42146f11ef8414ae929feaafc388ac00000000" +) +# The input of _VALID_TX_HEX spends this outpoint (little-endian in the hex). +_FUNDING_ID = "3140eb24b43386f35ba69e3875eb6c93130ac66201d01c58f598defc949a5c2a" +# PARENT: _VALID_TX_HEX itself. CHILD: the same tx with its input re-pointed to +# the parent's txid, so the child spends the parent's output 0. +_PARENT_ID = Transaction(_VALID_TX_HEX).txid() +_CHILD_TX_HEX = _VALID_TX_HEX.replace( + bytes.fromhex(_FUNDING_ID)[::-1].hex(), bytes.fromhex(_PARENT_ID)[::-1].hex() +) +# A txid that is not in the will (the "removed father" of the bug report). +_MISSING_ID = "a65a682f2975f0dffd9eccce9392833f227badf617fc67815674736d594d4115" + + +def _item(tx_hex=_VALID_TX_HEX, father=None): + """WillItem built from ``tx_hex``, optionally with a stored father.""" + data = {"tx": tx_hex, "heirs": {}, "willexecutor": None} + if father is not None: + data["Father"] = father + return WillItem(data) + + +def _wallet_without_adb(): + """Wallet whose check_invalidated path skips on-chain lookups.""" + wallet = MagicMock() + wallet.adb = None + return wallet + + +def test_stale_father_is_discarded_by_add_willtree(): + child = _item(father=_MISSING_ID) + will = {child._id: child} + Will.add_willtree(will) + assert child.father is None + + +def test_check_invalidated_does_not_crash_on_stale_father(): + """The exact failure of the bug report, now without KeyError.""" + child = _item(father=_MISSING_ID) + will = {child._id: child} + Will.add_willtree(will) + Will.check_invalidated(will, [], _wallet_without_adb()) + + +def test_check_invalidated_tolerates_missing_father_without_add_willtree(): + """Defensive path: even if the tree was not rebuilt, no KeyError.""" + child = _item(father=_MISSING_ID) + will = {child._id: child} + Will.check_invalidated(will, [], _wallet_without_adb()) + assert child.father == _MISSING_ID + + +def test_real_parent_child_link_is_built(): + parent = _item() + child = _item(_CHILD_TX_HEX) + will = {_PARENT_ID: parent, child._id: child} + Will.add_willtree(will) + assert child.father == _PARENT_ID + assert [c[0] for c in parent.children] == [child._id] + assert parent.father is None + + +def test_stale_father_is_replaced_by_the_real_parent(): + parent = _item() + child = _item(_CHILD_TX_HEX, father=_MISSING_ID) + will = {_PARENT_ID: parent, child._id: child} + Will.add_willtree(will) + assert child.father == _PARENT_ID + + +def test_child_of_unconfirmed_parent_is_not_checked_against_utxos(): + """Existing rule kept: a child of a VALID (not mined) parent is skipped.""" + parent = _item() + child = _item(_CHILD_TX_HEX) + will = {_PARENT_ID: parent, child._id: child} + Will.add_willtree(will) + wallet = MagicMock() + # Funding not spent on-chain: nothing gets invalidated, we only observe + # which items had their inputs checked. + with patch.object(Will, "check_tx_height", return_value=-2) as height, \ + patch.object(Will, "_funding_really_spent", return_value=False): + Will.check_invalidated({child._id: child, _PARENT_ID: parent}, [], wallet) + # Only the parent (a root) looked up its height; the child was skipped. + checked = [call.args[0] for call in height.call_args_list] + assert checked == [parent.tx] + + +def test_set_invalidate_cascades_to_children_built_by_add_willtree(): + """add_willtree builds children as a LIST: invalidation must cascade.""" + parent = _item() + child = _item(_CHILD_TX_HEX) + will = {parent._id: parent, child._id: child} + Will.add_willtree(will) + Will.set_invalidate(parent._id, will) + assert parent.get_status("INVALIDATED") is True + assert child.get_status("INVALIDATED") is True + + +if __name__ == "__main__": + for name in sorted(dir()): + if name.startswith("test_"): + globals()[name]() + print(f" [OK] {name}") + print("[OK] All will-tree father tests passed") diff --git a/tests/test_gui_will_menu.py b/tests/test_gui_will_menu.py index 2af33ea..a77f758 100644 --- a/tests/test_gui_will_menu.py +++ b/tests/test_gui_will_menu.py @@ -92,6 +92,31 @@ def test_can_delete_none_false(): assert _can_delete(None) is False +def _signed_valid(we="https://we.example", **flags): + """A VALID + COMPLETE (signed) item, optionally with a will-executor.""" + item = FakeWillItem(VALID=True, COMPLETE=True, **flags) + item.we = {"url": we, "selected": True} if we else None + return item + + +def test_can_delete_signed_but_never_received_by_executor(): + """Push failed / "Not on server": the executor does not hold it.""" + assert _can_delete(_signed_valid(PUSH_FAIL=True, CHECK_FAIL=True)) is True + assert _can_delete(_signed_valid()) is True # signed, never sent + + +def test_can_delete_signed_and_pushed_false(): + """The executor holds it: deleting locally would not cancel it.""" + assert _can_delete(_signed_valid(PUSHED=True)) is False + assert _can_delete(_signed_valid(PUSHED=True, CHECKED=True)) is False + assert _can_delete(_signed_valid(CHECKED=True)) is False + + +def test_can_delete_signed_backup_tx_without_executor_false(): + """Signed txs without a will-executor (backup) stay protected.""" + assert _can_delete(_signed_valid(we=None)) is False + + # ------------------------------------------------------------------ # # _apply_select_all # ------------------------------------------------------------------ #