JoinMarket-Org / joinmarket-clientserver

Bitcoin CoinJoin implementation with incentive structure to convince people to take part
GNU General Public License v3.0
734 stars 179 forks source link

Tests fail with Bitcoin Core 28.0 #1734

Closed kristapsk closed 1 month ago

kristapsk commented 1 month ago

See https://github.com/kristapsk/joinmarket-clientserver/actions/runs/11199073884/job/31131015181 and https://github.com/kristapsk/joinmarket-clientserver/actions/runs/11199169029/job/31131227626.

=================================== FAILURES ===================================
__________________ test_check_valid_candidate_not_replaceable __________________

setup_wallet = [<jmclient.wallet.SegwitWallet object at 0x7fd761518b50>, <jmclient.wallet_service.WalletService object at 0x7fd761f50d60>]

    def test_check_valid_candidate_not_replaceable(setup_wallet):
        # tests that the transaction is replaceable
        wallet = setup_wallet[0]
        wallet_service = setup_wallet[1]
        wallet_service.resync_wallet()
        addr = wallet.get_external_addr(0)
        utxo = fund_wallet_addr(wallet, addr)
        amount_sats = 10**7
        tx = btc.mktx([utxo],
                      [{"address": str(btc.CCoinAddress.from_scriptPubKey(
                          btc.CScript(b"\x00").to_p2sh_scriptPubKey())),
                        "value": amount_sats},
                       {"address": wallet.get_internal_addr(0),
                        "value": 10**8 - amount_sats - 142}])
        success, msg = wallet.sign_tx(tx, {0: (wallet.addr_to_script(addr), 10**8)})
        success = jm_single().bc_interface.pushtx(tx.serialize())

        with pytest.raises(ValueError, match="Transaction not replaceable."):
>           check_valid_candidate(tx, wallet)
E           Failed: DID NOT RAISE <class 'ValueError'>

