rpc: Return total fee in getmempoolinfo#20944
Conversation
maflcko
changed the title
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
maflcko
force-pushed
the
2101-rpcMempoolTotalFee
branch
from
fa19a80 to
fa1c01e
Compare
|
Concept ACK, light code review. Looks good. |
|
Concept ACK |
maflcko
force-pushed
the
2101-rpcMempoolTotalFee
branch
2 times, most recently
from
fa878bb to
fa79be7
Compare
|
Good point. Force pushed to make the test more useful. |
There was a problem hiding this comment.
Tested ACK fa79be7aef9c75c78a4f6177051f867cbb623a9e. Tested both the RPC call and the REST interface (/rest/mempool/info.json).
Could have some total_fee documentation in the REST-interface.md here.
EDIT: Unrelated to this PR, but it seems like documentation for unbroadcastcount is missing from REST-interface.md too.
I'll leave fixing the docs as a follow-up. Ideally the docs are auto-generated, which is on my roadmap. |
Also, add missing lock annotations
|
Didn't realize this had only one ACK after my force push, so I went ahead and fixed the documentation. |
maflcko
force-pushed
the
2101-rpcMempoolTotalFee
branch
from
fa79be7 to
fa36206
Compare
Github-Pull: bitcoin#20944 Rebased-From: fa36206 (partial) Diff-minimised
glozow
left a comment
glozow
left a comment
There was a problem hiding this comment.
ACK fa36206 🧸
I would probably split the mempool and rpc commits, but nbd. Mempool code looks correct. I sanity-checked by adding some stuff to functional tests like
old_fee = node.getmempoolinfo()['total_fee']
node.sendrawtransaction(raw_tx_0)
assert_equal(old_fee + fee, node.getmempoolinfo()['total_fee'])
assert_equal(node.getmempoolinfo()['total_fee'], sum(v['fees']['base'] for k, v in self.nodes[0].getrawmempool(verbose=True).items()))
print(node.getmempoolinfo()['total_fee'])Tried it out with RBFing in mempool_accept.py, after expired txns in mempool_expiry, and expired txns in mempool_expiry.py, all worked for me.
| assert_equal(len(self.nodes[0].getrawmempool()), 5) | ||
| assert_equal(len(self.nodes[1].getrawmempool()), 5) | ||
|
|
||
| total_fee_old = self.nodes[0].getmempoolinfo()['total_fee'] |
There was a problem hiding this comment.
Perhaps could do a check here that total_fee_old is a positive integer?
There was a problem hiding this comment.
Could also be checked via CHECK_NONFATAL on the server side.
|
ACK fa36206 |
9 participants
This avoids having to loop over the whole mempool to query each entry's fee