RPC: add weight to mempool entry output #11256

pull esotericnonsense wants to merge 4 commits into bitcoin:master from esotericnonsense:2017-09-add-weight-to-mempool-entry changing 2 files +40 −17

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.
fanquake added the label RPC/REST/ZMQ on Sep 6, 2017
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 😉?
laanwj commented at 5:50 PM on September 6, 2017: member

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

I guess you need to update the documentation as well.
esotericnonsense force-pushed on Sep 7, 2017
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.
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).
RPC: add weight to mempool entry output 8385101f6c
qa: Add RPC tests for weight in mempool entry 9e338bc905
qa: Add more RPC tests for wtxid in mempool entry, clarify comment 22086cce4e
qa: Refactor segwit 3-tx-chain functional test to clarify use of 'tx' variable d4b0d81b58
esotericnonsense force-pushed on Sep 8, 2017
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.
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.
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...?
luke-jr referenced this in commit 2a88c1eb11 on Nov 11, 2017
luke-jr referenced this in commit 812bf314d1 on Nov 11, 2017
luke-jr referenced this in commit a36907e529 on Nov 11, 2017
luke-jr referenced this in commit 30b269614d on Nov 11, 2017

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

MarcoFalke added the label Needs rebase on Jun 6, 2018
laanwj commented at 12:21 PM on August 7, 2018: member

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

Closing this, please see #14649
meshcollider closed this on Nov 4, 2018
MarcoFalke referenced this in commit 70b12af87e on Aug 20, 2019
sidhujag referenced this in commit 4bc70112f1 on Aug 22, 2019
laanwj removed the label Needs rebase on Oct 24, 2019
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#14649 RPC: add weight to mempool entry output #16647 rpc: add weight to getmempoolentry output