test/unified/test_bumpfee.py:109: Failed
----------------------------- Captured stdout call -----------------------------
JoinMarket Alert Message: LATEST RELEASE v0.2.2. Useful new features. Update ASAP, and do not use pre-0.2.0! https://bitcointalk.org/index.php?topic=919116.msg16714124#msg16714124
2024-10-06 05:20:28,101 [DEBUG]  rpc: listaddressgroupings []
JoinMarket Alert Message: LATEST RELEASE v0.2.2. Useful new features. Update ASAP, and do not use pre-0.2.0! https://bitcointalk.org/index.php?topic=919116.msg16714124#msg16714124
2024-10-06 05:20:28,264 [DEBUG]  Fast sync in progress. Got this many used addresses: 5
JoinMarket Alert Message: LATEST RELEASE v0.2.2. Useful new features. Update ASAP, and do not use pre-0.2.0! https://bitcointalk.org/index.php?topic=919116.msg16714124#msg16714124
2024-10-06 05:20:28,278 [DEBUG]  rpc: listunspent [0]
JoinMarket Alert Message: LATEST RELEASE v0.2.2. Useful new features. Update ASAP, and do not use pre-0.2.0! https://bitcointalk.org/index.php?topic=919116.msg16714124#msg16714124
2024-10-06 05:20:28,532 [DEBUG]  bitcoind sync_unspent took 0.25427937507629395sec
JoinMarket Alert Message: LATEST RELEASE v0.2.2. Useful new features. Update ASAP, and do not use pre-0.2.0! https://bitcointalk.org/index.php?topic=919116.msg16714124#msg16714124
2024-10-06 05:20:28,534 [DEBUG]  rpc: sendtoaddress ['bcrt1q23lxvv8l66r2e4cnx58ygahp6438knfwrwxsk3', 1]
JoinMarket Alert Message: LATEST RELEASE v0.2.2. Useful new features. Update ASAP, and do not use pre-0.2.0! https://bitcointalk.org/index.php?topic=919116.msg16714124#msg16714124
2024-10-06 05:20:28,741 [DEBUG]  rpc: generatetoaddress [1, 'bcrt1qtnarrg3hszexna5ctpmaadxu9rns08x2ds64we']
JoinMarket Alert Message: LATEST RELEASE v0.2.2. Useful new features. Update ASAP, and do not use pre-0.2.0! https://bitcointalk.org/index.php?topic=919116.msg16714124#msg16714124
2024-10-06 05:20:28,753 [DEBUG]  rpc: sendrawtransaction ['01000000000101261c9700cf1869412fecf6fbebd3f77bdcb78acc11a48ecfa9d21b090edd87d30100000000ffffffff02809698000000000017a9149f7fd096d37ed2c0e3f7f0cfc924beef4ffceb6887f2495d0500000000160014cee1c38f2bf7b161e8e307b896db8d04cc96dfc20247304402205253feb069792c0e0b4558fc53ca1940f41b8467001bd7187167f7c94c583f2e02207052bbb95f435358c54aa0ab886f9b38a97a22753f07a1983706ffaeef101291012103990d2c46d92704c1f7a57cb99220e376d399550c4515ca107db1d02deb2e933a00000000']
JoinMarket Alert Message: LATEST RELEASE v0.2.2. Useful new features. Update ASAP, and do not use pre-0.2.0! https://bitcointalk.org/index.php?topic=919116.msg16714124#msg16714124
2024-10-06 05:20:28,754 [DEBUG]  will call tfc after 2 seconds.
JoinMarket Alert Message: LATEST RELEASE v0.2.2. Useful new features. Update ASAP, and do not use pre-0.2.0! https://bitcointalk.org/index.php?topic=919116.msg16714124#msg16714124
2024-10-06 05:20:28,757 [DEBUG]  rpc: getmempoolinfo []
------------------------------ Captured log call -------------------------------
DEBUG    joinmarket:blockchaininterface.py:433 rpc: listaddressgroupings []
DEBUG    joinmarket:wallet_service.py:569 Fast sync in progress. Got this many used addresses: 5
DEBUG    joinmarket:blockchaininterface.py:433 rpc: listunspent [0]
DEBUG    joinmarket:wallet_service.py:868 bitcoind sync_unspent took 0.25427937507629395sec
DEBUG    joinmarket:blockchaininterface.py:433 rpc: sendtoaddress ['bcrt1q23lxvv8l66r2e4cnx58ygahp6438knfwrwxsk3', 1]
DEBUG    joinmarket:blockchaininterface.py:433 rpc: generatetoaddress [1, 'bcrt1qtnarrg3hszexna5ctpmaadxu9rns08x2ds64we']
DEBUG    joinmarket:blockchaininterface.py:433 rpc: sendrawtransaction ['01000000000101261c9700cf1869412fecf6fbebd3f77bdcb78acc11a48ecfa9d21b090edd87d30100000000ffffffff02809698000000000017a9149f7fd096d37ed2c0e3f7f0cfc924beef4ffceb6887f2495d0500000000160014cee1c38f2bf7b161e8e307b896db8d04cc96dfc20247304402205253feb069792c0e0b4558fc53ca1940f41b8467001bd7187167f7c94c583f2e02207052bbb95f435358c54aa0ab886f9b38a97a22753f07a1983706ffaeef101291012103990d2c46d92704c1f7a57cb99220e376d399550c4515ca107db1d02deb2e933a00000000']
DEBUG    joinmarket:blockchaininterface.py:836 will call tfc after 2 seconds.
DEBUG    joinmarket:blockchaininterface.py:433 rpc: getmempoolinfo []
=========================== short test summary info ============================
FAILED test/unified/test_bumpfee.py::test_check_valid_candidate_not_replaceable
================== 1 failed, 424 passed in 327.35s (0:05:27) ===================
Error: Process completed with exit code 1.
overcoin commented 1 month ago

laanwj commented on Apr 12

Correct, version 28.0 (development on current master now that 27.0 is branched off) is extremely likely not going to have legacy wallet support anymore. We'll have to find some solution for this.

https://github.com/JoinMarket-Org/joinmarket-clientserver/issues/1571#issuecomment-2051080385

kristapsk commented 1 month ago

laanwj commented on Apr 12

Correct, version 28.0 (development on current master now that 27.0 is branched off) is extremely likely not going to have legacy wallet support anymore. We'll have to find some solution for this.

#1571 (comment)

Ahh, yes, right, totally forgot about this (really busy with other stuff recently).

As there is not much development activity recently, guess we will need to just implement hackish descriptor wallet support I proposed in #1064.

kristapsk commented 1 month ago

There is also another problem, that even with my previously proposed PR, tests would still not be compatible with v28 and descriptor wallets in general, as currently we have single Bitcoin Core wallet which are used both for mining new coins and then spending them and as a watch-only wallet, but descriptor wallets does not allow importing addresses into wallets with enabled private keys.

whitslack commented 1 month ago

The test case failure in this issue is unrelated to legacy-vs-descriptor wallets. This issue is due to the change in Bitcoin Core 28.0 of the default value for the mempoolfullrbf config option from 0 to 1. You can make the test_check_valid_candidate_not_replaceable test case pass by adding this line to the bitcoin.conf used for the tests:

mempoolfullrbf=0

In the long run, though, the test case is arguably ill-conceived (or at best obsolete). It has never been safe to assume that an unconfirmed transaction could not be replaced merely because it did not "opt in" to RBF "eligibility."

kristapsk commented 1 month ago

Thanks @whitslack for the hint!