From 05002dc0a55da8a73cb4bb12a3a099cf6a809f0b Mon Sep 17 00:00:00 2001 From: bitcoinafterlife Date: Fri, 4 Sep 2026 11:51:45 +0200 Subject: [PATCH] core+gui: name the real cause of a failed build instead of guessing The Building Will report showed a fixed list of three "possible reasons" whenever a build produced nothing, regardless of what actually happened; in a case reproduced from the owner log all three were false and the real cause was not even listed. The "Checking your will" row had the same flaw, showing one sentence ("Found CHANGES to the DATE or the HEIRS") for five situations, including one where it is plainly wrong (funds received). core/heirs.py: record WHY buildTransactions gave up in a new last_build_error attribute (8 reason codes), set at each path that previously returned empty with no explanation, plus a processed_willexecutors counter to tell "every will-executor was skipped" apart from "we tried and failed". Also fix a latent crash in the prepare_transactions handler, which read a no-longer-existing e.heirname attribute and re-raised the resulting AttributeError, masking the real error. gui/qt/dialogs.py: add msg_alert() (amber warning sign, body text in the theme colour, readable in both themes), _build_failure_message() and _check_failure_message() to turn those causes into one precise sentence each, with an honest "cause could not be determined" fallback. Catch BalanceTooLowException, which already carried the figures but fell through to the generic red technical error. No new exception classes were introduced (owner request): the plain NotCompleteWillException cases are told apart structurally, not by text. --- CHANGELOG.md | 85 +++++++++++++ bal/core/heirs.py | 74 +++++++++-- bal/gui/qt/common.py | 1 + bal/gui/qt/dialogs.py | 276 ++++++++++++++++++++++++++++++++++-------- 4 files changed, 377 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 24623fd..a45776b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2908,3 +2908,88 @@ renumber the grid rows. - Full test suite: 438 passed (unchanged). **Outcome:** DONE. + +--- + +## 57. Name the real cause of a failed build instead of guessing + +**Date:** 2026-09-04 + +**Goal (owner request):** the "Building Will" report was hard to read and often +misleading. + +1. The long "could not build the will" block was printed entirely in amber + (`COLOR_WARNING`), which the owner reported as barely legible. +2. Whenever the build produced nothing, the dialog printed a FIXED list of + three "possible reasons" (low balance / dust shares / check-alive later than + the delivery date) regardless of what had actually happened. In a case + reproduced from the owner's log all three were false, and the real cause + (no delivery date left to build) was not even in the list. +3. The "Checking your will" row had the same problem: the single sentence + "Found CHANGES to the DATE or the HEIRS" was shown for five different + situations, including one where it is plainly wrong - funds received, where + neither the date nor the heirs changed. + +**What changed:** + +- `bal/core/heirs.py` + - `Heirs.__init__` / `buildTransactions`: new `last_build_error` attribute + recording WHY a build produced no transaction. Reset at the start of every + build, and set at each path that previously returned empty with no + explanation at all: `NO_HEIRS`, `NO_UTXO`, `NO_WILLEXECUTOR_USABLE`, + `NO_FUTURE_DATE`, `WILLEXECUTOR_FEE`, `WILLEXECUTOR_FEE_TOO_HIGH`, + `TX_BUILD_FAILED`, `WILLEXECUTOR_TX_ERROR`. + - Added a `processed_willexecutors` counter so that "the loop skipped every + will-executor" - which returned silently, with no log line whatsoever - is + told apart from "we tried and the build failed". + - Fixed a latent crash in the `prepare_transactions` exception handler. It + read `e.heirname` in order to auto-deselect the offending will-executor, + but NOTHING in the plugin sets that attribute any more (leftover from an + older exception design), so the lookup itself raised AttributeError and the + inner `except Exception: raise` re-raised THAT, aborting the whole build + with a confusing secondary error instead of the real one. The handler now + records `WILLEXECUTOR_TX_ERROR`, logs the actual exception together with + the will-executor it happened on, and moves on to the next one - which is + what the original code was clearly trying to do. + +- `bal/gui/qt/dialogs.py` + - New `msg_alert()`: an amber warning sign (U+26A0, written as a numeric HTML + entity so the source stays ASCII) followed by text in the theme's default + colour. Colour is what ATTRACTS attention, not what is read, so it is kept + on the sign alone; the message body stays readable and still works under + the dark theme, where a hard-coded black would disappear. + - New `_build_failure_message()`: maps `last_build_error` to ONE specific + sentence. When the code is missing or unrecognised it SAYS the cause could + not be determined and lists what to check, instead of asserting three + guesses as if they were the only possibilities. + - New `_check_failure_message()`: replaces the single "Found CHANGES to the + DATE or the HEIRS" line with seven precise messages, reusing the detail the + exceptions already carry (heir name, will-executor URL, old and new fee + rate). The two plain `NotCompleteWillException` cases are told apart + STRUCTURALLY (raised with no argument vs. with one), not by matching + message text, which would be fragile. No new exception classes were added + (owner request). + - Added a dedicated `except BalanceTooLowException` handler. The exception + already carried the balance, the fees and the dust threshold, but was + falling through to the generic handler, which printed the raw technical + string in red and re-raised. It now shows the real figures. + - "Checking variables" row: `No Heirs` now uses `msg_alert()`. The + "Check Alive Threshold Passed" message deliberately STAYS red + (`COLOR_ERROR`) because it is the more urgent situation (owner request). + +- `bal/gui/qt/common.py` + - Re-export `BalanceTooLowException` from `core.heirs` so the Qt layer can + catch it. + +**Verification:** +- `py_compile` clean on all 44 files of the package. +- The real `msg_alert`, `_build_failure_message` and `_check_failure_message` + were extracted from the source via AST and executed against every reason code + and every exception type, with the exception hierarchy rebuilt from + `will.py`: 9 build cases and 8 check cases all produce the intended text. +- NOT RUN: the official test suite. The machine used for this task (Windows) + has no importable `electrum` module, so `tests/` could not be executed. +- Manually tested by the owner in Electrum 4.8.1: `NO_FUTURE_DATE`, + `WILLEXECUTOR_FEE` and `No Heirs` were all confirmed on screen. + +**Outcome:** DONE. diff --git a/bal/core/heirs.py b/bal/core/heirs.py index 7a169a5..e2d4dad 100644 --- a/bal/core/heirs.py +++ b/bal/core/heirs.py @@ -363,6 +363,10 @@ class Heirs(dict, Logger): Logger.__init__(self) self.db = wallet.db self.wallet = wallet + # Reason code explaining why the last buildTransactions() produced no + # transaction (None when the last build succeeded or never ran). See + # buildTransactions for the list of codes and why they exist. + self.last_build_error = None d = self.db.get("heirs", {}) try: self.update(d) @@ -630,6 +634,20 @@ class Heirs(dict, Logger): def buildTransactions( self, bal_plugin, wallet, tx_fees=None, utxos=None, from_locktime=0 ): + # Reset the diagnostic reason at the start of every build attempt. + # + # WHY: when the build produced nothing, the GUI used to show a fixed + # list of three "possible reasons" (low balance / dust shares / + # check-alive after the delivery date). In practice the real cause is + # often NONE of those three - several code paths below simply return + # an empty result with no explanation at all, so the user was shown + # three guesses that were all wrong. Each such path now records WHY + # it gave up, and BalBuildWillDialog names the actual cause. + # + # Codes: NO_HEIRS, NO_UTXO, NO_WILLEXECUTOR_USABLE, NO_FUTURE_DATE, + # WILLEXECUTOR_FEE, WILLEXECUTOR_FEE_TOO_HIGH, TX_BUILD_FAILED, + # WILLEXECUTOR_TX_ERROR. + self.last_build_error = None _before = list(self.keys()) Heirs._validate(self, persist=False) _removed = [k for k in _before if k not in self] @@ -644,6 +662,7 @@ class Heirs(dict, Logger): ", ".join(_removed), ) if len(self) <= 0: + self.last_build_error = "NO_HEIRS" _logger.info("while building transactions there was no heirs") return balance = 0.0 @@ -660,12 +679,18 @@ class Heirs(dict, Logger): len_utxo_set += 1 available_utxos.append(utxo) if len_utxo_set == 0: + self.last_build_error = "NO_UTXO" _logger.info("no usable utxos") return j = -2 willexecutorsitems = list(willexecutors.items()) willexecutorslen = len(willexecutorsitems) alltxs = {} + # Counts how many will-executors were actually PROCESSED (i.e. passed + # the is_selected/is_valid filter below and reached the build loop). + # If it stays 0 the loop silently skipped every single one, which is a + # distinct failure from "we tried and the build failed". + processed_willexecutors = 0 while True: j += 1 if j >= willexecutorslen: @@ -682,6 +707,7 @@ class Heirs(dict, Logger): url = willexecutor = None else: break + processed_willexecutors += 1 fees = {} i = 0 txs = {} @@ -699,9 +725,11 @@ class Heirs(dict, Logger): max_fee=bal_plugin.MAX_WILLEXECUTOR_FEE.get(), ) except WillExecutorFeeException: + self.last_build_error = "WILLEXECUTOR_FEE" i = 10 continue except WillExecutorFeeTooHighException: + self.last_build_error = "WILLEXECUTOR_FEE_TOO_HIGH" i = 10 continue if locktimes: @@ -710,19 +738,33 @@ class Heirs(dict, Logger): locktimes, available_utxos[:], fees, wallet ) if not txs: + self.last_build_error = "TX_BUILD_FAILED" return {} except Exception as e: + # An unexpected failure while assembling the + # transactions for THIS will-executor. + # + # WHY THIS CHANGED: the previous code read + # ``e.heirname`` here, in order to auto-deselect the + # will-executor blamed by the exception. NOTHING in + # the plugin sets that attribute any more (it is a + # leftover from an older exception design), so the + # lookup itself raised AttributeError, and the inner + # ``except Exception: raise`` re-raised THAT - aborting + # the whole build with a confusing secondary error + # instead of the real one. We now record the reason, + # log the actual exception together with the + # will-executor it happened on, and simply move on to + # the next one, which is what the original code was + # clearly trying to do. + self.last_build_error = "WILLEXECUTOR_TX_ERROR" _logger.error( - f"build transactions: error preparing transactions: {e}" + "build transactions: error preparing transactions " + "for will-executor %s: %r", + (willexecutor or {}).get("url", "(none)"), + e, ) - try: - if "w!ll3x3c" in e.heirname: - Willexecutors.is_selected( - e.heirname[len("w!ll3x3c") :], False - ) - break - except Exception: - raise + break total_fees = 0 total_fees_real = 0 total_in = 0 @@ -746,12 +788,26 @@ class Heirs(dict, Logger): if i >= 10: break else: + self.last_build_error = "NO_FUTURE_DATE" _logger.info( f"no locktimes for willexecutor {willexecutor} skipped" ) break alltxs.update(txs) + # Every will-executor was skipped by the is_selected/is_valid filter + # (or the list was empty) and no "no will-executor" build was allowed, + # so the loop above never even attempted a build. This path used to + # return silently with no log line at all, which is exactly the case + # the owner hit: the dialog then blamed balance/dust/check-alive, none + # of which was true. + if not alltxs and processed_willexecutors == 0: + self.last_build_error = "NO_WILLEXECUTOR_USABLE" + _logger.info( + "no usable will-executor: all %d skipped (not selected or not valid)", + willexecutorslen, + ) + return alltxs def get_transactions( diff --git a/bal/gui/qt/common.py b/bal/gui/qt/common.py index 3ce13ca..ec07f37 100644 --- a/bal/gui/qt/common.py +++ b/bal/gui/qt/common.py @@ -108,6 +108,7 @@ from ...core.heirs import ( HEIR_DUST_AMOUNT, HEIR_REAL_AMOUNT, OP_RETURN_PREFIX, + BalanceTooLowException, HeirAmountIsDustException, Heirs, WillExecutorFeeTooHighException, diff --git a/bal/gui/qt/dialogs.py b/bal/gui/qt/dialogs.py index 23e514d..552c5cb 100644 --- a/bal/gui/qt/dialogs.py +++ b/bal/gui/qt/dialogs.py @@ -28,6 +28,7 @@ from .common import ( AmountException, Any, BalTimestamp, + BalanceTooLowException, BestEffortRequestFailed, Buttons, Callable, @@ -690,12 +691,19 @@ class BalBuildWillDialog(BalDialog): _logger.debug( "during phase1 CAE: {}, Continue to invalidate".format(cae) ) - self.msg_set_status("Checking variables",varrow, "Check Alive Threshold Passed: you have to Invalidate your old Will",self.COLOR_ERROR) + self.msg_set_status( + "Checking variables", varrow, + "Check Alive Threshold Passed: you have to Invalidate " + "your old Will", + self.COLOR_ERROR, + ) else: raise cae return None, tx except NoHeirsException: - self.msg_set_status("Checking variables", varrow,"No Heirs",self.COLOR_ERROR) + self.msg_set_status( + "Checking variables", varrow, self.msg_alert("No Heirs") + ) return "no_heirs", None except Exception as e: raise e @@ -811,61 +819,29 @@ class BalBuildWillDialog(BalDialog): _("Inheritance in mempool (waiting confirmation)"), self.COLOR_WARNING, ) - if isinstance(e, HeirChangeException): - message = _("Heirs changed:") - elif isinstance(e, WillExecutorNotPresent): - message = _("Will-Executor not present") - elif isinstance(e, WillexecutorChangeException): - message = _("Will-Executor changed") - elif isinstance(e, TxFeesChangedException): - message = _("Txfees are changed") - elif isinstance(e, HeirNotFoundException): - # Task #01b: the old text "Heir not found" was misleading. - # In practice this branch is reached whenever the will is no - # longer coherent and must be rebuilt - very often simply - # because the delivery date was anticipated, NOT because an heir - # is genuinely missing. We therefore show a clear, accurate - # message that covers both the DATE and the HEIRS cases. - message = _( - "Found CHANGES to the DATE or the HEIRS,\n" - "a NEW WILL must be prepared." - ) - if message: - _logger.debug(f"message: {message}") - self.msg_set_checking(message) - else: - # Task #01b: the old fallback text "New" was unclear. When the - # will is incomplete without a more specific reason, it still - # means the will has to be rebuilt, so we use the same clear - # message as the HeirNotFoundException branch above. - self.msg_set_checking( - _( - "Found CHANGES to the DATE or the HEIRS,\n" - "a NEW WILL must be prepared." - ) - ) + # All of these situations used to collapse into the SAME sentence + # ("Found CHANGES to the DATE or the HEIRS, a NEW WILL must be + # prepared"), shown for five genuinely different causes - and + # plainly WRONG for the most common one, receiving funds, where + # neither the date nor the heirs changed. _check_failure_message + # names the real cause using the detail each exception already + # carries (heir name, will-executor URL, old/new fee rate). + message = self._check_failure_message(e) + _logger.debug(f"message: {message}") + self.msg_set_checking(message) if have_to_build: self.msg_set_building() try: txs = self.bal_window.build_will() if not txs: + # The message now names the ACTUAL reason the build gave + # up (recorded by Heirs.buildTransactions) instead of + # listing three fixed guesses that were frequently all + # wrong. msg_alert keeps the warning sign coloured and the + # text in the default colour so it stays readable. self.msg_set_building( - _( - "Could not build the will ! Possible reasons:\n" - "1- the Balance of wallet is too low to cover the " - "fees for miners and will executors,\n" - "2- the Heirs' shares are below the minimum (Dust " - "UTXO, less than 546 Satoshi),\n" - "3- the Check Alive Date/Time is later than the " - "delivery time (it must be earlier),\n" - "Skipped" - ), - # Orange (warning) instead of red (error): an empty - # wallet after the inheritance was executed is a NORMAL - # situation, not a failure, so the colour should not - # alarm the user (owner request). - color=self.COLOR_WARNING, + self.msg_alert(self._build_failure_message()) ) return False, None @@ -950,6 +926,28 @@ class BalBuildWillDialog(BalDialog): ) return False, None + except BalanceTooLowException as e: + # The core DOES detect this precisely and carries the numbers, + # but the exception was never caught here: it fell through to + # the generic handler below, which printed the raw technical + # string in red and re-raised. Show the real figures instead. + self.msg_set_building( + self.msg_alert( + _( + "Wallet balance is too low: {} satoshi available, " + "but the miner and will-executor fees need {} " + "satoshi (the minimum usable amount is {} " + "satoshi). Add funds, or select fewer " + "will-executors." + ).format( + int(e.balance), int(e.fees), int(e.dust_threshold) + ) + + "\n\n" + + _("Skipped") + ) + ) + return False, None + except Exception as e: self.msg_set_building(self.msg_error(e)) raise e @@ -2009,6 +2007,168 @@ class BalBuildWillDialog(BalDialog): _logger.debug(f"_executed_inheritance_status error: {_err}") return "MEMPOOL" if has_mempool else None + def _check_failure_message(self, e): + """Return a precise explanation of why the will is no longer coherent. + + ``Will.is_will_valid`` raises NotCompleteWillException (or one of its + subclasses) when the stored will stops matching the wallet, the heirs + or the will-executors. Those exceptions ALREADY carry the useful + detail - the heir name, the will-executor URL, the old and new fee + rate - but this dialog used to discard all of it and print the same + sentence, "Found CHANGES to the DATE or the HEIRS", for every case. + + That was not merely vague, it was WRONG for the most common situation: + when the wallet simply receives new funds, neither the date nor the + heirs changed, yet the user was sent hunting for edits never made. + + NOTE: no new exception classes were added for this (owner request). + Every case below is told apart using only what the core already + raises today. + """ + # Subclasses first - they are all NotCompleteWillException. + if isinstance(e, TxFeesChangedException): + # The core raises TxFeesChangedException(f"{tx_fees}: {w.tx_fees}"), + # i.e. "current: stored". Show both rates when they can be read, + # and fall back to a plain sentence if that format ever changes. + try: + now_fee, old_fee = [p.strip() for p in str(e).split(":", 1)] + return _( + "Miner fee rate changed (the will was built with {} " + "sat/byte, now it is {}): a new will must be prepared." + ).format(old_fee, now_fee) + except Exception: + return _( + "The miner fee rate changed: a new will must be prepared." + ) + if isinstance(e, WillExecutorNotPresent): + return _( + 'Will-executor "{}" is not covered by the current will: ' + "a new will must be prepared." + ).format(str(e)) + if isinstance(e, NoWillExecutorNotPresent): + return _( + "Backup mode is enabled but the will has no backup " + "transaction: a new will must be prepared." + ) + if isinstance(e, HeirNotFoundException): + # Raised when an heir was added, removed, or had its delivery date + # changed. We deliberately do NOT try to tell those three apart + # (owner request: too fine-grained); naming the heir is what makes + # the message actionable. + return _( + 'Heir "{}" is not covered by the current will (it was added ' + "or removed, or its delivery date changed): a new will must " + "be prepared." + ).format(str(e)) + # Kept for completeness: nothing in the plugin raises these two today, + # but they ARE NotCompleteWillException subclasses, so should future + # code raise them they get a sensible message rather than the fallback. + if isinstance(e, HeirChangeException): + return _("The heirs changed: a new will must be prepared.") + if isinstance(e, WillexecutorChangeException): + return _("A will-executor changed: a new will must be prepared.") + + # A plain NotCompleteWillException. The core raises it in exactly two + # places, told apart STRUCTURALLY (not by matching message text, which + # would be fragile): with no argument when the will holds no valid + # transaction, and with one argument when a wallet utxo is not + # included in the will. + if type(e) is NotCompleteWillException: + if e.args: + return _( + "The wallet contains funds that the current will does not " + "cover yet: a new will must be prepared." + ) + return _( + "The will contains no valid transaction: a new will must be " + "prepared." + ) + + # An unrecognised subclass: say so honestly instead of guessing. + return _( + "The will is no longer coherent and must be rebuilt; the exact " + "reason could not be determined." + ) + + def _build_failure_message(self): + """Return a plain-language explanation of why the will was not built. + + ``Heirs.buildTransactions`` records a reason code in + ``last_build_error`` every time it gives up (the codes are listed in + that method). Here we turn that code into ONE specific sentence + telling the user what to fix. + + WHY: this dialog used to print the same three "possible reasons" + (low balance / dust shares / check-alive after the delivery date) + whenever the build returned nothing. The owner hit a real case where + all three were false - the actual cause was that no will-executor was + usable, which the list did not even mention - so the message actively + misled. When the reason is unknown we now SAY that it is unknown and + list what to check, instead of asserting three guesses as if they were + the only possibilities. + """ + reason = None + try: + reason = getattr(self.bal_window.heirs, "last_build_error", None) + except Exception as _err: + # A diagnostic must never break the report it is explaining. + _logger.debug(f"_build_failure_message: {_err}") + + messages = { + "NO_HEIRS": _( + "No heirs: add at least one heir before building the will." + ), + "NO_UTXO": _( + "The wallet has no spendable funds, so no inheritance " + "transaction can be created." + ), + "NO_WILLEXECUTOR_USABLE": _( + "No usable will-executor: none of the servers in the list is " + "both selected and valid. Open the will-executor settings, " + "select at least one server and check that it is reachable." + ), + "NO_FUTURE_DATE": _( + "No delivery date left to build: every heir's date is already " + "covered by the existing will. Choose a later delivery date, " + "or change an heir's date." + ), + "WILLEXECUTOR_FEE": _( + "The amount to send must cover the miner fees plus this " + "will-executor's fee, and the wallet balance is not enough: " + "select cheaper will-executors, or add funds to the wallet." + ), + "WILLEXECUTOR_FEE_TOO_HIGH": _( + "A will-executor asks for more than the maximum fee you " + "allowed: raise the maximum fee in the settings, or select a " + "cheaper will-executor." + ), + "TX_BUILD_FAILED": _( + "The inheritance transactions could not be assembled from the " + "available funds (the balance may not cover the miner fees)." + ), + "WILLEXECUTOR_TX_ERROR": _( + "An unexpected error stopped the transactions being prepared " + "for a will-executor, so it was skipped. Try again, or select " + "a different will-executor." + ), + } + + if reason in messages: + return messages[reason] + "\n\n" + _("Skipped") + + return ( + _( + "Could not build the will, and the exact cause could not be " + "determined. Please check that:\n" + "- the wallet balance covers the miner and will-executor fees,\n" + "- each heir's share is above the minimum (dust limit),\n" + "- the Check Alive date is EARLIER than the delivery date,\n" + "- at least one will-executor is selected and reachable." + ) + + "\n\n" + + _("Skipped") + ) + def msg_set_checking(self, status="Waiting", row=None): row = self.check_row if row is None else row self.check_row = self.msg_set_status(_("Checking your will"), row, status) @@ -2052,6 +2212,22 @@ class BalBuildWillDialog(BalDialog): # Results are shown in bold (see msg_error). return "{}".format(self.COLOR_WARNING, e) + def msg_alert(self, e): + """Amber warning sign followed by text in the theme's default colour. + + WHY: long warnings printed entirely in amber (COLOR_WARNING) are hard + to read - the owner reported the multi-line "could not build the will" + block as barely legible. Colour is only needed to ATTRACT attention, + not to be read, so we keep it on the "warning sign" character alone and + let the message body inherit Electrum's normal text colour. That also + keeps it readable under the dark theme, where a hard-coded black would + disappear. U+26A0 is written as a numeric HTML entity so the source + file stays pure ASCII; QLabel renders it as rich text. + """ + return "⚠ {}".format( + self.COLOR_WARNING, e + ) + def msg_set_status(self, msg, row=None, status=None, color=None): # The left "state" label keeps its normal weight; only the right-side # result (``status``) is rendered in bold so it is easy to read at a