Improve mempool_package_limits.py #22901

pull naiza2000 wants to merge 10 commits into bitcoin:master from naiza2000:master changing 3 files +39 −39
  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

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-10-31 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me