gui+cli+core: anticipated-delivery warning, task #67 amount checks, CLI tools

GUI - "Building your will" row (owner request): - when a rebuild anticipates the delivery date by one day, show "Anticipated - new delivery date: <date>" on the "Building your will" row instead of a plain "Ok"; - the detection compares the will's minimum locktime BEFORE and AFTER the rebuild, so it also covers relative recipes ("1y"/"30d") that the previous stored-vs-built comparison missed; - the sign explanation stays on its own "Signing your will" row so it no longer overwrites the anticipation warning. Task #67 - "Checking variables" stuck on "Wait": - core: Will.check_amounts only requires a fixed amount to be funded when it is > 0, and only requires percentages to total 100% when the will actually has percentage heirs; fixed-only / empty-wallet wills no longer raise; - gui: the amount warning is written on the "Checking variables" row itself (COLOR_WARNING) instead of being overwritten by the next row; the redundant "adjustment of the amounts" sentence was dropped. CLI - will-executor helpers: - willexecutors_update accepts promo_code; - willexecutors_select supports bulk apply with only_valid; - willexecutors_export supports selected/valid subsets; - import overwrites existing records with sane defaults; - raise NoServersForChainError with a clear user-facing message; map NotCompleteWill / HeirChange / WillexecutorChange in _user_facing. Tests and docs: - tests for the anticipation row, the CLI helpers and the amount checks; - i18n catalogs refreshed; CHANGELOG section 67; agent task memory updated.
2026-10-15 21:55:03 -04:00
parent c9f3f77a15
commit f3deccb626
14 changed files with 1396 additions and 1010 deletions
--- a/tests/test_cli_controller_offline.py
+++ b/tests/test_cli_controller_offline.py
@@ -25,9 +25,14 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir))
 from electrum.simple_config import SimpleConfig
 from electrum.util import UserFacingException

-from bal.cli.controller import BalController
+from bal.cli.controller import BalController, _user_facing
 from bal.core.heirs import Heirs
 from bal.core.util import Util
+from bal.core.will import (
+    HeirChangeException,
+    NotCompleteWillException,
+    WillexecutorChangeException,
+)

 VALID_ADDRESS = "bc1qusymuetsz2psaqzqxv8qmzcy64d9meckj3lxxf"

@@ -102,6 +107,13 @@ def _make_controller(plugin):
    return BalController(plugin, FakeWallet())


+def _tmp_path():
+    """Return the path of a fresh temporary JSON file (caller removes it)."""
+    fd, path = tempfile.mkstemp(prefix="bal_cli_", suffix=".json")
+    os.close(fd)
+    return path
+
+
 def test_controller_init_empty():
    with Plugin() as plugin:
        c = _make_controller(plugin)
@@ -194,6 +206,179 @@ def test_willexecutors_crud():
        assert renamed not in c.willexecutors


