Merge pull request #10111 from SomberNight/202508_iface_cache_broadcast_tx

interface: don't request same tx from server that we just broadcast to it

Author: SomberNight

electrum/interface.py

@@ -49,19 +49,22 @@
 
 from .util import (ignore_exceptions, log_exceptions, bfh, ESocksProxy,
                    is_integer, is_non_negative_integer, is_hash256_str, is_hex_str,
-                   is_int_or_float, is_non_negative_int_or_float, OldTaskGroup)
+                   is_int_or_float, is_non_negative_int_or_float, OldTaskGroup,
+                   send_exception_to_crash_reporter, error_text_str_to_safe_str)
 from . import util
 from . import x509
 from . import pem
 from . import version
 from . import blockchain
 from .blockchain import Blockchain, HEADER_SIZE, CHUNK_SIZE
 from . import bitcoin
+from .bitcoin import DummyAddress, DummyAddressUsedInTxException
 from . import constants
 from .i18n import _
 from .logging import Logger
 from .transaction import Transaction
 from .fee_policy import FEE_ETA_TARGETS
+from .lrucache import LRUCache
 
 if TYPE_CHECKING:
     from .network import Network
@@ -279,6 +282,34 @@ class InvalidOptionCombination(Exception): pass
 class ConnectError(NetworkException): pass
 
 
+class TxBroadcastError(NetworkException):
+    def get_message_for_gui(self):
+        raise NotImplementedError()
+
+
+class TxBroadcastHashMismatch(TxBroadcastError):
+    def get_message_for_gui(self):
+        return "{}\n{}\n\n{}" \
+            .format(_("The server returned an unexpected transaction ID when broadcasting the transaction."),
+                    _("Consider trying to connect to a different server, or updating Electrum."),
+                    str(self))
+
+
+class TxBroadcastServerReturnedError(TxBroadcastError):
+    def get_message_for_gui(self):
+        return "{}\n{}\n\n{}" \
+            .format(_("The server returned an error when broadcasting the transaction."),
+                    _("Consider trying to connect to a different server, or updating Electrum."),
+                    str(self))
+
+
+class TxBroadcastUnknownError(TxBroadcastError):
+    def get_message_for_gui(self):
+        return "{}\n{}" \
+            .format(_("Unknown error when broadcasting the transaction."),
+                    _("Consider trying to connect to a different server, or updating Electrum."))
+
+
 class _RSClient(RSClient):
     async def create_connection(self):
         try:
@@ -499,6 +530,7 @@ def __init__(self, *, network: 'Network', server: ServerAddr):
         assert isinstance(server, ServerAddr), f"expected ServerAddr, got {type(server)}"
         self.ready = network.asyncio_loop.create_future()
         self.got_disconnected = asyncio.Event()
+        self._blockchain_updated = asyncio.Event()
         self.server = server
         Logger.__init__(self)
         assert network.config.path
@@ -527,6 +559,7 @@ def __init__(self, *, network: 'Network', server: ServerAddr):
         self.tip = 0
 
         self._headers_cache = {}  # type: Dict[int, bytes]
+        self._rawtx_cache = LRUCache(maxsize=20)  # type: LRUCache[str, bytes]  # txid->rawtx
 
         self.fee_estimates_eta = {}  # type: Dict[int, int]
 
@@ -1028,6 +1061,8 @@ async def run_fetch_blocks(self):
                 self.logger.info(f"new chain tip. {height=}")
             if blockchain_updated:
                 util.trigger_callback('blockchain_updated')
+                self._blockchain_updated.set()
+                self._blockchain_updated.clear()
             util.trigger_callback('network_updated')
             await self.network.switch_unwanted_fork_interface()
             await self.network.switch_lagging_interface()
