rpc: Return wtxid from testmempoolaccept #20916

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2101-wtxidTestmempool changing 7 files +48 −10

MarcoFalke commented at 5:52 PM on January 12, 2021: member

It would be nice if testmempoolaccept returned the unique wtxid directly to avoid a costly decoderawtransaction roundtrip
rpc: Return wtxid from testmempoolaccept fa0aa87071
MarcoFalke commented at 5:53 PM on January 12, 2021: member

The test changes can be reviewed with --word-diff-regex=.
DrahtBot added the label RPC/REST/ZMQ on Jan 12, 2021
mjdietzx commented at 6:23 PM on January 12, 2021: contributor

utACK fa0aa87071eaec8a5df17774cdb352195e5e09de
DrahtBot commented at 7:09 PM on January 12, 2021: contributor


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #20889 (test: ensure any failing method in MiniWallet leaves no side-effects by mjdietzx)
- #20874 (test: Run mempool_limit.py even with wallet disabled by stackman27)
- #19801 (test: check for all possible OP_CLTV fail reasons in feature_cltv.py (BIP 65) by theStack)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot cross-referenced this on Jan 12, 2021 from issue test: ensure any failing method in MiniWallet leaves no side-effects by mjdietzx
DrahtBot cross-referenced this on Jan 12, 2021 from issue test: Run mempool_limit.py even with wallet disabled by stackman27
glozow commented at 7:43 PM on January 12, 2021: member

Concept ACK, useful information and we get it for free.
DrahtBot cross-referenced this on Jan 12, 2021 from issue rpc/validation: enable packages through testmempoolaccept by glozow
practicalswift commented at 9:02 PM on January 12, 2021: contributor

Concept ACK
stackman27 commented at 9:43 PM on January 12, 2021: contributor

utACK fa0aa87
DrahtBot cross-referenced this on Jan 12, 2021 from issue test: check for all possible OP_CLTV fail reasons in feature_cltv.py (BIP 65) by theStack

in test/functional/mempool_accept.py:55 in fa0aa87071

  50 | @@ -51,6 +51,8 @@ def skip_test_if_missing_module(self):
  51 |      def check_mempool_result(self, result_expected, *args, **kwargs):
  52 |          """Wrapper to check result of testmempoolaccept on node_0's mempool"""
  53 |          result_test = self.nodes[0].testmempoolaccept(*args, **kwargs)
  54 | +        for r in result_test:
  55 | +            r.pop('wtxid')  # Skip check for now

luke-jr commented at 3:46 PM on January 13, 2021:

        if 'wtxid' not in result_expected:
            for r in result_test:
                r.pop('wtxid')  # Skip check for now

MarcoFalke commented at 4:22 PM on January 13, 2021:

Thanks, will do if I have to push

luke-jr approved
luke-jr commented at 3:53 PM on January 13, 2021: member

utACK, but could make it nicer
glozow commented at 4:51 PM on January 13, 2021: member

cr ACK https://github.com/bitcoin/bitcoin/pull/20916/commits/fa0aa87071eaec8a5df17774cdb352195e5e09de
MarcoFalke merged this on Jan 15, 2021
MarcoFalke closed this on Jan 15, 2021
MarcoFalke deleted the branch on Jan 15, 2021
sidhujag referenced this in commit 4525e6783e on Jan 16, 2021
luke-jr referenced this in commit 204aa1a9a6 on Jan 28, 2021
bitcoin locked this on Aug 16, 2022

Contributors

Labels

rpc: Return wtxid from testmempoolaccept #20916

Conflicts