+def test_willexecutors_update_accepts_promo_code():
+    """Regression: the command transport used to forward six positional args to
+    a five-parameter controller method, raising TypeError.  ``promo_code`` must
+    also be applied to the record instead of being silently dropped.
+    """
+    with Plugin() as plugin:
+        c = _make_controller(plugin)
+        url = "https://executor-promo.example.invalid"
+        c.willexecutors_add(url, address="", base_fee=250)
+        # Positional call mirroring the transport layer: (url, address,
+        # base_fee, info, promo_code, rename_to).
+        result = c.willexecutors_update(
+            url, "", "300", "Promo executor", "PROMO10", None
+        )
+        assert result["willexecutor"]["base_fee"] == 300
+        assert result["willexecutor"]["info"] == "Promo executor"
+        assert c.willexecutors[url]["promo_code"] == "PROMO10"
+
+
+def test_heirs_locktime_forced_when_multiverse_disabled():
+    """When multiverse is disabled the heir locktime follows the will locktime,
+    exactly like the GUI's ``set_heir`` (which forces ``heir[3]``)."""
+    with Plugin() as plugin:
+        plugin.WILL_SETTINGS.set(
+            {"threshold": "-150d", "locktime": "2y", "baltx_fees": 20}
+        )
+        plugin.ENABLE_MULTIVERSE.set(False)
+        c = _make_controller(plugin)
+        c.heirs_add("bob", VALID_ADDRESS, "100000", locktime="30d")
+        assert c.heirs["bob"][2] == "2y"
+
+
+def test_heirs_locktime_kept_when_multiverse_enabled():
+    with Plugin() as plugin:
+        plugin.WILL_SETTINGS.set({"threshold": "-150d", "locktime": "2y", "baltx_fees": 20})
+        plugin.ENABLE_MULTIVERSE.set(True)
+        c = _make_controller(plugin)
+        c.heirs_add("carol", VALID_ADDRESS, "100000", locktime="30d")
+        assert c.heirs["carol"][2] == "30d"
+
+
+def test_user_facing_maps_not_complete_will():
+    """The check flow must translate every NotCompleteWillException subtype to a
+    user-facing message instead of leaking the raw internal exception.
+    """
+    assert "heirs changed" in str(_user_facing(HeirChangeException())).lower()
+    assert "will-executor changed" in str(
+        _user_facing(WillexecutorChangeException())
+    ).lower()
+    assert "no valid transaction" in str(
+        _user_facing(NotCompleteWillException())
+    ).lower()
+    assert "does not cover" in str(
+        _user_facing(NotCompleteWillException("utxo"))
+    ).lower()
+
+def test_willexecutors_delete_missing_is_noop():
+    with Plugin() as plugin:
+        c = _make_controller(plugin)
+        assert c.willexecutors_delete(["https://missing.example.invalid"]) == {
+            "deleted": []
+        }
+
+
+def test_willexecutors_select_only_valid():
+    with Plugin() as plugin:
+        c = _make_controller(plugin)
+        valid_url = "https://valid.example.invalid"
+        invalid_url = "https://invalid.example.invalid"
+        # Valid: real address, base_fee within [dust, max_fee].
+        c.willexecutors_add(valid_url, address=VALID_ADDRESS, base_fee=1000)
+        # Invalid: no address.
+        c.willexecutors_add(invalid_url, address="", base_fee=1000)
+
+        c.willexecutors_select(None, select=True, only_valid=True)
+        assert c.willexecutors[valid_url]["selected"] is True
+        assert c.willexecutors[invalid_url]["selected"] is False
+
+        # Select everything, then deselect only-valid: the valid one keeps its
+        # state, only the invalid one is cleared.
+        c.willexecutors_select(None, select=True)
+        c.willexecutors_select(None, select=False, only_valid=True)
+        assert c.willexecutors[valid_url]["selected"] is True
+        assert c.willexecutors[invalid_url]["selected"] is False
+
+
+def test_willexecutors_import_overwrites_existing():
+    import json
+
+    with Plugin() as plugin:
+        c = _make_controller(plugin)
+        url = "https://existing.example.invalid"
+        c.willexecutors_add(url, address="", base_fee=100, info="Old")
+        path = _tmp_path()
+        try:
+            with open(path, "w", encoding="utf-8") as f:
+                json.dump(
+                    {
+                        url: {
+                            "address": "",
+                            "base_fee": 999,
+                            "info": "New",
+                            "selected": True,
+                        }
+                    },
+                    f,
+                )
+            result = c.willexecutors_import(path)
+            assert result["imported"] == 1
+            # The imported record replaces the existing one (GUI behaviour).
+            assert c.willexecutors[url]["base_fee"] == 999
+            assert c.willexecutors[url]["info"] == "New"
+            assert c.willexecutors[url]["selected"] is True
+        finally:
+            os.remove(path)
+
+
+def test_willexecutors_export_subsets():
+    import json
+
+    with Plugin() as plugin:
+        c = _make_controller(plugin)
+        selected_url = "https://selected.example.invalid"
+        other_url = "https://other.example.invalid"
+        invalid_url = "https://invalid.example.invalid"
+        c.willexecutors_add(selected_url, address=VALID_ADDRESS, base_fee=1000)
+        c.willexecutors_add(other_url, address=VALID_ADDRESS, base_fee=1000)
+        c.willexecutors_add(invalid_url, address="", base_fee=1000)
+        c.willexecutors_select([selected_url], select=True)
+
+        def exported(subset):
+            path = _tmp_path()
+            try:
+                c.willexecutors_export(path, subset)
+                with open(path, encoding="utf-8") as f:
+                    return json.load(f)
+            finally:
+                os.remove(path)
+
+        assert set(exported(None)) == {selected_url, other_url, invalid_url}
+        assert set(exported("selected")) == {selected_url}
+        # "valid" keeps both real-address records and drops the address-less one.
+        assert set(exported("valid")) == {selected_url, other_url}
+
+
+def test_user_facing_maps_no_servers_for_chain():
+    from bal.core.willexecutors import Willexecutors
+
+    advanced = Willexecutors.NoServersForChainError(
+        "bitcoin", url="https://welist.example/", reason="boom"
+    )
+    assert "https://welist.example/" in str(_user_facing(advanced))
+    basic = Willexecutors.NoServersForChainError("bitcoin")
+    assert "bitcoin" in str(_user_facing(basic))
+
+
+def test_prepare_will_warns_on_amount_exception():
+    from bal.core import will as will_mod
+
+    with Plugin() as plugin:
+        plugin.NO_WILLEXECUTOR.set(True)
+        c = _make_controller(plugin)
+        c.heirs["alice"] = [VALID_ADDRESS, "100%", "2y"]
+        with mock.patch.object(
+            will_mod.Will,
+            "check_amounts",
+            side_effect=will_mod.FixedAmountException("too much"),
+        ), mock.patch.object(will_mod.Will, "is_will_valid", return_value=True):
+            result = c.prepare_will()
+        # The amount mismatch is a warning, not an abort: the flow completes.
+        assert result["result"] == "coherent"
+
+
 def test_will_status_empty():
    with Plugin() as plugin:
        c = _make_controller(plugin)
