[qa] feefilter: Clear mempool after each check #8316

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:Mf1607-qafeefilter changing 2 files +15 −12

MarcoFalke commented at 9:39 AM on July 8, 2016: member

Reported by @sdaftuar in #7148 (comment)
[qa] Promote usage of args_nodes fabfbb12d6
MarcoFalke added the label Tests on Jul 8, 2016
[qa] feefilter: Clear mempool after each check fa4a8cd840

in qa/rpc-tests/p2p-feefilter.py:None in fac1bb352c outdated

  22 | @@ -23,7 +23,7 @@ def allInvsMatch(invsExpected, testnode):
  23 |              if (sorted(invsExpected) == sorted(testnode.txinvs)):
  24 |                  return True;
  25 |          time.sleep(1)
  26 | -    return False;
  27 | +    raise AssertionError("Did not match; Timed out");

laanwj commented at 10:49 AM on July 11, 2016:

nit: No need for a ;

MarcoFalke commented at 11:45 AM on July 11, 2016:

Changed to ..

MarcoFalke force-pushed on Jul 11, 2016

MarcoFalke commented at 9:06 PM on July 16, 2016: member

This fixes the following travis failure:

  File "/home/travis/build/bitcoin/bitcoin/build/../qa/rpc-tests/p2p-feefilter.py", line 101, in run_test
    assert(allInvsMatch(txids, test_node))

MarcoFalke added the label Needs backport on Jul 18, 2016
sdaftuar commented at 8:25 PM on July 22, 2016: member

Thanks for trying to fix this. This might be good enough, but it occurred to me that in the situation where clearing the mempool is preventing the test failure we're trying to fix, it's really just covering up the fact that the test isn't exercising the code path we want to test.

The current test failures occur when the time from mempool-acceptance-to-relay exceeds 10 seconds (which it sometimes does, as the delay is random). So if we clear the mempool after 10 seconds to avoid this scenario, then we're not really exercising the feefilter logic in that situation.

So I think a more robust fix would be forget about clearing the mempool, and instead send a single transaction that should be received regardless of filter, and syncing with the receipt of that inv. I think we only need to do this in the test case where we call allInvsMatch and expect nothing (at line 96). We could then also get rid of the time.sleep(10) at line 95.

ping @morcos
MarcoFalke removed the label Needs backport on Aug 3, 2016
MarcoFalke closed this on Aug 3, 2016
MarcoFalke deleted the branch on Aug 3, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#7148 [NO MERGE] Travis: Run nightly test suite #8444 Fix p2p-feefilter.py for changed tx relay behavior