RPC: add weight to mempool entry output #11256

1. 
   esotericnonsense commented at 11:08 am on September 6, 2017: contributor

   Tested against master using the REST api (/rest/mempool/contents), simple addition of a field.

   Personal use case is for fee analysis software.

2. fanquake added the label RPC/REST/ZMQ on Sep 6, 2017
3. 
   promag commented at 4:16 pm on September 6, 2017: member

   No test affected 😞 care to improve by asserting the new field in the relevant RPC’s 😉?
4. 
   laanwj commented at 5:50 pm on September 6, 2017: member

   Concept ACK
5. 
   MarcoFalke commented at 5:59 pm on September 6, 2017: member

   I guess you need to update the documentation as well.
6. esotericnonsense force-pushed on Sep 7, 2017
7. 
   esotericnonsense commented at 1:18 am on September 7, 2017: contributor

   Tests are failing. txid2 and txid3 work as expected. txid1 fails on both my test and #11203 (add wtxid to mempool entry output).

   Do not merge as is.

8. 
   ajtowns commented at 6:18 am on September 8, 2017: member

   txid1 is failing because at that point “tx” is actually referring to the input to txid1, not the transaction for txid1. It works fine for me if I add

   tx = FromHex(CTransaction(), self.nodes[0].gettransaction(txid1)['hex'])

   prior to the assert_equal lines (and uncomment them obviously).

9. RPC: add weight to mempool entry output 8385101f6c
10. qa: Add RPC tests for weight in mempool entry 9e338bc905
11. qa: Add more RPC tests for wtxid in mempool entry, clarify comment 22086cce4e
12. qa: Refactor segwit 3-tx-chain functional test to clarify use of 'tx' variable d4b0d81b58
13. esotericnonsense force-pushed on Sep 8, 2017
14. 
    esotericnonsense commented at 7:30 pm on September 8, 2017: contributor

    Doh. You’re absolutely right. Fixed.

    The final commit ‘Refactor segwit 3-tx-chain’ changes all references to ’tx’ to ’tx/tx1/tx2/tx3’ in order to clarify that. It has a large diff and can be dropped if necessary (only affects code style).

    I have also rebased on master at 3255d63.

    Should be good to go now.

15. 
    morcos commented at 6:43 pm on September 11, 2017: member

    Concept ACK, but it turns out GetTxSize is not actually what we claim it is. See the calculation of GetVirtualTransactionSize which potentially includes number of sig ops in the calculation.

    I thinke the right path forward is:

    • a small BIP documenting this usage of virtual transaction size (which is used almost everywhere in our code
    • documentation update pointing to this new BIP instead of 141 for defining virtual size
    • this PR makes even more sense given this lack of easy conversion, but I think the newly added tests should at least be documented to note that they only work in the case that sig ops don’t factor in, and perhaps we should add a test with a tx that has more sigops to show what happens in that case.
16. 
    luke-jr commented at 9:11 am on November 10, 2017: member

    @morcos AFAIK that’s exclusively used for node policy, and as such isn’t a topic for standardisation…?
17. luke-jr referenced this in commit 2a88c1eb11 on Nov 11, 2017
18. luke-jr referenced this in commit 812bf314d1 on Nov 11, 2017
19. luke-jr referenced this in commit a36907e529 on Nov 11, 2017
20. luke-jr referenced this in commit 30b269614d on Nov 11, 2017
21. in test/functional/segwit.py:257 in d4b0d81b58

    255+        # Check that wtxid is properly reported in mempool entry (txid1)
    256+        assert_equal(int(self.nodes[0].getmempoolentry(txid1)["wtxid"], 16), tx1.calc_sha256(True))
    257+
    258+        # Check that weight and sizei (actually vsize) are properly reported in mempool entry (txid1)
    259+        assert_equal(self.nodes[0].getmempoolentry(txid1)["size"], (self.nodes[0].getmempoolentry(txid1)["weight"] + 3) // 4)
    260+        assert_equal(self.nodes[0].getmempoolentry(txid1)["weight"], len(tx1.serialize())*3 + len(tx1.serialize_with_witness()))
    

    ---

    ---

    

    luke-jr commented at 5:02 am on March 11, 2018:

    serialize needs to be made serialize_without_witness
22. MarcoFalke added the label Needs rebase on Jun 6, 2018
23. 
    laanwj commented at 12:21 pm on August 7, 2018: member

    Needs rebase and review comments addressed.
24. 
    meshcollider commented at 1:31 am on November 4, 2018: contributor

    Closing this, please see #14649
25. meshcollider closed this on Nov 4, 2018

    ---

26. MarcoFalke referenced this in commit 70b12af87e on Aug 20, 2019
27. sidhujag referenced this in commit 4bc70112f1 on Aug 22, 2019
28. laanwj removed the label Needs rebase on Oct 24, 2019
29. DrahtBot locked this on Dec 16, 2021

Contributors
esotericnonsense promag laanwj MarcoFalke ajtowns morcos luke-jr meshcollider

Labels
RPC/REST/ZMQ