Expand functional zmq transaction tests #19507

1. 
   instagibbs commented at 6:47 pm on July 13, 2020: member

   Tests written to better define what messages are sent when. Also did a bit of refactoring to make sure the exact notification channel ordering doesn’t matter.

   Confusions below aside, I believe having these more descriptive tests helps describe what behavior we expect from ZMQ notificaitons.

   Remaining confusion:

   1. Notification patterns seem to vary wildly with the inclusion of mempool transactions being reorg’ed. See difference between “Add zmq test for transaction pub during reorg” and “Have zmq reorg test cover mempool txns” commits for specifics.
   2. Why does a reorg’ed transaction get announced 3 times? From what I understand it can get announced once for disconnected block, once for mempool entry. What’s the third? It occurs a 4th time when included in a block(not added in test)
2. Make ordering of zmq consumption irrelevant to functional test e70512a83c
3. Add test case for mempool->block zmq notification 2399a0600c
4. Add zmq test for transaction pub during reorg a0f4f9c983
5. DrahtBot added the label Tests on Jul 13, 2020
6. 
   DrahtBot commented at 1:37 am on July 14, 2020: member

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #19572 (ZMQ: Create “sequence” notifier, enabling client-side mempool tracking by instagibbs)
   • #18309 (zmq: Add support to listen on multiple interfaces by n-thumann)

   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.

7. instagibbs force-pushed on Jul 14, 2020
8. Have zmq reorg test cover mempool txns 7356292e1d
9. instagibbs force-pushed on Jul 14, 2020
10. in test/functional/interface_zmq.py:129 in 2399a0600c outdated

    122@@ -123,6 +123,13 @@ def test_basic(self):
    123             hex = rawtx.receive()
    124             assert_equal(payment_txid, hash256_reversed(hex).hex())
    125 
    126+            # Mining the block with this tx should result in second notification
    127+            # after coinbase tx notification
    128+            self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)
    129+            hashtx.receive()
    

    ---

    ---

    

    promag commented at 2:58 pm on July 14, 2020:

    2399a0600ca9c4b676fa2f97520b8ecc44642246

    Could test coinbase:

    0-            self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)
    1-            hashtx.receive()
    2-            txid = hashtx.receive()
    3-            assert_equal(payment_txid, txid.hex())
    4+            hash = self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)[0]
    5+            coinbase_txid = self.nodes[0].getblock(hash)['tx'][0]
    6+            assert_equal(coinbase_txid, hashtx.receive().hex())
    7+            assert_equal(payment_txid, hashtx.receive().hex())
    

11. 
    promag commented at 3:03 pm on July 14, 2020: member

    Concept ACK, notifier order still exists but from the client side we don’t have to worry about order involving different messages.

    With this change we no longer test receiving different messages in the same socket. This could be done in a small test.

12. 
    laanwj commented at 3:31 pm on July 22, 2020: member

    code review ACK 7356292e1d7a44da8a2bd31c02c58d550bf38009
13. 
    promag commented at 10:52 pm on July 26, 2020: member

    Code review ACK 7356292e1d7a44da8a2bd31c02c58d550bf38009.

    It occurs a 4th time when included in a block(not added in test yet) @instagibbs will you update this branch?

14. 
    instagibbs commented at 1:09 pm on July 27, 2020: member

    @promag not really no, heh. Removed the “yet” in OP.
15. laanwj merged this on Aug 31, 2020
16. laanwj closed this on Aug 31, 2020

    ---

17. sidhujag referenced this in commit bca3840ae7 on Aug 31, 2020
18. jasonbcox referenced this in commit d4d6afe4b7 on Sep 3, 2020
19. jasonbcox referenced this in commit b365f2fe7d on Sep 3, 2020
20. deadalnix referenced this in commit 8ff44a4b27 on Sep 3, 2020
21. deadalnix referenced this in commit 389e1cb882 on Sep 3, 2020
22. DrahtBot locked this on Feb 15, 2022

Contributors
instagibbs DrahtBot promag laanwj

Labels
Tests