@@ -1070,6 +1105,8 @@ async def sync_until(
                     continue
                 # report progress to gui/etc
                 util.trigger_callback('blockchain_updated')
+                self._blockchain_updated.set()
+                self._blockchain_updated.clear()
                 util.trigger_callback('network_updated')
                 height += num_headers
                 assert height <= next_height+1, (height, self.tip)
@@ -1283,6 +1320,8 @@ async def get_merkle_for_transaction(self, tx_hash: str, tx_height: int) -> dict
     async def get_transaction(self, tx_hash: str, *, timeout=None) -> str:
         if not is_hash256_str(tx_hash):
             raise Exception(f"{repr(tx_hash)} is not a txid")
+        if rawtx_bytes := self._rawtx_cache.get(tx_hash):
+            return rawtx_bytes.hex()
         raw = await self.session.send_request('blockchain.transaction.get', [tx_hash], timeout=timeout)
         # validate response
         if not is_hex_str(raw):
@@ -1294,8 +1333,40 @@ async def get_transaction(self, tx_hash: str, *, timeout=None) -> str:
             raise RequestCorrupted(f"cannot deserialize received transaction (txid {tx_hash})") from e
         if tx.txid() != tx_hash:
             raise RequestCorrupted(f"received tx does not match expected txid {tx_hash} (got {tx.txid()})")
+        self._rawtx_cache[tx_hash] = bytes.fromhex(raw)
         return raw
 
+    async def broadcast_transaction(self, tx: 'Transaction', *, timeout=None) -> None:
+        """caller should handle TxBroadcastError and RequestTimedOut"""
+        txid_calc = tx.txid()
+        assert txid_calc is not None
+        rawtx = tx.serialize()
+        assert is_hex_str(rawtx)
+        if timeout is None:
+            timeout = self.network.get_network_timeout_seconds(NetworkTimeout.Urgent)
+        if any(DummyAddress.is_dummy_address(txout.address) for txout in tx.outputs()):
+            raise DummyAddressUsedInTxException("tried to broadcast tx with dummy address!")
+        try:
+            out = await self.session.send_request('blockchain.transaction.broadcast', [rawtx], timeout=timeout)
+            # note: both 'out' and exception messages are untrusted input from the server
+        except (RequestTimedOut, asyncio.CancelledError, asyncio.TimeoutError):
+            raise  # pass-through
+        except aiorpcx.jsonrpc.CodeMessageError as e:
+            self.logger.info(f"broadcast_transaction error [DO NOT TRUST THIS MESSAGE]: {error_text_str_to_safe_str(repr(e))}. tx={str(tx)}")
+            raise TxBroadcastServerReturnedError(sanitize_tx_broadcast_response(e.message)) from e
+        except BaseException as e:  # intentional BaseException for sanity!
+            self.logger.info(f"broadcast_transaction error2 [DO NOT TRUST THIS MESSAGE]: {error_text_str_to_safe_str(repr(e))}. tx={str(tx)}")
+            send_exception_to_crash_reporter(e)
+            raise TxBroadcastUnknownError() from e
+        if out != txid_calc:
+            self.logger.info(f"unexpected txid for broadcast_transaction [DO NOT TRUST THIS MESSAGE]: "
+                             f"{error_text_str_to_safe_str(out)} != {txid_calc}. tx={str(tx)}")
+            raise TxBroadcastHashMismatch(_("Server returned unexpected transaction ID."))
+        # broadcast succeeded.
+        # We now cache the rawtx, for *this interface only*. The tx likely touches some ismine addresses, affecting
+        # the status of a scripthash we are subscribed to. Caching here will save a future get_transaction RPC.
+        self._rawtx_cache[txid_calc] = bytes.fromhex(rawtx)
+
     async def get_history_for_scripthash(self, sh: str) -> List[dict]:
         if not is_hash256_str(sh):
             raise Exception(f"{repr(sh)} is not a scripthash")
@@ -1462,6 +1533,190 @@ def _assert_header_does_not_check_against_any_chain(header: dict) -> None:
         raise Exception('bad_header must not check!')
 
 
+def sanitize_tx_broadcast_response(server_msg) -> str:
+    # Unfortunately, bitcoind and hence the Electrum protocol doesn't return a useful error code.
+    # So, we use substring matching to grok the error message.
+    # server_msg is untrusted input so it should not be shown to the user. see #4968
+    server_msg = str(server_msg)
+    server_msg = server_msg.replace("\n", r"\n")
+
+    # https://github.com/bitcoin/bitcoin/blob/5bb64acd9d3ced6e6f95df282a1a0f8b98522cb0/src/script/script_error.cpp
+    script_error_messages = {
+        r"Script evaluated without error but finished with a false/empty top stack element",
+        r"Script failed an OP_VERIFY operation",
+        r"Script failed an OP_EQUALVERIFY operation",
+        r"Script failed an OP_CHECKMULTISIGVERIFY operation",
+        r"Script failed an OP_CHECKSIGVERIFY operation",
+        r"Script failed an OP_NUMEQUALVERIFY operation",
+        r"Script is too big",
+        r"Push value size limit exceeded",
+        r"Operation limit exceeded",
+        r"Stack size limit exceeded",
+        r"Signature count negative or greater than pubkey count",
+        r"Pubkey count negative or limit exceeded",
+        r"Opcode missing or not understood",
+        r"Attempted to use a disabled opcode",
+        r"Operation not valid with the current stack size",
+        r"Operation not valid with the current altstack size",
+        r"OP_RETURN was encountered",
+        r"Invalid OP_IF construction",
+        r"Negative locktime",
+        r"Locktime requirement not satisfied",
+        r"Signature hash type missing or not understood",
+        r"Non-canonical DER signature",
+        r"Data push larger than necessary",
+        r"Only push operators allowed in signatures",
+        r"Non-canonical signature: S value is unnecessarily high",
+        r"Dummy CHECKMULTISIG argument must be zero",
+        r"OP_IF/NOTIF argument must be minimal",
+        r"Signature must be zero for failed CHECK(MULTI)SIG operation",
+        r"NOPx reserved for soft-fork upgrades",
+        r"Witness version reserved for soft-fork upgrades",
+        r"Taproot version reserved for soft-fork upgrades",
+        r"OP_SUCCESSx reserved for soft-fork upgrades",
+        r"Public key version reserved for soft-fork upgrades",
+        r"Public key is neither compressed or uncompressed",
+        r"Stack size must be exactly one after execution",
+        r"Extra items left on stack after execution",
+        r"Witness program has incorrect length",
+        r"Witness program was passed an empty witness",
+        r"Witness program hash mismatch",
+        r"Witness requires empty scriptSig",
+        r"Witness requires only-redeemscript scriptSig",
+        r"Witness provided for non-witness script",
+        r"Using non-compressed keys in segwit",
+        r"Invalid Schnorr signature size",
+        r"Invalid Schnorr signature hash type",
+        r"Invalid Schnorr signature",
+        r"Invalid Taproot control block size",
+        r"Too much signature validation relative to witness weight",
+        r"OP_CHECKMULTISIG(VERIFY) is not available in tapscript",
+        r"OP_IF/NOTIF argument must be minimal in tapscript",
+        r"Using OP_CODESEPARATOR in non-witness script",
+        r"Signature is found in scriptCode",
+    }
+    for substring in script_error_messages:
+        if substring in server_msg:
+            return substring
+    # https://github.com/bitcoin/bitcoin/blob/5bb64acd9d3ced6e6f95df282a1a0f8b98522cb0/src/validation.cpp
+    # grep "REJECT_"
+    # grep "TxValidationResult"
+    # should come after script_error.cpp (due to e.g. "non-mandatory-script-verify-flag")
+    validation_error_messages = {
+        r"coinbase": None,
+        r"tx-size-small": None,
+        r"non-final": None,
+        r"txn-already-in-mempool": None,
+        r"txn-mempool-conflict": None,
+        r"txn-already-known": None,
+        r"non-BIP68-final": None,
+        r"bad-txns-nonstandard-inputs": None,
+        r"bad-witness-nonstandard": None,
+        r"bad-txns-too-many-sigops": None,
+        r"mempool min fee not met":
+            ("mempool min fee not met\n" +
+             _("Your transaction is paying a fee that is so low that the bitcoin node cannot "
+               "fit it into its mempool. The mempool is already full of hundreds of megabytes "
+               "of transactions that all pay higher fees. Try to increase the fee.")),
+        r"min relay fee not met": None,
+        r"absurdly-high-fee": None,
+        r"max-fee-exceeded": None,
+        r"too-long-mempool-chain": None,
+        r"bad-txns-spends-conflicting-tx": None,
+        r"insufficient fee": ("insufficient fee\n" +
+             _("Your transaction is trying to replace another one in the mempool but it "
+               "does not meet the rules to do so. Try to increase the fee.")),
+        r"too many potential replacements": None,
+        r"replacement-adds-unconfirmed": None,
+        r"mempool full": None,
+        r"non-mandatory-script-verify-flag": None,
+        r"mandatory-script-verify-flag-failed": None,
+        r"Transaction check failed": None,
+    }
+    for substring in validation_error_messages:
+        if substring in server_msg:
+            msg = validation_error_messages[substring]
+            return msg if msg else substring
+    # https://github.com/bitcoin/bitcoin/blob/5bb64acd9d3ced6e6f95df282a1a0f8b98522cb0/src/rpc/rawtransaction.cpp
+    # https://github.com/bitcoin/bitcoin/blob/5bb64acd9d3ced6e6f95df282a1a0f8b98522cb0/src/util/error.cpp
+    # https://github.com/bitcoin/bitcoin/blob/3f83c744ac28b700090e15b5dda2260724a56f49/src/common/messages.cpp#L126
+    # grep "RPC_TRANSACTION"
+    # grep "RPC_DESERIALIZATION_ERROR"
+    # grep "TransactionError"
+    rawtransaction_error_messages = {
+        r"Missing inputs": None,
+        r"Inputs missing or spent": None,
+        r"transaction already in block chain": None,
+        r"Transaction already in block chain": None,
+        r"Transaction outputs already in utxo set": None,
+        r"TX decode failed": None,
+        r"Peer-to-peer functionality missing or disabled": None,
+        r"Transaction rejected by AcceptToMemoryPool": None,
+        r"AcceptToMemoryPool failed": None,
+        r"Transaction rejected by mempool": None,
+        r"Mempool internal error": None,
+        r"Fee exceeds maximum configured by user": None,
+        r"Unspendable output exceeds maximum configured by user": None,
+        r"Transaction rejected due to invalid package": None,
+    }
+    for substring in rawtransaction_error_messages:
+        if substring in server_msg:
+            msg = rawtransaction_error_messages[substring]
+            return msg if msg else substring
+    # https://github.com/bitcoin/bitcoin/blob/5bb64acd9d3ced6e6f95df282a1a0f8b98522cb0/src/consensus/tx_verify.cpp
+    # https://github.com/bitcoin/bitcoin/blob/c7ad94428ab6f54661d7a5441e1fdd0ebf034903/src/consensus/tx_check.cpp
+    # grep "REJECT_"
+    # grep "TxValidationResult"
+    tx_verify_error_messages = {
+        r"bad-txns-vin-empty": None,
+        r"bad-txns-vout-empty": None,
+        r"bad-txns-oversize": None,
+        r"bad-txns-vout-negative": None,
+        r"bad-txns-vout-toolarge": None,
+        r"bad-txns-txouttotal-toolarge": None,
+        r"bad-txns-inputs-duplicate": None,
+        r"bad-cb-length": None,
+        r"bad-txns-prevout-null": None,
+        r"bad-txns-inputs-missingorspent":
+            ("bad-txns-inputs-missingorspent\n" +
+             _("You might have a local transaction in your wallet that this transaction "
+               "builds on top. You need to either broadcast or remove the local tx.")),
+        r"bad-txns-premature-spend-of-coinbase": None,
+        r"bad-txns-inputvalues-outofrange": None,
+        r"bad-txns-in-belowout": None,
+        r"bad-txns-fee-outofrange": None,
+    }
+    for substring in tx_verify_error_messages:
+        if substring in server_msg:
+            msg = tx_verify_error_messages[substring]
+            return msg if msg else substring
+    # https://github.com/bitcoin/bitcoin/blob/5bb64acd9d3ced6e6f95df282a1a0f8b98522cb0/src/policy/policy.cpp
+    # grep "reason ="
+    # should come after validation.cpp (due to "tx-size" vs "tx-size-small")
+    # should come after script_error.cpp (due to e.g. "version")
+    policy_error_messages = {
+        r"version": _("Transaction uses non-standard version."),
+        r"tx-size": _("The transaction was rejected because it is too large (in bytes)."),
+        r"scriptsig-size": None,
+        r"scriptsig-not-pushonly": None,
+        r"scriptpubkey":
+            ("scriptpubkey\n" +
+             _("Some of the outputs pay to a non-standard script.")),
+        r"bare-multisig": None,
+        r"dust":
+            (_("Transaction could not be broadcast due to dust outputs.\n"
+               "Some of the outputs are too small in value, probably lower than 1000 satoshis.\n"
+               "Check the units, make sure you haven't confused e.g. mBTC and BTC.")),
+        r"multi-op-return": _("The transaction was rejected because it contains multiple OP_RETURN outputs."),
+    }
+    for substring in policy_error_messages:
+        if substring in server_msg:
+            msg = policy_error_messages[substring]
+            return msg if msg else substring
+    # otherwise:
+    return _("Unknown error")
+
+
 def check_cert(host, cert):
     try:
         b = pem.dePem(cert, 'CERTIFICATE')