Feature: Add "Abandon transaction" option for low fee txs that have been purged from the mempool (#991)

* Initial commit

* configuring Black excludes

* fixing black excludes

* Interim commit

Mempool tx purge is working!

* Server side implementation and front end UI flow

* Remove debugging

* Updated docs, more comments

* Reverting datadir changes

No longer necessary to support final test case.

* final cleanup

* Improved guidance shown with the "Abandon transaction" button

* Optimize the is_tx_purged call

* spelling fix

* Test bugfixes

* remove assumption that we're running with `--docker` and already have 100 blocks mined.
* Pass `rpcport` into BitcoindPlainController so it can differentiate between the default port and the one-off instance used in the test.

* Test suite improvements

* Implement stop_bitcoind for BitcoindPlainController.
* The first `test_wallet_manager` tests were behaving differently between local bitcoind vs docker. Updated to instantiate their own bitcoind just for the two tests.

* Fixing merge

* Removing debugging

DEVELOPMENT.md

@@ -53,26 +53,40 @@ cd specter-desktop
 python3 -m cryptoadvance.specter server --config DevelopmentConfig
 ```
 
-## Howto run the tests
-Run the tests (still very limited):
+## How to run the tests
+_TODO: Need more thorough tests!_
 
+Set up the dependencies:
 ```sh
 pip3 install -r test_requirements.txt
 pip3 install -e .
+```
 
-# needs a bitcoind on your path
+If you have a local bitcoind already installed:
+```
+# Run all the tests
 pytest 
+```
 
-# needs a working docker-setup (but not bitcoind)
-# prerequsisite: 
-# docker pull registry.gitlab.com/cryptoadvance/specter-desktop/python-bitcoind:v0.20.1
+OR run against bitcoind in Docker:
+```
+# Pull the bitcoind image if you haven't already:
+docker pull registry.gitlab.com/cryptoadvance/specter-desktop/python-bitcoind:v0.20.1
+
+# Run all the tests against the docker bitcoind image
 pytest --docker 
+```
 
-# Run all the tests in a specific test-file
-pytest tests/test_specter
+Running specific test subsets:
+```
+# Run all the tests in a specific test file
+pytest tests/test_specter.py
 
 # Run all tests in a specific file matching "Manager"
-pytest tests/test_specter -k Manager 
+pytest tests/test_specter.py -k Manager 
+
+# Run a specific test
+pytest tests/test_specter.py::test_specter
 ```
 
 Check the cypress-section on how to run cypress-frontend-tests.
@@ -139,9 +153,10 @@ If Someone could figure out a better way to do that avoiding this strange this .
 Developing against a bitcoind-API makes most sense with the [Regtest Mode](https://bitcoin.org/en/developer-examples#regtest-mode). Depending on preferences and usecases, there are three major ways on how this dependency can be fullfilled:
 * Easiest way via Docker
 * The unittests on Travis-CI are using a script which is installing and compiling bitcoind
-* bitcoind is manually started (out of scope for this document)
+* Manually run local bitcoind in Regtest
 
-In order to make the "docker-way" even easier, there is a python-script which detects a running-docker-bitcoind and/or is booting one up. Use it like this:
+### Automatically mine and deposit test coins
+In order to make the "docker-way" even easier, there is a python-script which detects a running-docker-bitcoind and/or is boots one up. Use it like this:
 
 ```
 python3 -m cryptoadvance.specter bitcoind
@@ -159,6 +174,42 @@ After that, you can configure the bitcoin-core-connection in specter-desktop lik
 * Host: localhost
 * Port: 18443
 
+### Manually mine and deposit test coins
+If you're not using the integrated Docker method above, start your local bitcoind in regtest mode:
+```
+bitcoind -regtest -fallbackfee=0.0001
+```
+
+In another terminal initialize a default wallet to mine to:
+```
+bitcoin-cli -regtest createwallet satoshiswallet
+```
+
+Get a new address to deposit newly mined coins:
+```
+bitcoin-cli -regtest getnewaddress
+```
+
+Mine coins to the new address
+```
+bitcoin-cli -regtest generatetoaddress 101 <address>
+```
+
+Create a wallet in Specter and send test coins to a receive addr for the new wallet
+```
+bitcoin-cli -regtest sendtoaddress <address> <amount>
+```
+
+Mine the next block when you want a pending tx to be confirmed
+```
+bitcoin-cli -regtest generatetoaddress 1 <address>
+```
+
+Cleanup: Stop your local regtest instance
+```
+bitcoin-cli -regtest stop
+```
+
 ## IDE-specific Configuration (might be outdated)
 
 ### Visual Studio Code

src/cryptoadvance/specter/bitcoind.py

@@ -83,7 +83,9 @@ class BitcoindController:
     def __init__(self, rpcport=18443):
         self.rpcconn = Btcd_conn(rpcport=rpcport)
 
-    def start_bitcoind(self, cleanup_at_exit=False, cleanup_hard=False, datadir=None):
+    def start_bitcoind(
+        self, cleanup_at_exit=False, cleanup_hard=False, datadir=None, extra_args=[]
+    ):
         """starts bitcoind with a specific rpcport=18543 by default.
         That's not the standard in order to make pytest running while
         developing locally against a different regtest-instance
@@ -94,7 +96,10 @@ class BitcoindController:
 
         logger.debug("Starting bitcoind")
         self._start_bitcoind(
-            cleanup_at_exit, cleanup_hard=cleanup_hard, datadir=datadir
+            cleanup_at_exit,
+            cleanup_hard=cleanup_hard,
+            datadir=datadir,
+            extra_args=extra_args,
         )
 
         self.wait_for_bitcoind(self.rpcconn)
@@ -111,7 +116,9 @@ class BitcoindController:
         """ wrapper for convenience """
         return self.rpcconn.get_rpc()
 
-    def _start_bitcoind(self, cleanup_at_exit, cleanup_hard=False):
+    def _start_bitcoind(
+        self, cleanup_at_exit, cleanup_hard=False, datadir=None, extra_args=[]
+    ):
         raise Exception("This should not be used in the baseclass!")
 
     def check_existing(self):
@@ -185,7 +192,12 @@ class BitcoindController:
 
     @classmethod
     def construct_bitcoind_cmd(
-        cls, rpcconn, run_docker=True, datadir=None, bitcoind_path="bitcoind"
+        cls,
+        rpcconn,
+        run_docker=True,
+        datadir=None,
+        bitcoind_path="bitcoind",
+        extra_args=[],
     ):
         """ returns a bitcoind-command to run bitcoind """
         btcd_cmd = "{} ".format(bitcoind_path)
@@ -203,6 +215,8 @@ class BitcoindController:
             if datadir == None:
                 datadir = tempfile.mkdtemp(prefix="bitcoind_datadir")
             btcd_cmd += " -datadir={} ".format(datadir)
+        if extra_args:
+            btcd_cmd += " {}".format(" ".join(extra_args))
         logger.debug("constructed bitcoind-command: %s", btcd_cmd)
         return btcd_cmd
 
@@ -215,14 +229,18 @@ class BitcoindPlainController(BitcoindController):
         self.bitcoind_path = bitcoind_path
         self.rpcconn.ipaddress = "localhost"
 
-    def _start_bitcoind(self, cleanup_at_exit=True, cleanup_hard=False, datadir=None):
+    def _start_bitcoind(
+        self, cleanup_at_exit=True, cleanup_hard=False, datadir=None, extra_args=[]
+    ):
         if datadir == None:
             datadir = tempfile.mkdtemp(prefix="specter_btc_regtest_plain_datadir_")
+
         bitcoind_cmd = self.construct_bitcoind_cmd(
             self.rpcconn,
             run_docker=False,
             datadir=datadir,
             bitcoind_path=self.bitcoind_path,
+            extra_args=extra_args,
         )
         logger.debug("About to execute: {}".format(bitcoind_cmd))
         # exec will prevent creating a child-process and will make bitcoind_proc.terminate() work as expected
@@ -231,40 +249,38 @@ class BitcoindPlainController(BitcoindController):
             "Running bitcoind-process with pid {}".format(self.bitcoind_proc.pid)
         )
 
-        def cleanup_bitcoind(*args):
-            timeout = 50  # in secs
-            if cleanup_hard:
-                self.bitcoind_proc.kill()  # might be usefull for e.g. testing. We can't wait for so long
-                logger.info(
-                    f"Killed bitcoind with pid {self.bitcoind_proc.pid}, Removing {datadir}"
-                )
-                shutil.rmtree(datadir, ignore_errors=True)
-            else:
-                self.bitcoind_proc.terminate()  # might take a bit longer than kill but it'll preserve block-height
-                logger.info(
-                    f"Terminated bitcoind with pid {self.bitcoind_proc.pid}, waiting for termination (timeout {timeout} secs)..."
-                )
-                # self.bitcoind_proc.wait() # doesn't have a timeout
-                procs = psutil.Process().children()
-                for p in procs:
-                    p.terminate()
-                _, alive = psutil.wait_procs(procs, timeout=timeout)
-                for p in alive:
-                    logger.info("bitcoind did not terminated in time, killing!")
-                    p.kill()
-
         if cleanup_at_exit:
             logger.debug("Register function cleanup_bitcoind for SIGINT and SIGTERM")
             # atexit.register(cleanup_bitcoind)
             # This is for CTRL-C --> SIGINT
-            signal.signal(signal.SIGINT, cleanup_bitcoind)
+            signal.signal(signal.SIGINT, self.cleanup_bitcoind)
             # This is for kill $pid --> SIGTERM
-            signal.signal(signal.SIGTERM, cleanup_bitcoind)
+            signal.signal(signal.SIGTERM, self.cleanup_bitcoind)
+
+    def cleanup_bitcoind(self, cleanup_hard=None, datadir=None):
+        timeout = 50  # in secs
+        if cleanup_hard:
+            self.bitcoind_proc.kill()  # might be usefull for e.g. testing. We can't wait for so long
+            logger.info(
+                f"Killed bitcoind with pid {self.bitcoind_proc.pid}, Removing {datadir}"
+            )
+            shutil.rmtree(datadir, ignore_errors=True)
+        else:
+            self.bitcoind_proc.terminate()  # might take a bit longer than kill but it'll preserve block-height
+            logger.info(
+                f"Terminated bitcoind with pid {self.bitcoind_proc.pid}, waiting for termination (timeout {timeout} secs)..."
+            )
+            # self.bitcoind_proc.wait() # doesn't have a timeout
+            procs = psutil.Process().children()
+            for p in procs:
+                p.terminate()
+            _, alive = psutil.wait_procs(procs, timeout=timeout)
+            for p in alive:
+                logger.info("bitcoind did not terminated in time, killing!")
+                p.kill()
 
     def stop_bitcoind(self):
-        # not necessary as the cleanup_bitcoind() will do it automatically!
-        # ToDo: Implement it nevertheless
-        pass
+        self.cleanup_bitcoind()
 
     def check_existing(self):
         """other then in docker, we won't check on the "instance-level". This will return true if if a
@@ -290,11 +306,13 @@ class BitcoindDockerController(BitcoindController):
             btcd_container.stop()
             btcd_container.remove()
 
-    def _start_bitcoind(self, cleanup_at_exit, cleanup_hard=False, datadir=None):
+    def _start_bitcoind(
+        self, cleanup_at_exit, cleanup_hard=False, datadir=None, extra_args=[]
+    ):
         if datadir != None:
             # ignored
             pass
-        bitcoind_path = self.construct_bitcoind_cmd(self.rpcconn)
+        bitcoind_path = self.construct_bitcoind_cmd(self.rpcconn, extra_args=extra_args)
         dclient = docker.from_env()
         logger.debug("Running (in docker): {}".format(bitcoind_path))
         ports = {

src/cryptoadvance/specter/devices/bitcoin_core.py

@@ -144,7 +144,7 @@ class BitcoinCore(Device):
     def create_psbts(self, base64_psbt, wallet):
         return {"core": base64_psbt}
 
-    def sign_psbt(self, base64_psbt, wallet, file_password):
+    def sign_psbt(self, base64_psbt, wallet, file_password=None):
         # Load the wallet if not loaded
         self._load_wallet(wallet.manager)
         rpc = wallet.manager.rpc.wallet(
@@ -161,6 +161,19 @@ class BitcoinCore(Device):
             rpc.walletlock()
         return signed_psbt
 
+    def sign_raw_tx(self, raw_tx, wallet, file_password=None):
+        # Load the wallet if not loaded
+        self._load_wallet(wallet.manager)
+        rpc = wallet.manager.rpc.wallet(
+            os.path.join(wallet.manager.rpc_path + "_hotstorage", self.alias)
+        )
+        if file_password:
+            rpc.walletpassphrase(file_password, 60)
+        signed_tx = rpc.signrawtransactionwithwallet(raw_tx)
+        if file_password:
+            rpc.walletlock()
+        return signed_tx
+
     def delete(
         self, wallet_manager, bitcoin_datadir=get_default_datadir(), chain="main"
     ):

src/cryptoadvance/specter/server_endpoints/wallets.py

@@ -474,6 +474,12 @@ def history(wallet_alias):
         if action == "freezeutxo":
             wallet.toggle_freeze_utxo(request.form.getlist("selected_utxo"))
             tx_list_type = "utxo"
+        elif action == "abandon_tx":
+            txid = request.form["txid"]
+            try:
+                wallet.abandontransaction(txid)
+            except SpecterError as e:
+                flash(str(e), "error")
 
     # update balances in the wallet
     app.specter.check_blockheight()
@@ -1046,6 +1052,10 @@ def decoderawtx(wallet_alias):
             if "blockhash" in tx and "blockheight" not in tx:
                 tx["blockheight"] = wallet.rpc.getblockheader(tx["blockhash"])["height"]
             ##################### Remove until here after dropping Core v0.19 support #####################
+
+            if tx["confirmations"] == 0:
+                tx["is_purged"] = wallet.is_tx_purged(txid)
+
             return {
                 "success": True,
                 "tx": tx,

src/cryptoadvance/specter/templates/includes/tx-data.html

@@ -5,12 +5,12 @@
             text-align: left;
         }
         .tx_info {
-			padding: 1em;
-			border: 1px solid #506072;
-			border-radius: 4px;
-			text-align: center;
-			margin-bottom: 2em;
-		}
+            padding: 1em;
+            border: 1px solid #506072;
+            border-radius: 4px;
+            text-align: center;
+            margin-bottom: 2em;
+        }
     </style>
     <div class="tx-data">
         <h2>Transaction details</h2><br>
@@ -76,7 +76,26 @@
             let walletName = jsonResponse.walletName;
             let tx = jsonResponse.tx;
             let walletLink = `{{ url_for('wallets_endpoint.wallet', wallet_alias='WALLET_ALIAS') }}`.replace("WALLET_ALIAS", self.wallet);
-            let rawtxHTML = `
+            let rawtxHTML = "";
+            if(tx.is_purged) {
+                rawtxHTML += `
+                <div class="warning">
+                    <p><b>This transaction (aka "tx") could not be found in your node's mempool!</b></p>
+                    <p style="text-align:left;">If the bitcoin network is very busy, nodes will start purging the pending txs with the lowest fees. Txs older than two weeks are also purged.</p>
+                    <p style="text-align:left;">A purged tx will never complete. Abandoning this tx will clear it from your wallet, making those funds spendable once again.</p>
+                    <p style="text-align:left;">But first verify that other nodes have also purged your tx! Enter the tx id into a public block explorer (warning: slight privacy leak). If the tx cannot be found, it is safe to abandon this tx.</p>
+
+                    <br/><br/>
+                    <form method="POST" action="${walletLink}history/">
+                        <input type="hidden" class="csrf-token" name="csrf_token" value="{{ csrf_token() }}"/>
+                        <input type="hidden" name="action" value="abandon_tx"/>
+                        <input type="hidden" name="txid" value="${self.txid}"/>
+                        <button type="submit" name="action" value="abandontx" class="btn danger centered" style="max-width: 160px;">Abandon transaction</button>
+                    </form>
+                </div>
+                `;
+            }
+            rawtxHTML += `
             <table class="tx-data-table">
             <tbody>
             <tr><td><span class="optional">Transaction id:</span><span class="mobile-only">TxID:</span></td><td style="word-break: break-all;"><explorer-link data-type="tx" data-value="${self.txid}"></explorer-link></td></tr>

src/cryptoadvance/specter/wallet.py

@@ -258,7 +258,7 @@ class Wallet:
                 obj = self.rpc.listsinceblock()
         txs = obj["transactions"]
         last_block = obj["lastblock"]
-        addresses = [tx["address"] for tx in txs]
+        addresses = [tx["address"] for tx in txs if "address" in tx]
         # remove duplicates
         addresses = list(dict.fromkeys(addresses))
         max_recv = self.address_index - 1
@@ -713,6 +713,30 @@ class Wallet:
         except Exception as e:
             logger.warning("Could not get transaction {}, error: {}".format(txid, e))
 
+    def is_tx_purged(self, txid):
+        # Is tx unconfirmed and no longer in the mempool?
+        try:
+            tx = self.rpc.gettransaction(txid)
+
+            # Do this quick test first to avoid the costlier rpc call
+            if tx["confirmations"] > 0:
+                return False
+
+            return txid not in self.rpc.getrawmempool()
+        except Exception as e:
+            logger.warning("Could not check is_tx_purged {}, error: {}".format(txid, e))
+
+    def abandontransaction(self, txid):
+        # Sanity checks: tx must be unconfirmed and cannot be in the mempool
+        tx = self.rpc.gettransaction(txid)
+        if tx["confirmations"] != 0:
+            raise SpecterError("Cannot abandon a transaction that has a confirmation.")
+        elif txid in self.rpc.getrawmempool():
+            raise SpecterError(
+                "Cannot abandon a transaction that is still in the mempool."
+            )
+        self.rpc.abandontransaction(txid)
+
     def rescanutxo(self, explorer=None, requests_session=None, only_tor=False):
         delete_file(self._transactions.path)
         self.fetch_transactions()

tests/bitcoin_core/README.md

@@ -0,0 +1,10 @@
+These files have been directly copied from bitcoin core's `test` directory in order to test against more complicated node conditions.
+
+* messages.py:
+    * Copied as-is. There are quite a number of classes and utility functions in here and it was more straightforward to leave them all intact.
+    * `siphash` and `util` imports edited to relative imports so they'll work within the context of Specter's test runner.
+* siphash.py:
+    * Copied as-is due to its simplicity.
+* util.py:
+    * Heavily stripped down to its bare minimum.
+    * Edits to `create_lots_of_big_transactions` to be compatible with Specter's test suite node handling.

tests/bitcoin_core/test/functional/test_framework/siphash.py

@@ -0,0 +1,66 @@
+#!/usr/bin/env python3
+# Copyright (c) 2016-2018 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Specialized SipHash-2-4 implementations.
+
+This implements SipHash-2-4 for 256-bit integers.
+"""
+
+
+def rotl64(n, b):
+    return n >> (64 - b) | (n & ((1 << (64 - b)) - 1)) << b
+
+
+def siphash_round(v0, v1, v2, v3):
+    v0 = (v0 + v1) & ((1 << 64) - 1)
+    v1 = rotl64(v1, 13)
+    v1 ^= v0
+    v0 = rotl64(v0, 32)
+    v2 = (v2 + v3) & ((1 << 64) - 1)
+    v3 = rotl64(v3, 16)
+    v3 ^= v2
+    v0 = (v0 + v3) & ((1 << 64) - 1)
+    v3 = rotl64(v3, 21)
+    v3 ^= v0
+    v2 = (v2 + v1) & ((1 << 64) - 1)
+    v1 = rotl64(v1, 17)
+    v1 ^= v2
+    v2 = rotl64(v2, 32)
+    return (v0, v1, v2, v3)
+
+
+def siphash256(k0, k1, h):
+    n0 = h & ((1 << 64) - 1)
+    n1 = (h >> 64) & ((1 << 64) - 1)
+    n2 = (h >> 128) & ((1 << 64) - 1)
+    n3 = (h >> 192) & ((1 << 64) - 1)
+    v0 = 0x736F6D6570736575 ^ k0
+    v1 = 0x646F72616E646F6D ^ k1
+    v2 = 0x6C7967656E657261 ^ k0
+    v3 = 0x7465646279746573 ^ k1 ^ n0
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0 ^= n0
+    v3 ^= n1
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0 ^= n1
+    v3 ^= n2
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0 ^= n2
+    v3 ^= n3
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0 ^= n3
+    v3 ^= 0x2000000000000000
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0 ^= 0x2000000000000000
+    v2 ^= 0xFF
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    v0, v1, v2, v3 = siphash_round(v0, v1, v2, v3)
+    return v0 ^ v1 ^ v2 ^ v3

tests/bitcoin_core/test/functional/test_framework/util.py

@@ -0,0 +1,77 @@
+#!/usr/bin/env python3
+# Copyright (c) 2014-2020 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Helpful routines for regression testing."""
+
+"""
+HEAVILY trimmed down to bare minimum for Specter test cases
+* create_lots_of_big_transactions edited for Specter compatibility
+"""
+
+
+from binascii import unhexlify
+from decimal import Decimal, ROUND_DOWN
+from io import BytesIO
+
+
+def assert_equal(thing1, thing2, *args):
+    if thing1 != thing2 or any(thing1 != arg for arg in args):
+        raise AssertionError(
+            "not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)
+        )
+
+
+def hex_str_to_bytes(hex_str):
+    return unhexlify(hex_str.encode("ascii"))
+
+
+def satoshi_round(amount):
+    return Decimal(amount).quantize(Decimal("0.00000001"), rounding=ROUND_DOWN)
+
+
+# Create large OP_RETURN txouts that can be appended to a transaction
+# to make it large (helper for constructing large transactions).
+def gen_return_txouts():
+    # Some pre-processing to create a bunch of OP_RETURN txouts to insert into transactions we create
+    # So we have big transactions (and therefore can't fit very many into each block)
+    # create one script_pubkey
+    script_pubkey = "6a4d0200"  # OP_RETURN OP_PUSH2 512 bytes
+    for _ in range(512):
+        script_pubkey = script_pubkey + "01"
+    # concatenate 128 txouts of above script_pubkey which we'll insert before the txout for change
+    txouts = []
+    from .messages import CTxOut
+
+    txout = CTxOut()
+    txout.nValue = 0
+    txout.scriptPubKey = hex_str_to_bytes(script_pubkey)
+    for _ in range(128):
+        txouts.append(txout)
+    return txouts
+
+
+# Create a spend of each passed-in utxo, splicing in "txouts" to each raw
+# transaction to make it large.  See gen_return_txouts() above.
+def create_lots_of_big_transactions(wallet, txouts, utxos, num, fee):
+    node = wallet.rpc
+    addr = node.getnewaddress()
+    txids = []
+    from .messages import CTransaction
+
+    for _ in range(num):
+        t = utxos.pop()
+        inputs = [{"txid": t["txid"], "vout": t["vout"]}]
+        outputs = {}
+        change = t["amount"] - fee
+        outputs[addr] = float(satoshi_round(change))
+        rawtx = node.createrawtransaction(inputs, outputs)
+        tx = CTransaction()
+        tx.deserialize(BytesIO(hex_str_to_bytes(rawtx)))
+        for txout in txouts:
+            tx.vout.append(txout)
+        newtx = tx.serialize().hex()
+        signresult = wallet.devices[0].sign_raw_tx(newtx, wallet)
+        txid = node.sendrawtransaction(signresult["hex"], 0)
+        txids.append(txid)
+    return txids

tests/conftest.py

@@ -44,24 +44,25 @@ def pytest_generate_tests(metafunc):
             metafunc.parametrize("docker", [False], scope="module")
 
 
-@pytest.fixture(scope="module")
-def bitcoin_regtest(docker, request):
+def instantiate_bitcoind_controller(docker, request, rpcport=18543, extra_args=[]):
     # logging.getLogger().setLevel(logging.DEBUG)
     requested_version = request.config.getoption("--bitcoind-version")
     if docker:
         bitcoind_controller = BitcoindDockerController(
-            rpcport=18543, docker_tag=requested_version
+            rpcport=rpcport, docker_tag=requested_version
         )
     else:
         if os.path.isfile("tests/bitcoin/src/bitcoind"):
             bitcoind_controller = BitcoindPlainController(
-                bitcoind_path="tests/bitcoin/src/bitcoind"
+                bitcoind_path="tests/bitcoin/src/bitcoind", rpcport=rpcport
             )  # always prefer the self-compiled bitcoind if existing
         else:
-            bitcoind_controller = (
-                BitcoindPlainController()
+            bitcoind_controller = BitcoindPlainController(
+                rpcport=rpcport
             )  # Alternatively take the one on the path for now
-    bitcoind_controller.start_bitcoind(cleanup_at_exit=True, cleanup_hard=True)
+    bitcoind_controller.start_bitcoind(
+        cleanup_at_exit=True, cleanup_hard=True, extra_args=extra_args
+    )
     running_version = bitcoind_controller.version()
     requested_version = request.config.getoption("--bitcoind-version")
     assert (
@@ -72,6 +73,11 @@ def bitcoin_regtest(docker, request):
     return bitcoind_controller
 
 
+@pytest.fixture(scope="module")
+def bitcoin_regtest(docker, request):
+    return instantiate_bitcoind_controller(docker, request, extra_args=None)
+
+
 @pytest.fixture
 def empty_data_folder():
     # Make sure that this folder never ever gets a reasonable non-testing use-case

tests/test_bitcoind.py

@@ -7,6 +7,8 @@ from cryptoadvance.specter.bitcoind import fetch_wallet_addresses_for_mining
 
 
 def test_bitcoinddocker_running(caplog, docker, request):
+    # TODO: Refactor this to use conftest.instantiate_bitcoind_controller
+    # to reduce redundant code?
     caplog.set_level(logging.INFO)
     caplog.set_level(logging.DEBUG, logger="cryptoadvance.specter")
     requested_version = request.config.getoption("--bitcoind-version")

tests/test_specter.py

@@ -1,7 +1,11 @@
 import json, logging, pytest
-from cryptoadvance.specter.specter import get_rpc, Specter
-from cryptoadvance.specter.helpers import alias
+from decimal import Decimal
+from cryptoadvance.specter.helpers import alias, generate_mnemonic
+from cryptoadvance.specter.key import Key
 from cryptoadvance.specter.rpc import BitcoinRPC
+from cryptoadvance.specter.specter import get_rpc, Specter
+from cryptoadvance.specter.specter_error import SpecterError
+from cryptoadvance.specter.wallet_manager import WalletManager
 
 
 def test_alias():
@@ -39,3 +43,176 @@ def test_specter(specter_regtest_configured, caplog):
     # that might only work if your chain is fresh
     # assert json_return['blocks'] == 100
     assert json_return["chain"] == "regtest"
+
+
+def test_abandon_purged_tx(
+    caplog, docker, request, devices_filled_data_folder, device_manager
+):
+    # Specter should support calling abandontransaction if a pending tx has been purged
+    # from the mempool. Test starts a new bitcoind with a restricted mempool to make it
+    # easier to spam the mempool and purge our target tx.
+    # TODO: Similar test but for maxmempoolexpiry?
+
+    # Copied and adapted from:
+    #    https://github.com/bitcoin/bitcoin/blob/master/test/functional/mempool_limit.py
+    from bitcoin_core.test.functional.test_framework.util import (
+        gen_return_txouts,
+        satoshi_round,
+        create_lots_of_big_transactions,
+    )
+    from conftest import instantiate_bitcoind_controller
+
+    caplog.set_level(logging.DEBUG)
+
+    # ==== Specter-specific: do custom setup ====
+    # Instantiate a new bitcoind w/limited mempool. Use a different port to not interfere
+    # with existing instance for other tests.
+    bitcoind_controller = instantiate_bitcoind_controller(
+        docker,
+        request,
+        rpcport=18998,
+        extra_args=["-acceptnonstdtxn=1", "-maxmempool=5", "-spendzeroconfchange=0"],
+    )
+    rpcconn = bitcoind_controller.rpcconn
+    rpc = rpcconn.get_rpc()
+    assert rpc is not None
+    assert rpc.ipaddress != None
+
+    # Note: Our utxo creation is simpler than mempool_limit.py's approach since we're
+    # running in regtest and can just use generatetoaddress().
+
+    # Instantiate a new Specter instance to talk to this bitcoind
+    config = {
+        "rpc": {
+            "autodetect": False,
+            "user": rpcconn.rpcuser,
+            "password": rpcconn.rpcpassword,
+            "port": rpcconn.rpcport,
+            "host": rpcconn.ipaddress,
+            "protocol": "http",
+        },
+        "auth": {
+            "method": "rpcpasswordaspin",
+        },
+    }
+    specter = Specter(data_folder=devices_filled_data_folder, config=config)
+    specter.check()
+
+    specter.check_node_info()
+    assert specter._info["mempool_info"]["maxmempool"] == 5 * 1000 * 1000  # 5MB
+
+    # Largely copy-and-paste from test_wallet_manager.test_wallet_createpsbt.
+    # TODO: Make a test fixture in conftest.py that sets up already funded wallets
+    # for a bitcoin core hot wallet.
+    wallet_manager = WalletManager(
+        200100,
+        devices_filled_data_folder,
+        rpc,
+        "regtest",
+        device_manager,
+    )
+
+    # Create a new device that can sign psbts (Bitcoin Core hot wallet)
+    device = device_manager.add_device(
+        name="bitcoin_core_hot_wallet", device_type="bitcoincore", keys=[]
+    )
+    device.setup_device(file_password=None, wallet_manager=wallet_manager)
+    device.add_hot_wallet_keys(
+        mnemonic=generate_mnemonic(strength=128),
+        passphrase="",
+        paths=["m/49h/0h/0h"],
+        file_password=None,
+        wallet_manager=wallet_manager,
+        testnet=True,
+        keys_range=[0, 1000],
+        keys_purposes=[],
+    )
+
+    wallet = wallet_manager.create_wallet(
+        "bitcoincore_test_wallet", 1, "sh-wpkh", [device.keys[0]], [device]
+    )
+
+    # Fund the wallet. Going to need a LOT of utxos to play with.
+    logging.info("Generating utxos to wallet")
+    address = wallet.getnewaddress()
+    wallet.rpc.generatetoaddress(91, address)
+
+    # newly minted coins need 100 blocks to get spendable
+    # let's mine another 100 blocks to get these coins spendable
+    wallet.rpc.generatetoaddress(101, address)
+
+    # update the wallet data
+    wallet.get_balance()
+
+    # ==== Begin test from mempool_limit.py ====
+    txouts = gen_return_txouts()
+    relayfee = satoshi_round(rpc.getnetworkinfo()["relayfee"])
+
+    logging.info("Check that mempoolminfee is minrelytxfee")
+    assert satoshi_round(rpc.getmempoolinfo()["minrelaytxfee"]) == Decimal("0.00001000")
+    assert satoshi_round(rpc.getmempoolinfo()["mempoolminfee"]) == Decimal("0.00001000")
+
+    txids = []
+    utxos = wallet.rpc.listunspent()
+
+    logging.info("Create a mempool tx that will be evicted")
+    us0 = utxos.pop()
+    inputs = [{"txid": us0["txid"], "vout": us0["vout"]}]
+    outputs = {wallet.getnewaddress(): 0.0001}
+    tx = wallet.rpc.createrawtransaction(inputs, outputs)
+    wallet.rpc.settxfee(str(relayfee))  # specifically fund this tx with low fee
+    txF = wallet.rpc.fundrawtransaction(tx)
+    wallet.rpc.settxfee(0)  # return to automatic fee selection
+    txFS = device.sign_raw_tx(txF["hex"], wallet)
+    txid = wallet.rpc.sendrawtransaction(txFS["hex"])
+
+    # ==== Specter-specific: can't abandon a valid pending tx ====
+    try:
+        wallet.abandontransaction(txid)
+    except SpecterError as e:
+        assert "Cannot abandon" in str(e)
+
+    # ==== Resume test from mempool_limit.py ====
+    # Spam the mempool with big transactions!
+    relayfee = satoshi_round(rpc.getnetworkinfo()["relayfee"])
+    base_fee = float(relayfee) * 100
+    for i in range(3):
+        txids.append([])
+        txids[i] = create_lots_of_big_transactions(
+            wallet, txouts, utxos[30 * i : 30 * i + 30], 30, (i + 1) * base_fee
+        )
+
+    logging.info("The tx should be evicted by now")
+    assert txid not in wallet.rpc.getrawmempool()
+    txdata = wallet.rpc.gettransaction(txid)
+    assert txdata["confirmations"] == 0  # confirmation should still be 0
+
+    # ==== Specter-specific: Verify purge and abandon ====
+    assert wallet.is_tx_purged(txid)
+    wallet.abandontransaction(txid)
+
+    # tx will still be in the wallet but marked "abandoned"
+    txdata = wallet.rpc.gettransaction(txid)
+    for detail in txdata["details"]:
+        if detail["category"] == "send":
+            assert detail["abandoned"]
+
+    # Can we now spend those same inputs?
+    outputs = {wallet.getnewaddress(): 0.0001}
+    tx = wallet.rpc.createrawtransaction(inputs, outputs)
+
+    # Fund this tx with a high enough fee
+    relayfee = satoshi_round(rpc.getnetworkinfo()["relayfee"])
+    wallet.rpc.settxfee(str(relayfee * Decimal("3.0")))
+
+    txF = wallet.rpc.fundrawtransaction(tx)
+    wallet.rpc.settxfee(0)  # return to automatic fee selection
+    txFS = device.sign_raw_tx(txF["hex"], wallet)
+    txid = wallet.rpc.sendrawtransaction(txFS["hex"])
+
+    # Should have been accepted by the mempool
+    assert txid in wallet.rpc.getrawmempool()
+    assert wallet.get_balance()["untrusted_pending"] == 0.0001
+
+    # Clean up
+    bitcoind_controller.stop_bitcoind()

tests/test_wallet_manager.py

@@ -1,4 +1,5 @@
 import json, os
+from conftest import instantiate_bitcoind_controller
 from cryptoadvance.specter.rpc import RpcError
 from cryptoadvance.specter.specter_error import SpecterError
 from cryptoadvance.specter.wallet import Wallet
@@ -6,11 +7,16 @@ from cryptoadvance.specter.key import Key
 from cryptoadvance.specter.wallet_manager import WalletManager
 
 
-def test_WalletManager(bitcoin_regtest, devices_filled_data_folder, device_manager):
+def test_WalletManager(docker, request, devices_filled_data_folder, device_manager):
+    # Instantiate a fresh bitcoind instance to isolate this test.
+    bitcoind_controller = instantiate_bitcoind_controller(
+        docker, request, rpcport=18998
+    )
+
     wm = WalletManager(
         200100,
         devices_filled_data_folder,
-        bitcoin_regtest.get_rpc(),
+        bitcoind_controller.rpcconn.get_rpc(),
         "regtest",
         device_manager,
     )
@@ -71,12 +77,20 @@ def test_WalletManager(bitcoin_regtest, devices_filled_data_folder, device_manag
     assert not os.path.exists(wallet_fullpath)
     assert len(wm.wallets) == 1
 
+    # cleanup
+    bitcoind_controller.stop_bitcoind()
+
+
+def test_wallet_createpsbt(docker, request, devices_filled_data_folder, device_manager):
+    # Instantiate a fresh bitcoind instance to isolate this test.
+    bitcoind_controller = instantiate_bitcoind_controller(
+        docker, request, rpcport=18998
+    )
 
-def test_wallet_createpsbt(bitcoin_regtest, devices_filled_data_folder, device_manager):
     wm = WalletManager(
         200100,
         devices_filled_data_folder,
-        bitcoin_regtest.get_rpc(),
+        bitcoind_controller.rpcconn.get_rpc(),
         "regtest",
         device_manager,
     )
@@ -166,6 +180,9 @@ def test_wallet_createpsbt(bitcoin_regtest, devices_filled_data_folder, device_m
     assert len(wallet.rpc.listlockunspent()) == 0
     assert wallet.full_available_balance == wallet.fullbalance
 
+    # cleanup
+    bitcoind_controller.stop_bitcoind()
+
 
 def test_wallet_sortedmulti(
     bitcoin_regtest, devices_filled_data_folder, device_manager