test: run mempool_resurrect.py even with wallet disabled

mjdietzx commented at 9:03 pm on December 17, 2020: contributor

Another functional test rewritten as proposed in #20078

Request for help: node.gettransaction(txid) fails for transactions sent with wallet.send_self_transfer. Even though the txids look correct, are added to the mempool correctly, and removed from the mempool when a block is mined - all as expected.

However, node.gettransaction(txid) throws the error:

 0Traceback (most recent call last):
 1  File "/Users/michaeldietz/Documents/bitcoin/test/functional/test_framework/test_framework.py", line 126, in main
 2    self.run_test()
 3  File "/Users/michaeldietz/Documents/bitcoin/test/functional/mempool_resurrect.py", line 43, in run_test
 4    assert_equal(len(list(filter(lambda txid: node.gettransaction(txid)["confirmations"] > 0, spends_ids))), len(spends_ids))
 5  File "/Users/michaeldietz/Documents/bitcoin/test/functional/mempool_resurrect.py", line 43, in <lambda>
 6    assert_equal(len(list(filter(lambda txid: node.gettransaction(txid)["confirmations"] > 0, spends_ids))), len(spends_ids))
 7  File "/Users/michaeldietz/Documents/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
 8    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
 9  File "/Users/michaeldietz/Documents/bitcoin/test/functional/test_framework/authproxy.py", line 146, in __call__
10    raise JSONRPCException(response['error'], status)
11test_framework.authproxy.JSONRPCException: Invalid or non-wallet transaction id (-5)

Anyone know what’s going wrong / can point me in the right direction if I’m making a mistake, or MiniWallet needs to be improved for this to work correctly?

DrahtBot added the label Tests on Dec 17, 2020

lontivero commented at 10:58 pm on December 17, 2020: contributor

gettransaction is for wallet transactions and the node doesn’t know anything about MiniWallet.

mjdietzx commented at 0:24 am on December 18, 2020: contributor

gettransaction is for wallet transactions and the node doesn’t know anything about MiniWallet.

Really? What about here: https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/wallet.py#L74

The transactions are definitely in the node’s mempool (ie here: https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/wallet.py#L76)

lontivero commented at 1:06 am on December 18, 2020: contributor

MiniWallet is not a wallet handled by the node, it is an “external” wallet coded in python for testing. gettransaction only works for those wallets handled by the node, that’s why the error says: non-wallet transaction id

mjdietzx force-pushed on Dec 18, 2020

mjdietzx commented at 3:32 am on December 18, 2020: contributor

MiniWallet is not a wallet handled by the node, it is an “external” wallet coded in python for testing. gettransaction only works for those wallets handled by the node, that’s why the error says: non-wallet transaction id

Ah I got it - thanks for the explanation. I fixed and pushed/rebased

test: run mempool_resurrect.py even with wallet disabled 11a32722f0

mjdietzx force-pushed on Dec 18, 2020

lontivero commented at 1:25 pm on December 21, 2020: contributor

ACK. Good job.

MarcoFalke commented at 2:23 pm on December 21, 2020: member

ACK 11a32722f09f1d81f34bd09b26248ba99f2e7f07

MarcoFalke merged this on Dec 21, 2020

MarcoFalke closed this on Dec 21, 2020

in test/functional/mempool_resurrect.py:55 in 11a32722f0

80-            assert tx["confirmations"] == 0
81+        assert_equal(set(node.getrawmempool()), spends_ids)
82 
83         # Generate another block, they should all get mined
84-        self.nodes[0].generate(1)
85+        node.generate(1)

MarcoFalke commented at 2:35 pm on December 21, 2020:

Ah, missed this

blocks needs to be assigned here. See #20737

mjdietzx commented at 4:04 pm on December 21, 2020:

Great catch 👀

sidhujag referenced this in commit 0d702e3bdb on Dec 21, 2020

DrahtBot locked this on Feb 15, 2022

test: run mempool_resurrect.py even with wallet disabled #20692