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 <noreply@anthropic.com>
This commit is contained in:
2026-10-07 11:20:04 +02:00
parent 827a1d1e54
commit 2bb9fe812c
14 changed files with 1089 additions and 25 deletions

View File

@@ -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: '<txid>'` (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.