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