Page MenuHomePhabricator

test: test mempool rejection in interface_zmq.py
ClosedPublic

Authored by PiRK on Oct 12 2021, 12:44.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC0f144de39fd3: test: test mempool rejection in interface_zmq.py
Summary

Add a test to generate a "R" notification by:

  • sending a tx on node0 while the nodes are disconnected
  • verifying that this transaction is added to node0's mempool
  • generating a conflicting tx on node1
  • mining the conflicting tx into a block on node1
  • reconnecting the nodes
  • checking that the first tx is rejected on node0

Also add noban permission to all nodes to avoid the tx relay delays, because there are a lot of calls to sync_all and this diff adds more of them. This reduces the test run time from 45s to 14s.

This replaces the RBF tests that were in [[https://github.com/bitcoin/bitcoin/pull/19572 | core#19572]] backported in D10306.

Test Plan

test/functional/test_runner.py interface_zmq

Diff Detail

Event Timeline

PiRK requested review of this revision.Oct 12 2021, 12:44
Fabien requested changes to this revision.Oct 13 2021, 12:29
Fabien added a subscriber: Fabien.
Fabien added inline comments.
test/functional/interface_zmq.py
120

just use self.extra_args here, so you have a single spot to update

211

dito

289

dito

357

You may want to wrap this into a function to avoid having to search for where the difference is from the previous tx. Also you avoid tx1 and tx2 vars which are not needed

358

This comment is wrong, you got it the opposite way.
Also it's a good idea to make sure tx2 is in the block.

364

you could have used getbestblockhash but it's not even needed, as you can look at the output from generatetoaddress above

374

don't reuse the same variable, this is confusing. You should change the name of the previous one to something like txid_to_be_replaced

390

Don't mix these random refactors with the actual changes

439

what is supposed to be tested here ?

511

That belongs to D10306

520

dito

This revision now requires changes to proceed.Oct 13 2021, 12:29
test/functional/interface_zmq.py
439

It is the same replacement sequence as tested in this diff, but more elaborate thanks to RBF: a replaceable tx is added (A), then bumped (R, A), then the original pre-bump tx is mined into a block (conflict R, block connected C).
I don't think there is a way we can replicate that intermediate (R, A), and that's good news for 0-conf :)

Address review.

Refactor the conflicting transaction generation and mining into a method, and used it also in test_mempool_sync to replace the missing RBF notification.

fix a mistake in a comment: the number of remaining tx is 5. I changed it to 4 while when earlier I used range(num_txs - 1), then reverted that change. Just remove the number from the comment, so it cannot get wrong in the future

Fabien added inline comments.
test/functional/interface_zmq.py
118 ↗(On Diff #30492)

Nit: the extra_args being the "constant" part they should be placed first

This revision is now accepted and ready to land.Oct 13 2021, 15:58