test: use MiniWallet for mempool_accept.py #24035

pull theStack wants to merge 4 commits into bitcoin:master from theStack:202201-test-use_MiniWallet_for_mempool_accept_etc changing 16 files +88 −86
  1. theStack commented at 2:58 pm on January 11, 2022: member

    This PR enables one more of the non-wallet functional tests (mempool_accept.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in #20078.

    It also includes some other minor changes that came up while working on the replacement:

    • [commit 1/4] replace magic number 0xffffffff for a tx’s nSequence with a new constant SEQUENCE_FINAL
    • [commit 2/4] create CTransaction instances with the current nVersion=2 by default, in order to use BIP68 for tests
    • [commit 3/4] support default from_node parameter for creating txs (this is a stripped down version of PR #24025)
  2. test: refactor: add constant for sequence number `SEQUENCE_FINAL` 2f79786822
  3. test: create txs with current `nVersion` (2) by default 
    This enables testing of BIP68 without the need of explicitly
setting nVersion to 2. This is e.g. useful for transactions
created with MiniWallet.
    f30041c914
  4. test: MiniWallet: support default `from_node` for creating txs 
    If no `from_node` parameter is passed explicitely to the
`create_self_transfer` method, the test node passed in the course
of creating the MiniWallet instance is used.  This seems to
be the main use-case in most of the current functional
tests, i.e. in many instances the calls can be shortened.
    b24f6c6855
  5. theStack force-pushed on Jan 11, 2022
  6. in test/functional/mempool_accept.py:89 in ebc3d51bf5 outdated 
    85@@ -86,27 +86,24 @@ def run_test(self):
86 
87         self.log.info('A transaction not in the mempool')
88         fee = Decimal('0.000007')
89-        raw_tx_0 = node.signrawtransactionwithwallet(node.createrawtransaction(
90-            inputs=[{"txid": txid_in_block, "vout": 0, "sequence": BIP125_SEQUENCE_NUMBER}],  # RBF is used later
91-            outputs=[{node.getnewaddress(): Decimal('0.3') - fee}],
92-        ))['hex']
93-        tx = tx_from_hex(raw_tx_0)
94+        self.wallet.scan_tx(node.decoderawtransaction(raw_tx_in_block))
    MarcoFalke commented at 4:35 pm on January 11, 2022:
    You can use self.wallet.sendrawtransaction(node, tx), no?
    theStack commented at 12:13 pm on January 12, 2022:
    This didn’t work due too high fee caused by the missing -maxfeerate=0 parameter. Fixed that by changing the amounts now, i.e. we can use wallet.sendrawtransaction(...) and don’t to call wallet.scan_tx(...) manually anymore.
    MarcoFalke commented at 12:23 pm on January 12, 2022:
    It might be cleaner to keep the amounts and change sendraw to accept and pass on **kwargs (like def generate).
    theStack commented at 3:54 pm on January 12, 2022:
    Nice idea, done!
  7. in test/functional/mempool_accept.py:189 in ebc3d51bf5 outdated 
    191+        tx = self.wallet.create_self_transfer(utxo_to_spend=utxo_to_spend, sequence=SEQUENCE_FINAL)['tx']
192+        tx.vout[0].nValue = int(0.05 * COIN)
193+        raw_tx_reference = tx.serialize().hex()
194         # Reference tx should be valid on itself
195         self.check_mempool_result(
196             result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal('0.1') - Decimal('0.05')}}],
    MarcoFalke commented at 4:46 pm on January 11, 2022:
    You can remove signrawtransactionwithwallet a few lines down?
    theStack commented at 12:14 pm on January 12, 2022:
    Thanks, done. Also removed a bunch of other comments about skipping (re)-signing, since we don’t sign anything here in this test in the first place.
  8. in test/functional/mempool_accept.py:183 in ebc3d51bf5 outdated 
    184-            inputs=[{'txid': txid_spend_both, 'vout': 0}],
185-            outputs=[{node.getnewaddress(): 0.05}],
186-        ))['hex']
187-        tx = tx_from_hex(raw_tx_reference)
188+        self.log.info('Create a "reference" tx for later use')
189+        self.wallet.scan_tx(node.decoderawtransaction(raw_tx_spend_both))
    MarcoFalke commented at 4:47 pm on January 11, 2022:
    Same
  9. in test/functional/mempool_accept.py:184 in ebc3d51bf5 outdated 
    186-        ))['hex']
