Rpc removemempoolentry #15873

pull IntegralTeam wants to merge 4 commits into bitcoin:master from IntegralTeam:rpc-removemempoolentry changing 3 files +79 −0
  1. IntegralTeam commented at 6:36 am on April 23, 2019: none
    Add new rpc removemempoolentry which removes transactions from mempool by hash
  2. add new `removemempoolentry` RPC 81d545f1ce
  3. fanquake added the label Mempool on Apr 23, 2019
  4. fanquake added the label RPC/REST/ZMQ on Apr 23, 2019
  5. fanatid commented at 6:41 am on April 23, 2019: contributor
    I support service which provide transactions history for users and accept their transactions for sending to network through bitcoind. Sometimes by some reason users transactions stuck for long time (very small percent). While blockexplorers kick users transactions from their mempool (which looks for users that their transaction not exists anymore), bitcoind which used by service still hold it. Previously for removing such transactions I was need stop bitcoind, remove mempool.dat and start bitcoind again. This is not good way for removing just 1 transaction from whole mempool. With this new RPC command removing stuck tx from mempool will be much easier.
  6. add test for `removemempoolentry` RPC 3007d9717a
  7. IntegralTeam force-pushed on Apr 23, 2019
  8. fix skip test `removemempoolentry` if no wallet f516ab29f4
  9. promag commented at 8:54 am on April 23, 2019: member

    You can refactor the test to not depend on the wallet.

    Concept ACK, at least this way there’s no need to restart the node to delete mempool.dat.

  10. jnewbery commented at 3:02 pm on April 23, 2019: member

    I think this PR is the result of a misunderstanding of how transaction propagation and confirmation works in Bitcoin.

    While blockexplorers kick users transactions from their mempool (which looks for users that their transaction not exists anymore)

    I expect that the vast majority of block explorers are backed by a Bitcoin Core node. The mempool will not remove a transaction except for:

    • expiry (if the transaction has been in the mempool for two weeks and is not confirmed)
    • limiting (the mempool is full and low-feerate transactions are removed from the bottom)
    • removeForBlock (removes any transactions that were included in or conflicted in a confirmed transaction in the blockchain)
    • replacement (replaced by fee under BIP 125 rules)

    Removing a transaction from your local mempool will not result in that transaction being removed from anyone else’s mempool. Even if you then create a new transaction that spends the same inputs, it will not cause the original transaction to be displaced in other node’s mempools (except if it meets the BIP 125 conditions).

    Perhaps I’m misunderstanding the original motivation for this PR. If so, can you give more details?

  11. MarcoFalke commented at 3:07 pm on April 23, 2019: member
    Agree with @jnewbery. Even though some miners run full-rbf, it might be easier and cleaner to use BIP 125 for now.
  12. fanatid commented at 3:41 pm on April 23, 2019: contributor

    I agree that mempool will not remove transaction, while it not expired/limited/included/replaced. I also realize that if transaction broadcasted to network we can not remove it from network, while somebody else hold it. But, by some case bitcoind (0.17.1) hold broadcasted transaction, while it was removed from blockchain.info/blockchair.com/etc. I’m sorry, but I can not provide more details, because this happened just few times in my experience.

    JFYI, parity has such method: parity_removeTransaction

  13. gmaxwell commented at 4:28 pm on April 23, 2019: contributor

    We’ve rejected this change before when it came without a really clear justification for its usefulness. I haven’t seen anything yet to change my mind there.

    Being in bitcoind while blockchain.info has removed it is not, in and of itself, a good case for it.

  14. andrewtoth commented at 10:11 pm on April 23, 2019: contributor

    One thing this would be useful for would be testing mining replaced transactions in regtest. Unless I’m overlooking a way to do this, there isn’t really a way to replace a transaction using RBF, then instead mine the transaction that was replaced. This is a common scenario on mainnet, since your replaced transaction might not propagate to a miner in time.

    The workflow to do it with this rpc would then be: sendrawtransaction to add the transction to mempool, sendrawtransaction with replacing transaction, removemempoolentry to remove the replacing transaction, then sendrawtransaction with the original transaction again, which won’t propagate to peer mempools, and then generatetoaddress.

  15. sipa commented at 10:51 pm on April 23, 2019: member

    Unless I’m overlooking a way to do this, there isn’t really a way to replace a transaction using RBF, then instead mine the transaction that was replaced.

    You can construct and mine blocks directly from the python test framework, without bitcoind RPCs.

  16. andrewtoth commented at 11:03 pm on April 23, 2019: contributor
    @sipa I’m talking about testing external systems systems which are talking to bitcoind, not using the python test framework.
  17. gmaxwell commented at 5:31 am on April 24, 2019: contributor
    @andrewtoth the same thing applies to whatever creates the block in the test, it can just manually inject the transaction.
  18. fix test `removemempoolentry` not to depend on the wallet. 51179cd273
  19. Relaxo143 commented at 10:04 am on April 26, 2019: none
    I like the idea of this rpc method since I’ve also personally needed to restart bitcoind in order to flush the mempool. Can someone explain though, could it be abused by any way?
  20. andrewtoth commented at 3:39 pm on April 26, 2019: contributor
    I suppose for my use case using this rpc would be a hack. A better solution would be modifying the generatetoaddress rpc to take a list of raw transactions to include in the block.
  21. MarcoFalke commented at 4:53 pm on April 26, 2019: member

    Simply rewrite the def create_block in the test framwork (or translate it to the language your test framework is written)

    If anyone needs this feature on mainnet, I fail to see why this can´t be achieved by BIP-125 (with the additional advantage that the remainder of the network and miners will also switch out the transaction)

  22. andrewtoth commented at 5:03 pm on April 26, 2019: contributor
    The case I am talking about is not using a test framework, but using bitcoind in regtest. For testing a system that listens directly on the zmq interface and makes requests using rpc, it would be nice to be able to have an rpc that could generate a block with specified txs instead of using whatever is in the mempool.
  23. sipa commented at 5:20 pm on April 26, 2019: member
    @andrewtoth I think that’s a reasonable thing to add (an RPC to generate a block with specified contents).
  24. jnewbery commented at 3:47 pm on April 28, 2019: member
    I think this is slightly less bad if it’s for testing only (hidden rpc, only functions on regtest), but I’d still prefer not to add it if there are other ways to achieve the same thing. @andrewtoth - could you start a bitcoind in -blocksonly, then explicitly feed it the transactions you want to include in the block via the sendrawtransaction RPC or a -whitelist peer, and then generate the block?
  25. andrewtoth commented at 11:37 pm on April 28, 2019: contributor
    @jnewbery Yes that is a workaround to achieve what I want, but it requires running a second bitcoind.
  26. luke-jr commented at 6:06 am on May 1, 2019: member
    I think #7533 is a cleaner solution to the desired goal as I understand it? (Despite the PR closed status, I have continued to maintain it, and it is available in Knots already.)
  27. DrahtBot commented at 5:45 am on July 13, 2019: member

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

    Conflicts

    No conflicts as of last run.

  28. laanwj added the label Feature on Sep 30, 2019
  29. fanquake commented at 2:13 am on February 25, 2020: member
    Doesn’t seem to be any consensus that this is a good idea, or would be merged. Going to close this for now. Commenters might be interested in #17693 instead.
  30. fanquake closed this on Feb 25, 2020

  31. DrahtBot locked this on Feb 15, 2022

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

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