Implement BIP159 NODE_NETWORK_LIMITED (pruned peers) *signaling only* #11740

pull jonasschnelli wants to merge 5 commits into bitcoin:master from jonasschnelli:2017/11/NNL_signaling changing 7 files +102 −4
  1. jonasschnelli commented at 9:02 PM on November 20, 2017: contributor

    Extracted from #10387. Does implement BIP159, but only the signalling part. No connections are made to NODE_NETWORK_LIMITED in this PR.

    The address relay and connection work (the more complicated part) can then be separated (probably in #10387).

  2. jonasschnelli added the label P2P on Nov 20, 2017
  3. fanquake commented at 11:26 PM on November 20, 2017: member

    e874478a42eded1c5952fc0407f04279dd4837c1 forgot to add node_network_limited.py to test_runner.py.

  4. jonasschnelli force-pushed on Nov 20, 2017
  5. jonasschnelli commented at 11:28 PM on November 20, 2017: contributor

    @fanquake! Thanks. Done.

  6. gmaxwell approved
  7. gmaxwell commented at 1:07 AM on November 23, 2017: contributor

    utACK

  8. laanwj commented at 11:10 AM on November 29, 2017: member

    utACK af05b92

  9. fanquake renamed this:
    Implement BIP159 NODE_NETWORK_LIMITED (pruned peers) *signalling only*
    Implement BIP159 NODE_NETWORK_LIMITED (pruned peers) *signaling only*
    on Nov 29, 2017
  10. theuni commented at 6:50 PM on November 30, 2017: member

    utACK af05b92591b72c34f8fa04a0917b3c1225e2417c

  11. in src/net_processing.cpp:1096 in af05b92591 outdated 
    1090 | @@ -1091,6 +1091,16 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
1091 |                      pfrom->fDisconnect = true;
1092 |                      send = false;
1093 |                  }
1094 | +                // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold
1095 | +                if (send && !pfrom->fWhitelisted && (
1096 | +                        (((pfrom->GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom->GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (chainActive.Tip()->nHeight - mi->second->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS) )
    TheBlueMatt commented at 3:49 PM on December 4, 2017:

    This needs a buffer beyond NODE_NETWORK_LIMITED_MIN_BLOCK. Because it gets a disconnect, one or two blocks should suffice.

  12. in src/net_processing.cpp:1100 in af05b92591 outdated 
    1095 | +                if (send && !pfrom->fWhitelisted && (
1096 | +                        (((pfrom->GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom->GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (chainActive.Tip()->nHeight - mi->second->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS) )
1097 | +                   )) {
1098 | +                    LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold from peer=%d\n", pfrom->GetId());
1099 | +
1100 | +                    //disconnect node
    TheBlueMatt commented at 3:50 PM on December 4, 2017:

    Can you fill out this comment a bit more? Something about how we could otherwise stall a node if we were just finishing IBD and announced blocks to our peers which they wanted?

  13. in test/functional/node_network_limited.py:46 in af05b92591 outdated 
      41 | +            node.wait_for_block(int(blockhash, 16), 3)
  42 | +            res = True
  43 | +        except:
  44 | +            pass
  45 | +        assert_equal(res, result)
  46 | +        self.nodes[0].disconnect_p2ps()
    TheBlueMatt commented at 3:53 PM on December 4, 2017:

    Can you move this into the try block? This checks that the node disconnected us, we didn't just timeout in the fail case.

  14. Add NODE_NETWORK_LIMITED flags and min block amount constants 7caba38568
  15. Always set NODE_NETWORK_LIMITED bit
    The current pruning implementation does ensure to always conform to BIP159
    27df193efd
  16. Avoid leaking the prune height through getdata (fingerprinting countermeasure) bd09416524
  17. jonasschnelli force-pushed on Dec 5, 2017
  18. jonasschnelli commented at 9:21 PM on December 5, 2017: contributor

    Fixed points reported by @TheBlueMatt. Had to rebase and therefor amend changed the fixes. The only code change is the two blocks buffer extension for possible race conditions.

  19. [QA] Add node_network_limited test e054d0e532
  20. [Doc] Update bip.md, add support for BIP 159 de74c62583
  21. in test/functional/node_network_limited.py:43 in d57a4e51a7 outdated 
      38 | +        # check if we the peer sends us the block
  39 | +        res = False
  40 | +        try:
  41 | +            node.wait_for_block(int(blockhash, 16), 3)
  42 | +            res = True
  43 | +            assert_equal(res, result)
    TheBlueMatt commented at 9:51 PM on December 6, 2017:

    It seems we never check anything if we got disconnected, I guess this check needs to move to after the except?

  22. jonasschnelli force-pushed on Dec 7, 2017
  23. jonasschnelli commented at 7:14 AM on December 7, 2017: contributor

    Improved the disconnect test (amend commit).

  24. TheBlueMatt commented at 3:54 PM on December 8, 2017: member

    utACK de74c625833bba8d8171a2d0dd6ede2e9d5da88b

  25. jonasschnelli commented at 9:10 PM on December 8, 2017: contributor

    @laanwj, @gmaxwell: mind to re-ack?

  26. laanwj commented at 7:38 AM on December 9, 2017: member

    utACK de74c62

  27. laanwj merged this on Dec 9, 2017
  28. laanwj closed this on Dec 9, 2017
  29. laanwj referenced this in commit 59d3dc85b6 on Dec 9, 2017
  30. jnewbery commented at 6:42 PM on December 11, 2017: member

    test is racy:

    node_network_limited.py failed, Duration: 61 s

stdout:
2017-12-11 18:39:35.906000 TestFramework (INFO): Initializing test directory /tmp/user/1000/bitcoin_test_runner_20171211_183839/node_network_limited_227
2017-12-11 18:39:36.171000 TestFramework.mininode (INFO): Connecting to Bitcoin Node: 127.0.0.1:12816
2017-12-11 18:39:36.498000 TestFramework.mininode (INFO): Connecting to Bitcoin Node: 127.0.0.1:12816
2017-12-11 18:39:36.604000 TestFramework.mininode (INFO): Connecting to Bitcoin Node: 127.0.0.1:12816
2017-12-11 18:39:36.757000 TestFramework.mininode (INFO): Connecting to Bitcoin Node: 127.0.0.1:12816
2017-12-11 18:40:36.768000 TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 118, in main
    self.run_test()
  File "/home/ubuntu/bitcoin/test/functional/node_network_limited.py", line 65, in run_test
    self.tryGetBlockViaGetData(blocks[0], True) #first block outside of the 288+2 limit
  File "/home/ubuntu/bitcoin/test/functional/node_network_limited.py", line 33, in tryGetBlockViaGetData
    node.wait_for_verack()
  File "/home/ubuntu/bitcoin/test/functional/test_framework/mininode.py", line 373, in wait_for_verack
    wait_until(test_function, timeout=timeout, lock=mininode_lock)
  File "/home/ubuntu/bitcoin/test/functional/test_framework/util.py", line 222, in wait_until
    assert_greater_than(timeout, time.time())
  File "/home/ubuntu/bitcoin/test/functional/test_framework/util.py", line 42, in assert_greater_than
    raise AssertionError("%s <= %s" % (str(thing1), str(thing2)))
AssertionError: 1513017636.75866 <= 1513017636.7688699
2017-12-11 18:40:36.769000 TestFramework (INFO): Stopping nodes
2017-12-11 18:40:36.931000 TestFramework (WARNING): Not cleaning up dir /tmp/user/1000/bitcoin_test_runner_20171211_183839/node_network_limited_227
2017-12-11 18:40:36.931000 TestFramework (ERROR): Test failed. Test logging available at /tmp/user/1000/bitcoin_test_runner_20171211_183839/node_network_limited_227/test_framework.log
2017-12-11 18:40:36.931000 TestFramework (ERROR): Hint: Call /home/ubuntu/bitcoin/test/functional/combine_logs.py '/tmp/user/1000/bitcoin_test_runner_20171211_183839/node_network_limited_227' to consolidate all logs

    fix is here: https://github.com/jnewbery/bitcoin/tree/improve_node_network_test . I'll open a PR.

  31. jonasschnelli commented at 6:43 PM on December 11, 2017: contributor

    Thanks @jnewbery!

  32. MarcoFalke referenced this in commit 18a1bbad98 on Dec 19, 2017
  33. PastaPastaPasta referenced this in commit 399155e6e1 on Jan 26, 2020
  34. PastaPastaPasta referenced this in commit a0709c490b on Feb 13, 2020
  35. PastaPastaPasta referenced this in commit 5d7dc51092 on Feb 27, 2020
  36. PastaPastaPasta referenced this in commit f175a2e351 on Feb 27, 2020
  37. PastaPastaPasta referenced this in commit ef5919f666 on Apr 4, 2020
  38. PastaPastaPasta referenced this in commit 561451810e on Apr 5, 2020
  39. ckti referenced this in commit 2bcba6fb76 on Mar 28, 2021
  40. ckti referenced this in commit 3dbf94324e on Mar 28, 2021
  41. gades referenced this in commit f29be0db44 on Jun 30, 2021
  42. gades referenced this in commit bb8fc00969 on Jun 30, 2021
  43. DrahtBot locked this on Sep 8, 2021
Contributors
jonasschnellifanquakegmaxwelllaanwjtheuniTheBlueMattjnewbery
Labels
P2P
Linked (view graph)
#10387 Eventually connect to NODE_NETWORK_LIMITED peers#11867 Improve node network test
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: 2026-04-13 21:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me