187-        tx = tx_from_hex(raw_tx_reference)
188+        self.log.info('Create a "reference" tx for later use')
189+        self.wallet.scan_tx(node.decoderawtransaction(raw_tx_spend_both))
190+        utxo_to_spend = self.wallet.get_utxo(txid=txid_spend_both)
191+        tx = self.wallet.create_self_transfer(utxo_to_spend=utxo_to_spend, sequence=SEQUENCE_FINAL)['tx']
    MarcoFalke commented at 4:49 pm on January 11, 2022:
    Why set FINAL when it wasn’t set previously?
    theStack commented at 12:19 pm on January 12, 2022:

    Per default createrawtransaction sets the input’s nSequence fields to 0xffffffff i.e. it actually was already final before. This is needed because of the following line below:

    https://github.com/bitcoin/bitcoin/blob/318c79e8096ce477561ef099835954b95a687685/test/functional/mempool_accept.py#L326

    Probably it would make sense in a follow-up to also create CTxIn instances with nSequence=SEQUENCE_FINAL by default in the test framework, to match the behaviour in the bitcoind codebase?

    https://github.com/bitcoin/bitcoin/blob/318c79e8096ce477561ef099835954b95a687685/src/primitives/transaction.h#L105-L106

    MarcoFalke commented at 12:29 pm on January 12, 2022:
    ok, fair enough
  10. DrahtBot added the label Tests on Jan 11, 2022
  11. MarcoFalke approved
  12. MarcoFalke commented at 4:49 pm on January 11, 2022: member

    ACK ebc3d51bf50fa88042bb2620c37206f0528a70b0 🕉

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3ACK ebc3d51bf50fa88042bb2620c37206f0528a70b0 🕉
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUiyQgv/RaF3J1PaDgEOYs6oJ32aX+c/14xaYW5lzUWPzXT7qa/UzlZI1ZgiHpPM
 8/2aQ0cbjYwRHdDBNS0oE/V/N3DI28oxxOHzTVaCbQwqVeVcMLDsZhb43831NQcz1
 9otfqSomZZJineWtjZRaKcOB8b20qwy0uGvyVvKgvNmeFDMaZC8K1JKNnar4+UtAU
10oJQoZBPbWzY5GHsj3v9B7HpJN+K3iIYHlbaBs+x9jOlLZHnJT896cKBycw4JrDXL
11bw45hBwK4EdnK4+KJZWzC3l9OeZEZ4aubNFtX2MlANp5l+CUsaZVgnwi4UnJebUn
12/fTef1W+Vg6B7pOvqa3iiMaIN1wBVEs3BLG+PjBvnVTDz9CMQryzHpZB13gk42O7
13BO1gsOrB346P6zpGR5smobMWabyqZhbDH2I7/8qbCdQxKQ3VSUGlGh7r3y2BoW+j
143SrVsPFifDRMjqHZy697eFxFpjj/6SDwUrLEYfHrdBz1U8P/xReuJFh9eyjpHV5w
15XnSV1rOS
16=ULDi
17-----END PGP SIGNATURE-----
  13. theStack force-pushed on Jan 12, 2022
  14. theStack commented at 12:24 pm on January 12, 2022: member
    Thanks for the review @MarcoFalke, tried to address all of your comments (getting rid of wallet.scan_tx() calls, removing comments about skipping (re-)signing that don’t apply anymore).
  15. MarcoFalke commented at 12:27 pm on January 12, 2022: member

    re-ACK ebd3b552686a3333e06cb2e151ef5c99af327c03 🐘

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3re-ACK ebd3b552686a3333e06cb2e151ef5c99af327c03  🐘
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUj1/Qv9H86e10i7hwNLo1kyCzTSTysj1Zv24M5ZpSF+L+A62IoPFsn0aDDM0DHY
 8ksIUKe2s32PHhM+/CPS4LYr1q+B3NuEEir1MTrnucfdEwpEyb05cWHGxFANr275o
 9Ztj2Nxvb4Qgdt6YS4lkxfmP0aKG8PTKzAPaIUbpjIbtfxE/i2piMIu0W5a8MxA7D
10BKSC04U2L0te8k+aFBhKKcZUHBoj+ETk4te66xOrO30WTPyq5AmsuFufyplnAV7S
11rd94vZ4her2/8WtRdkAAhyPEdfrS9n909PwPQkC8Twh0zJ2PxBhW9sByWPgDHpWC
12ZqCU+ziyZXl1KENKaddQCaQKlJ9udCqVGDFGhxlZrjbS1EtqHAy2AgJKSdYOUiiO
13Grk2Bd0FiRkuq6akZt3qyIEpq9RChFdhzUy9yhybZfF3RakPKtzv6g2kNLz4OhE8
147v8Jf3JQTbYBZ+Z6AlzxGSnd8zlb6yfOjHaSmg7zSed/djfsXtUGeYo+5gttWF12
15n9YAaOLW
16=gJ7h
17-----END PGP SIGNATURE-----
  16. in test/functional/mempool_accept.py:105 in ebd3b55268 outdated 
    105-        coin = coins.pop()  # Pick a random coin(base) to spend