--- a/tests/test_gui_will_flows.py
+++ b/tests/test_gui_will_flows.py
@@ -386,7 +386,9 @@ def test_insufficient_funds_warns():

    assert not will, "no wallet funds must not build a will"
    assert ctl.window.warnings, "user must be warned about the balance"
-    assert any("adjustment" in w.lower() for w in ctl.window.warnings)
+    # The wording was shortened (task #67): the "adjustment of the amounts"
+    # sentence was dropped, the "fully emptied" note stays.
+    assert any("fully emptied" in w.lower() for w in ctl.window.warnings)
    assert not ctl.willitems


--- a/tests/test_sync_locktime_built_txs.py
+++ b/tests/test_sync_locktime_built_txs.py
@@ -1,10 +1,11 @@
 """
-Tests for ``BalBuildWillDialog._sync_locktime_to_built_txs``.
+Tests for ``BalBuildWillDialog._sync_locktime_to_built_txs`` and the
+"Building your will" row of ``BalBuildWillDialog._build_success_report``.

-This is the post-build sync that keeps the plugin's stored delivery date
-(WILL_SETTINGS["locktime"]) in lockstep with the BUILT transactions' fixed
-locktime when the core AUTOMATICALLY anticipates it (one day earlier than
-stored).
+``_sync_locktime_to_built_txs`` is the post-build sync that keeps the plugin's
+stored delivery date (WILL_SETTINGS["locktime"]) in lockstep with the BUILT
+transactions' fixed locktime when the core AUTOMATICALLY anticipates it (one
+day earlier than stored).

 RELATIVE recipes ("90d" / "1y") are now PRESERVED: the daily-drift problem
 that once forced freezing them to absolute timestamps is solved at the root by
@@ -14,8 +15,14 @@ anchoring every relative recipe against the built transactions
 timestamp).  Only a genuine automatic anticipation on an ABSOLUTE stored date
 moves the stored value.

-The method is exercised with a lightweight fake ``self`` (no Qt event loop, no
-Electrum wallet) by calling it as an unbound method.
+WHY THE FLAG IS NOT SET HERE ANY MORE: because a relative recipe is anchored to
+the built tx, a stored-vs-built comparison cannot detect the anticipation for
+it.  ``_build_success_report`` therefore detects it by comparing the will's
+minimum locktime BEFORE the rebuild with the one AFTER it (see there), which
+works for both absolute and relative recipes.
+
+The methods are exercised with a lightweight fake ``self`` (no Qt event loop,
+no Electrum wallet) by calling them as unbound methods.

 Run:
    source "$BAL_HOME/electrum/env/bin/activate"
@@ -35,10 +42,12 @@ from bal.gui.qt.dialogs import BalBuildWillDialog  # noqa: E402  (path insert ab
 # ------------------------------------------------------------------ #

 def _fake_willitem(tx_locktime):
-    """Minimal stand-in for a WillItem: enough for Will.get_min_locktime."""
+    """Minimal stand-in for a WillItem: enough for Will.get_min_locktime and
+    the heir listing in _build_success_report."""
    return SimpleNamespace(
        tx=SimpleNamespace(locktime=tx_locktime),
        get_status=lambda name: True,
+        heirs={},
    )


@@ -57,7 +66,6 @@ def _make_dialog(will_settings, tx_locktimes, recorded):
            will_settings=will_settings,
            update_setting_widgets=update_setting_widgets,
        ),
-        _date_was_anticipated=False,
    )


@@ -83,7 +91,6 @@ def test_relative_locktime_preserved():
    assert fake.bal_window.will_settings["locktime"] == "90d"
    assert fake.bal_window.will_settings["threshold"] == "30d"
    assert recorded == [], "a relative recipe must never be rewritten"
-    assert fake._date_was_anticipated is False


 def test_relative_threshold_preserved():
@@ -108,12 +115,13 @@ def test_absolute_locktime_unchanged_on_equal():
        recorded,
    )
    assert fake.bal_window.will_settings["locktime"] == tx_locktime
-    assert fake._date_was_anticipated is False
+    assert fake.bal_window.will_settings["threshold"] == 1_700_000_000


-def test_anticipation_sets_flag_and_moves_earlier():
+def test_anticipation_moves_stored_date_earlier():
    """A real automatic anticipation of an ABSOLUTE stored date (built earlier
-    than stored) still moves the date earlier and flags the sign prompt."""
+    than stored) moves the stored date earlier so the next Check does not
+    mistake it for a postpone."""
    tx_locktime = 1_700_000_000
    recorded = []
    fake = _call_sync(
@@ -122,7 +130,6 @@ def test_anticipation_sets_flag_and_moves_earlier():
        recorded,
    )
    assert fake.bal_window.will_settings["locktime"] == tx_locktime
