Improve mempool_package_limits.py #22901

1. 
   naiza2000 commented at 10:16 am on September 6, 2021: contributor

   Follow-up to #21800 adding changes suggested in #21800#pullrequestreview-725180293
2. Improve rpc_packages.py 37e1d34c8e
3. Improve test_framework/wallet.py 28a120f790
4. Improve make_chain() in wallet.py caa72d8ea8
5. Improve mempool_package_limits.py 8ff1a13e64
6. Merge branch 'bitcoin:master' into master b72d61fddf
7. fanquake added the label Tests on Sep 6, 2021
8. 
   DrahtBot commented at 10:29 am on September 6, 2021: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   No conflicts as of last run.

9. fanquake requested review from glozow on Sep 6, 2021
10. in test/functional/test_framework/wallet.py:203 in b72d61fddf outdated

    204     }] if parent_locking_script else None
    205     signedtx = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=privkeys, prevtxs=prevtxs)
    206     assert signedtx["complete"]
    207     tx = tx_from_hex(signedtx["hex"])
    208-    return (tx, signedtx["hex"], my_value, tx.vout[0].scriptPubKey.hex())
    209+    return tx, signedtx["hex"], my_value, tx.vout[0].scriptPubKey.hex()
    

    ---

    ---

    

    glozow commented at 1:53 pm on September 6, 2021:

    I think #21800 (review) suggested changing this return value to a named tuple instead?
11. in test/functional/test_framework/wallet.py:275 in b72d61fddf outdated

    239@@ -238,18 +240,18 @@ def bulk_transaction(tx, node, target_weight, privkeys, prevtxs=None):
    240     """Pad a transaction with extra outputs until it reaches a target weight (or higher).
    241     returns CTransaction object
    242     """
    243-    tx_heavy = deepcopy(tx)
    

    ---

    ---

    

    glozow commented at 1:55 pm on September 6, 2021:

    Can also remove the import as well, iirc
12. in test/functional/test_framework/wallet.py:185 in b72d61fddf outdated

    181@@ -182,28 +182,30 @@ def sendrawtransaction(self, *, from_node, tx_hex):
    182         from_node.sendrawtransaction(tx_hex)
    183         self.scan_tx(from_node.decoderawtransaction(tx_hex))
    184 
    185-def make_chain(node, address, privkeys, parent_txid, parent_value, n=0, parent_locking_script=None, fee=DEFAULT_FEE):
    186-    """Build a transaction that spends parent_txid.vout[n] and produces one output with
    187+def make_chain(node, address, privkeys, parent_txid, parent_value, parent_locking_script=None, fee=DEFAULT_FEE):
    

    ---

    ---

    

    glozow commented at 1:59 pm on September 6, 2021:

    https://github.com/bitcoin/bitcoin/pull/21800/files#r685091478 also suggested renaming this function

    I’d suggest something like create_child ?

13. 
    glozow commented at 1:59 pm on September 6, 2021: member

    Concept ACK, thanks for working on this

    Linking the suggestions in the PR description and/or describing the changes within the commit messages would help reviewers understand what the improvements are. Also, you seem to have a merge commit that should be squashed?

14. in test/functional/rpc_packages.py:136 in b72d61fddf outdated

    132@@ -133,7 +133,7 @@ def test_independent(self):
    133     def test_chain(self):
    134         node = self.nodes[0]
    135         first_coin = self.coins.pop()
    136-        (chain_hex, chain_txns) = create_raw_chain(node, first_coin, self.address, self.privkeys)
    137+        chain_hex, chain_txns = create_raw_chain(node, first_coin, self.address, self.privkeys)
    

    ---

    ---

    

    glozow commented at 2:01 pm on September 6, 2021:

    You need to add the chain length here now that the default has been removed
15. 
    jnewbery commented at 1:26 pm on September 9, 2021: member

    Concept ACK. Will review once @glozow’s comments have been addressed and the CI failures are fixed.
16. fanquake added the label Waiting for author on Sep 10, 2021
17. 
    glozow commented at 6:56 am on September 27, 2021: member

    @naiza2000 still working on this?
18. Improve wallet.py c43fd4f44d
19. 
    naiza2000 commented at 3:03 pm on September 28, 2021: contributor

    @naiza2000 still working on this?

    Yes, I was out of town for the last few days, I have started working on it again.

20. fanquake removed the label Waiting for author on Sep 29, 2021
21. Rename make_chain() in wallet.py a65cd63449
22. resolve issues 269aeea72e
23. Merge branch 'bitcoin:master' into master adb1c5d4ed
24. DrahtBot added the label Needs rebase on Oct 5, 2021
25. DrahtBot removed the label Needs rebase on Oct 5, 2021
26. Merge branch 'bitcoin:master' into master 108603ef83
27. 
    glozow commented at 11:30 am on October 5, 2021: member

    Needs rebase and squash?
28. MarcoFalke added the label Waiting for author on Nov 9, 2021
29. 
    MarcoFalke commented at 9:11 am on November 9, 2021: member

    Can be closed?
30. 
    glozow commented at 5:44 pm on November 13, 2021: member

    Can be closed?

    Seems so. I can open a PR to clean up all the followups from #21800

31. MarcoFalke closed this on Nov 13, 2021

    ---

32. MarcoFalke added the label Up for grabs on Nov 13, 2021
33. 
    fanquake commented at 2:03 pm on August 4, 2022: member

    Seems so. I can open a PR to clean up all the followups from #21800 @glozow did this happen? Can we remove up for grabs?

34. glozow removed the label Up for grabs on Aug 4, 2022
35. bitcoin locked this on Aug 4, 2023

Contributors
naiza2000 DrahtBot glozow jnewbery MarcoFalke fanquake

Labels
Tests Waiting for author