106         output_amount = Decimal('0.025')
107-        raw_tx_final = node.signrawtransactionwithwallet(node.createrawtransaction(
108-            inputs=[{'txid': coin['txid'], 'vout': coin['vout'], "sequence": 0xffffffff}],  # SEQUENCE_FINAL
109-            outputs=[{node.getnewaddress(): output_amount}],
110-            locktime=node.getblockcount() + 2000,  # Can be anything
    MarcoFalke commented at 1:04 pm on January 12, 2022:
    nit: any reason to remove this comment?
    theStack commented at 3:54 pm on January 12, 2022:
    No reason, this was not intended. Put it back in the latest force-push.
    MarcoFalke commented at 5:51 pm on January 12, 2022:
    nit: any reason to remove the trailing comma? :sweat_smile:
    theStack commented at 5:35 pm on January 13, 2022:
    D’oh, for some reason I thought that’s only allowed for import lists and not for function calls :smirk: Done!
  17. theStack force-pushed on Jan 12, 2022
  18. theStack commented at 3:57 pm on January 12, 2022: member
    Force-pushed again with two suggestions tackled (https://github.com/bitcoin/bitcoin/pull/24035#discussion_r783026046 and #24035 (review)). Thanks for your patience!
  19. MarcoFalke approved
  20. MarcoFalke commented at 5:53 pm on January 12, 2022: member

    re-ACK 0dd28a752f15df117c810123810729703c7c575c 🍢

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3re-ACK 0dd28a752f15df117c810123810729703c7c575c 🍢
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUirgAv/Zcb5audGGnebIIxRH85LrU+UjCbmnBPH4iENA6AFzCUPRGkbigiGM1dq
 85JqciQbgKy7ooOooat0gGntexJDQ7A42HVcCoXkEo9GtGUC4VjF8o0rF+J3CoSzQ
 99IIeggEEgUWsC0Fuj4jtf45AA1YWxr2RlF1ztTCuIWNsa/AdDhSr+raGmwCsj8uR
10vnrzNuCU16gBghLTS/ClU030mX2mjr4AcNkbEy1UIN/khyxsLN/XKEKrfMCpuWdB
11mC/FDbbAFJcLu8ERe+V0PYjxsOqWC2lvM712iE5RwuBPaMAAms4rzc8AL28zUFQ0
12AUokMsm71IbOHzGRFXV8sNLvcKACj23l8HS+0/6QdX7VThefClUS27T2ATi5NDgO
13jbCnHT7Ea/zdzkNlOyOmdUmrJ8QiatwgsJ50ZIDqStMYjXP5mgABDASuLyZeCA7S
14vGUjYZaTPY3giAgx3NV9TX8EOJVWflDYEXpx5y6+4Bi+5u7fCA7uuAYqQM8fER0Q
15OzDTk3jT
16=S1cB
17-----END PGP SIGNATURE-----
  21. test: use MiniWallet for mempool_accept.py 
    This test can now be run even with the Bitcoin Core wallet disabled.
    aa8a65e4a8
  22. theStack force-pushed on Jan 13, 2022
  23. MarcoFalke commented at 5:39 pm on January 13, 2022: member

    re-ACK aa8a65e4a88bfbd83ac756a87bfb8faf52fb675d 📊

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3re-ACK aa8a65e4a88bfbd83ac756a87bfb8faf52fb675d 📊
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUi3mgv/fsG1sHFrfTMhZoVxBbD6+vaKEU/xFzPRQZWnF1k+GN2EzwX5NdzpgTqu
 8+OPUKrlj61y4bHAJVtp2EccDdqMCvSgsBwKB5Td1tGFLTk81LZYFPUb1cvaO9y0N
 9lORWGlIiyePaUOOBTA1ahiHAiQvr7UfuU64kmfjSCOgtpQ+LgSoJgwoUMi6KY47a
10I998bcENP130pfAX8LkUF2vNz86Vp7S/ej1rPSU8OHhImkjy8Ie0lY1pi+ZcHZ73
111yufp27fvleFRRd5WbcFQz/m0M53VM/VP+ZpeqH2Ka5cSHyZV7niLnXOMf/BLtWv
125MQP/08ZRx3Bt0xITD0bIolbf68be7mQKvBb8LrTCsT5isV7koTSPafQJ5f9lsae
13ZfgiJkjv7DoHUzvia37r1AESOCVkmFcP5LBp9HTesaqa7pwhoZi5oXzEscmISyyl
14A+RZIYazGUk4ZL8iK7jiM0uaZETvxeYWkYVXbSuz0GFahPncvbhQOwV2+5Zgmn23
15UTiuj0qJ
16=ZFWj
17-----END PGP SIGNATURE-----
  24. MarcoFalke merged this on Jan 13, 2022
  25. MarcoFalke closed this on Jan 13, 2022
  26. theStack deleted the branch on Jan 13, 2022
  27. sidhujag referenced this in commit ee6e8d51f7 on Jan 14, 2022
  28. DrahtBot locked this on Jan 13, 2023


theStack MarcoFalke

Labels
Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-29 01:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me