-    assert fake._date_was_anticipated is True


 def test_stored_earlier_than_built_never_moved_later():
@@ -137,7 +144,6 @@ def test_stored_earlier_than_built_never_moved_later():
        recorded,
    )
    assert fake.bal_window.will_settings["locktime"] == stored
-    assert fake._date_was_anticipated is False


 def test_multiple_txs_uses_minimum_locktime():
@@ -182,6 +188,76 @@ def test_relative_locktime_stays_coherent_via_anchor():
    assert drifted > tx_locktime


+# ------------------------------------------------------------------ #
+# _build_success_report: the "Building your will" row
+# ------------------------------------------------------------------ #
+
+def _make_report_dialog(new_locktimes, previous_min_locktime):
+    """Fake ``self`` for _build_success_report (no Qt event loop).
+
+    ``_sync_locktime_to_built_txs`` is stubbed (it is exercised separately), so
+    the test drives the AFTER-build locktimes and the captured BEFORE-build
+    locktime directly; the message helpers capture what would be written on the
+    "Building your will" row.
+    """
+    captured = {}
+
+    def _set_building(status=None, row=None, color=None):
+        captured["build_status"] = status
+
+    fake = SimpleNamespace(
+        bal_window=SimpleNamespace(
+            willitems={
+                f"tx{i}": _fake_willitem(lt)
+                for i, lt in enumerate(new_locktimes)
+            },
+            wallet=SimpleNamespace(set_label=lambda *a, **k: None),
+        ),
+        _previous_min_locktime=previous_min_locktime,
+        _sync_locktime_to_built_txs=lambda: None,
+        msg_set_building=_set_building,
+        msg_set_status=lambda *a, **k: None,
+        msg_alert=lambda e: "ALERT:{}".format(e),
+        msg_ok=lambda e=None: "OK",
+    )
+    return fake, captured
+
+
+def test_build_report_shows_anticipated_warning():
+    """When the rebuild moved the delivery one day earlier than the previous
+    will, the "Building your will" row shows a warning naming the NEW delivery
+    date instead of a plain "Ok"."""
+    previous = 1_800_000_000  # 2027-01-15 (UTC)
+    rebuilt = previous - 86_400  # anticipated one day earlier
+    fake, captured = _make_report_dialog([rebuilt], previous)
+    BalBuildWillDialog._build_success_report(fake)
+    assert fake._date_was_anticipated is True
+    assert fake._anticipated_locktime == rebuilt
+    assert captured["build_status"] == (
+        "ALERT:Anticipated - new delivery date: 2027-01-14"
+    )
+
+
+def test_build_report_shows_ok_when_not_anticipated():
+    """Without anticipation the row keeps the plain "Ok" and the state stays
+    clean."""
+    previous = 1_800_000_000
+    fake, captured = _make_report_dialog([previous], previous)
+    BalBuildWillDialog._build_success_report(fake)
+    assert fake._date_was_anticipated is False
+    assert fake._anticipated_locktime is None
+    assert captured["build_status"] == "OK"
+
+
+def test_build_report_first_build_is_not_anticipated():
+    """A first build (no previous will) is never flagged as anticipated."""
+    fake, captured = _make_report_dialog([1_800_000_000], None)
+    BalBuildWillDialog._build_success_report(fake)
+    assert fake._date_was_anticipated is False
+    assert fake._anticipated_locktime is None
+    assert captured["build_status"] == "OK"
+
+
 if __name__ == "__main__":
    for name in sorted(dir()):
        if name.startswith("test_"):