Eventually connect to NODE_NETWORK_LIMITED peers #10387

jonasschnelli · 2017-05-11T15:08:49Z

Eventually connect to peers signalling NODE_NETWORK_LIMITED if we are out of IBD.
Accept and relay NODE_NETWORK_LIMITED peers in addrman.

jonasschnelli · 2017-08-08T20:53:51Z

Rebased, overhauled and removed WIP tag.
Conforms now to BIP159.
Added 0.16 milestone tag.

jonasschnelli · 2017-08-11T19:34:37Z

~~Added a fingerprinting counter-measure which protects from leaking the prune depth by finding it through getdata requests (21c347c8c1a9ee4803b8f8f124185c218e01db76).~~

fanquake · 2017-09-26T02:46:55Z

src/net_processing.cpp

@@ -1006,6 +1020,13 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
                    pfrom->fDisconnect = true;
                    send = false;
                }
+                // Avoid leaking prune-height by never send blocks below the NODE_NETWORK_LIMITED threshold


nit: sending

fanquake · 2017-09-26T02:48:51Z

src/validation.h

@@ -203,6 +203,8 @@ extern bool fPruneMode;
 extern uint64_t nPruneTarget;
 /** Block files containing a block-height within MIN_BLOCKS_TO_KEEP of chainActive.Tip() will not be pruned. */
 static const unsigned int MIN_BLOCKS_TO_KEEP = 288;
+/** Minimum blocks to must be available to signal NODE_NETWORK_LIMITED */


s/to must be available/required

jonasschnelli · 2017-09-28T04:10:43Z

Fixed travis trailing whitespace in test issue.

TheBlueMatt

I think this PR is significantly needlessly over-complicated (mostly we dont allow nodes to go below 288 blocks, no need to check for it in so many places and update our services accordingly). Excited to see this go in, though.

TheBlueMatt · 2017-09-28T22:35:51Z

src/net_processing.h

+ * @param[in]   interrupt       Interrupt condition for processing threads
+ * @return                      True if there is more work to be done
+ */
+bool SendMessages(CNode* pto, CConnman& connman, const std::atomic<bool>& interrupt);


Looks like a rebase error - these functions were moved to the PeerLogicValidation.

TheBlueMatt · 2017-09-28T22:38:53Z

src/net_processing.cpp

+        nLocalServices = ServiceFlags(nLocalServices & ~NODE_NETWORK);
+
+        // don't signal NODE_NETWORK_LIMITED during initial block download
+        if (IsInitialBlockDownload()) {


Hmm, this feels pretty strange to me. Service bits indicate a commitment to provide certain services, not your current state. More importantly, IBD is always neccessarily fuzzy - we can't know how "in sync" we are.

TheBlueMatt · 2017-09-28T22:42:05Z

src/net_processing.cpp

+        // currently, the prune mode can't be changed during runtime
+        // implement the service bit switch for possible future flexibility
+        nLocalServices = ServiceFlags(nLocalServices | NODE_NETWORK);
+        nLocalServices = ServiceFlags(nLocalServices & ~NODE_NETWORK_LIMITED);


Why? The BIP seems somewhat unclear here, but I'd stronlgy prefer that NODE_NETWORK_LIMITED simply be always set to indicate that we can always provide the most recent 288 blocks, irrespective of if we're pruned or not.

TheBlueMatt · 2017-09-28T22:45:31Z

src/net_processing.cpp

+        }
+
+        // check how many blocks we have available (backwards from tip)
+        // don't go futher down then chainActive.Height()-NODE_NETWORK_LIMITED


This seems entirely redundant - even if by super rare chance we end up due to reorg short of 288 blocks, we won't be very short of 288 blocks, and will be caught back up within the next block or two, might as well just always set NODE_NETWORK_LIMITED (you can static_assert MIN_BLOCKS_TO_KEEP is at least 288 if you really want to have a check).

TheBlueMatt · 2017-09-28T22:52:04Z

src/protocol.h

-    // set by all Bitcoin Core nodes, and is unset by SPV clients or other peers that just want
-    // network services but don't provide them.
+    // set by all Bitcoin Core non pruned nodes, and is unset by SPV clients or other peers
+    // that just want network services but don't provide them.


If you're gonna update this maybe overhaul the text? The concept of "want network services but don't provide them" doesn't really make sense anymore (especially with NODE_NETWORK_LIMITED). Maybe just say "indicates peer is capable of serving the full historical block chain".

TheBlueMatt · 2017-09-28T22:55:49Z

src/net.cpp

@@ -2672,6 +2672,11 @@ ServiceFlags CConnman::GetLocalServices() const
    return nLocalServices;
 }

+void CConnman::SetLocalServices(ServiceFlags flagsIn)


I'm entirely unconvinced NODE_NETWORK_LIMITED needs this (and would prefer to avoid adding it until we need it, though its no big deal to add it).

TheBlueMatt · 2017-09-28T22:56:51Z

src/rpc/blockchain.cpp

+    // check if we need to change the local service flags in order to signal NODE_NETWORK_LIMITED flags
+    if(g_connman) {
+        ServiceFlags nFlags = g_connman->GetLocalServices();
+        CheckLocalServices(nFlags);


Why do we need to do this? You can't prune deeper than 128 and NODE_NETWORK is already unset for manually-pruned nodes.

TheBlueMatt · 2017-09-28T23:01:50Z

src/net_processing.cpp

@@ -1020,6 +1020,13 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
                    pfrom->fDisconnect = true;
                    send = false;
                }
+                // Avoid leaking prune-height by never send blocks below the NODE_NETWORK_LIMITED threshold
+                if (send && !pfrom->fWhitelisted && (
+                        (((connman->GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && (chainActive.Tip()->nHeight - mi->second->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS) )


This should use pfrom->GetLocalServices() not connman->GetLocalServices() - our decisions should be based on what we told the peer we support, not what we would tell a new peer to support.

jonasschnelli · 2017-09-29T02:56:13Z

~~Thanks for the review @TheBlueMatt. I agree with all you points and most things can now go away since the NODE_NETWORK_LIMITED_HIGH flag (1152 blocks minimum) is gone.~~

~~Will overhaul and simplify it now.~~

jonasschnelli · 2017-10-01T04:22:37Z

Simplified the PR and addressed @TheBlueMatt and @fanquake points.

TheBlueMatt · 2017-10-03T14:02:04Z

src/init.cpp

@@ -1590,6 +1590,8 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
    if (fPruneMode) {
        LogPrintf("Unsetting NODE_NETWORK on prune mode\n");
        nLocalServices = ServiceFlags(nLocalServices & ~NODE_NETWORK);
+        // always set NODE_NETWORK_LIMITED (BIP159) in pruned mode
+        nLocalServices = ServiceFlags(nLocalServices | NODE_NETWORK_LIMITED);


Hmm, I thought we were just going to alays set this? Why bother only setting it when pruned - a node that is not pruned is also capable of serving the last 288 blocks, which is the way I read the definition of NODE_NETWORK_LIMITED.

I agree. Came to complain about the same thing.

NODE_NETWORK implies NODE_NETWORK_LIMITED, so _LIMITED should always be set.
Otherwise, we're introducing negative service flags, which is a bad idea imo.

Okay. Agree... will change it.

theuni · 2017-10-03T17:56:20Z

src/net_processing.cpp

@@ -1302,7 +1312,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
            pfrom->cleanSubVer = cleanSubVer;
        }
        pfrom->nStartingHeight = nStartingHeight;
-        pfrom->fClient = !(nServices & NODE_NETWORK);
+        pfrom->fClient = (!(nServices & NODE_NETWORK) && !(nServices & NODE_NETWORK_LIMITED));


I'd rather not use fClient as a proxy for NODE_NETWORK_LIMITED. It deserves its own classification.

Yes, this needs its own bool so that fClient gets checked when testing for our initial headers-sync peer. Then, fPreferredDownload is only set to false if we're still in IBD.

That way, we request from this peer as if they were a regular full node as long as we're synced.

IMO we want pruned peers for header sync... but right, we need a protection to not request blocks from limited peers during IBD.
What about adding an fLimitedNode bool and check it at the point we populate vGetData with block requests?

Seems also simpler and more constant (NODE_NETWORK_LIMITED is not really a "client").

I changed this PR and applied my approach which I have written above.

theuni · 2017-10-03T18:07:24Z

test/functional/test_framework/mininode.py

        self.nServices = 1
        self.pchReserved = b"\x00" * 10 + b"\xff" * 2
        self.ip = "0.0.0.0"
        self.port = 0

-    def deserialize(self, f):
+    def deserialize(self, f, without_time=False):


nit: a default of without_foo = False isn't no bad thing for nobody.

with_time=True please :)

jimpo

This change prefers connecting to full nodes rather than pruned nodes even if fully synced. Why is that the desired approach?

I find that the required services/relevant services/alternative services stuff is getting confusing. If I'm reading it right, the order of preference is:

NODE_NETWORK & NODE_WITNESS
NODE_NETWORK_LIMITED & NODE_WITNESS (if !IsInitialDownload)
NODE_NETWORK

It seems like there should be a way to simplify this, where the ConnManager tracks the ranked preferences of peer services with the nOutbound quota and nTries threshold for each.

jimpo · 2017-10-03T17:47:51Z

src/protocol.h

@@ -267,6 +266,9 @@ enum ServiceFlags : uint64_t {
    // NODE_XTHIN means the node supports Xtreme Thinblocks
    // If this is turned off then the node will not service nor make xthin requests
    NODE_XTHIN = (1 << 4),
+    // NODE_NETWORK_LIMITED means the same as NODE_NETWORK with the limitation of only
+    // serving the last 288 (2 day) blocks


Perhaps reference BIP 159 in the comment here for additional explanation?

jimpo · 2017-10-03T17:51:01Z

src/net_processing.cpp

+
+    // update service flags
+    {
+        LOCK(cs_main);


We can avoid obtaining this lock if the alternative services is already NODE_NETWORK_LIMITED.

I think we should not connect to NODE_NETWORK_LIMITED peers if IBD is still ongoing. NODE_NETWORK_LIMITED implementations really should make sure to not serve deeper then >288 blocks (fingerprinted). Seems useless to me connect until clear IBD.

Yeah, I understand that, but unless I'm missing something, this code is run every time there is a new block after IBD is done instead of happening only once. It seems possible to change the logic to exit before obtaining the lock if AlternativeServices has already been switched from NODE_NETWORK to NODE_NETWORK_LIMITED.

jimpo · 2017-10-03T18:08:14Z

src/net.cpp

+            // only consider nodes matching alternative and missing relevant services after 10 failed attempts and only if less than a fourth of the outbound are up.
+            if ( ((addr.nServices & nAlternativeServices) == nAlternativeServices) && (addr.nServices & nRelevantServices) != nRelevantServices && (nTries < 20 || nOutbound >= (nMaxOutbound >> 2)))
+                continue;
+
            // only consider nodes missing relevant services after 40 failed attempts and only if less than half the outbound are up.
            ServiceFlags nRequiredServices = nRelevantServices;
            if (nTries >= 40 && nOutbound < (nMaxOutbound >> 1)) {


In this clause, should nRequiredServices get set to NODE_NETWORK_LIMITED if peer is in sync? Essentially, replace REQUIRED_SERVICES below with a value that goes from NODE_NETWORK to NODE_NETWORK_LIMITED when peer is out of IBD.

jonasschnelli · 2018-02-09T09:02:11Z

Rebased.

laanwj · 2018-02-14T16:15:37Z

utACK 3f56df5

laanwj · 2018-02-14T16:19:39Z

src/protocol.h

@@ -277,6 +278,7 @@ enum ServiceFlags : uint64_t {
    // BIP process.
 };

+extern std::atomic<bool> g_initial_block_download_completed;


I don't like that we need to introduce a global here, especially one that has to be accessible externally (in the header file).
Any drawback to moving GetDesirableServiceFlags to the .cpp instead?

jonasschnelli · 2018-02-17T10:30:41Z

Added a commit that hides the new protocol.h/.cpp global (g_initial_block_download_completed) behinds a setter (@laanwj / #10387 (comment)).

eb91835 Add setter for g_initial_block_download_completed (Jonas Schnelli) 3f56df5 [QA] add NODE_NETWORK_LIMITED address relay and sync test (Jonas Schnelli) 158e1a6 [QA] fix mininode CAddress ser/deser (Jonas Schnelli) fa999af [QA] Allow addrman loopback tests (add debug option -addrmantest) (Jonas Schnelli) 6fe57bd Connect to peers signaling NODE_NETWORK_LIMITED when out-of-IBD (Jonas Schnelli) 31c45a9 Accept addresses with NODE_NETWORK_LIMITED flag (Jonas Schnelli) Pull request description: Eventually connect to peers signalling NODE_NETWORK_LIMITED if we are out of IBD. Accept and relay NODE_NETWORK_LIMITED peers in addrman. Tree-SHA512: 8a238fc97f767f81cae1866d6cc061390f23a72af4a711d2f7158c77f876017986abb371d213d1c84019eef7be4ca951e8e6f83fda36769c4e1a1d763f787037

@theuni

…suite. 15f5d3b Switch DNSSeed-needed metric to any-automatic-nodes, not services (Matt Corallo) 5ee88b4 Clarify docs for requirements/handling of addnode/connect nodes (Matt Corallo) 57edc0b Rename fAddnode to a more-descriptive "manual_connection" (Matt Corallo) 4440710 Replace relevant services logic with a function suite. (Matt Corallo) Pull request description: This was mostly written as a way to clean things up so that the NETWORK_LIMITED PR (bitcoin#10387) can be simplified a ton, but its also a nice standalone cleanup that will also require a bit of review because it tweaks a lot of stuff across net. The new functions are fine in protocol.h right now since they're straight-forward, but after NETWORK_LIMITED will really want to move elsewhere after @theuni moves the nServices-based selection to addrman from connman. Adds HasAllRelevantServices and GetRelevantServices, which check for NETWORK|WITNESS. This changes the following: * Removes nRelevantServices from CConnman, disconnecting it a bit more from protocol-level logic. * Replaces our sometimes-connect-to-!WITNESS-nodes logic with simply always requiring WITNESS|NETWORK for outbound non-feeler connections (feelers still only require NETWORK). * This has the added benefit of removing nServicesExpected from CNode - instead letting net_processing's VERSION message handling simply check HasAllRelevantServices. * This implies we believe WITNESS nodes to continue to be a significant majority of nodes on the network, but also because we cannot sync properly from !WITNESS nodes, it is strange to continue using our valuable outbound slots on them. * In order to prevent this change from preventing connection to -connect= nodes which have !WITNESS, -connect nodes are now given the "addnode" flag. This also allows outbound connections to !NODE_NETWORK nodes for -connect nodes (which was already true of addnodes). * Has the (somewhat unintended) consequence of changing one of the eviction metrics from the same sometimes-connect-to-!WITNESS-nodes metric to requiring HasRelevantServices. This should make NODE_NETWORK_LIMITED much simpler to implement. Tree-SHA512: 90606896c86cc5da14c77843b16674a6a012065e7b583d76d1c47a18215358abefcbab44ff4fab3fadcd39aa9a42d4740c6dc8874a58033bdfc8ad3fb5c649fc

@theuni

…suite. 15f5d3b Switch DNSSeed-needed metric to any-automatic-nodes, not services (Matt Corallo) 5ee88b4 Clarify docs for requirements/handling of addnode/connect nodes (Matt Corallo) 57edc0b Rename fAddnode to a more-descriptive "manual_connection" (Matt Corallo) 4440710 Replace relevant services logic with a function suite. (Matt Corallo) Pull request description: This was mostly written as a way to clean things up so that the NETWORK_LIMITED PR (bitcoin#10387) can be simplified a ton, but its also a nice standalone cleanup that will also require a bit of review because it tweaks a lot of stuff across net. The new functions are fine in protocol.h right now since they're straight-forward, but after NETWORK_LIMITED will really want to move elsewhere after @theuni moves the nServices-based selection to addrman from connman. Adds HasAllRelevantServices and GetRelevantServices, which check for NETWORK|WITNESS. This changes the following: * Removes nRelevantServices from CConnman, disconnecting it a bit more from protocol-level logic. * Replaces our sometimes-connect-to-!WITNESS-nodes logic with simply always requiring WITNESS|NETWORK for outbound non-feeler connections (feelers still only require NETWORK). * This has the added benefit of removing nServicesExpected from CNode - instead letting net_processing's VERSION message handling simply check HasAllRelevantServices. * This implies we believe WITNESS nodes to continue to be a significant majority of nodes on the network, but also because we cannot sync properly from !WITNESS nodes, it is strange to continue using our valuable outbound slots on them. * In order to prevent this change from preventing connection to -connect= nodes which have !WITNESS, -connect nodes are now given the "addnode" flag. This also allows outbound connections to !NODE_NETWORK nodes for -connect nodes (which was already true of addnodes). * Has the (somewhat unintended) consequence of changing one of the eviction metrics from the same sometimes-connect-to-!WITNESS-nodes metric to requiring HasRelevantServices. This should make NODE_NETWORK_LIMITED much simpler to implement. Tree-SHA512: 90606896c86cc5da14c77843b16674a6a012065e7b583d76d1c47a18215358abefcbab44ff4fab3fadcd39aa9a42d4740c6dc8874a58033bdfc8ad3fb5c649fc

@theuni

…suite. 15f5d3b Switch DNSSeed-needed metric to any-automatic-nodes, not services (Matt Corallo) 5ee88b4 Clarify docs for requirements/handling of addnode/connect nodes (Matt Corallo) 57edc0b Rename fAddnode to a more-descriptive "manual_connection" (Matt Corallo) 4440710 Replace relevant services logic with a function suite. (Matt Corallo) Pull request description: This was mostly written as a way to clean things up so that the NETWORK_LIMITED PR (bitcoin#10387) can be simplified a ton, but its also a nice standalone cleanup that will also require a bit of review because it tweaks a lot of stuff across net. The new functions are fine in protocol.h right now since they're straight-forward, but after NETWORK_LIMITED will really want to move elsewhere after @theuni moves the nServices-based selection to addrman from connman. Adds HasAllRelevantServices and GetRelevantServices, which check for NETWORK|WITNESS. This changes the following: * Removes nRelevantServices from CConnman, disconnecting it a bit more from protocol-level logic. * Replaces our sometimes-connect-to-!WITNESS-nodes logic with simply always requiring WITNESS|NETWORK for outbound non-feeler connections (feelers still only require NETWORK). * This has the added benefit of removing nServicesExpected from CNode - instead letting net_processing's VERSION message handling simply check HasAllRelevantServices. * This implies we believe WITNESS nodes to continue to be a significant majority of nodes on the network, but also because we cannot sync properly from !WITNESS nodes, it is strange to continue using our valuable outbound slots on them. * In order to prevent this change from preventing connection to -connect= nodes which have !WITNESS, -connect nodes are now given the "addnode" flag. This also allows outbound connections to !NODE_NETWORK nodes for -connect nodes (which was already true of addnodes). * Has the (somewhat unintended) consequence of changing one of the eviction metrics from the same sometimes-connect-to-!WITNESS-nodes metric to requiring HasRelevantServices. This should make NODE_NETWORK_LIMITED much simpler to implement. Tree-SHA512: 90606896c86cc5da14c77843b16674a6a012065e7b583d76d1c47a18215358abefcbab44ff4fab3fadcd39aa9a42d4740c6dc8874a58033bdfc8ad3fb5c649fc

…ers) *signaling only* de74c62 [Doc] Update bip.md, add support for BIP 159 (Jonas Schnelli) e054d0e [QA] Add node_network_limited test (Jonas Schnelli) bd09416 Avoid leaking the prune height through getdata (fingerprinting countermeasure) (Jonas Schnelli) 27df193 Always set NODE_NETWORK_LIMITED bit (Jonas Schnelli) 7caba38 Add NODE_NETWORK_LIMITED flags and min block amount constants (Jonas Schnelli) Pull request description: Extracted from bitcoin#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 bitcoin#10387). Tree-SHA512: e3218eb4789a9320b0f42dc10f62d30c13c49bdef00443fbe653bee22933477adcfc1cf8f6a95269324560b5721203ed41f3c5e2dd8a98ec2791f6a9d8346b1a

eb91835 Add setter for g_initial_block_download_completed (Jonas Schnelli) 3f56df5 [QA] add NODE_NETWORK_LIMITED address relay and sync test (Jonas Schnelli) 158e1a6 [QA] fix mininode CAddress ser/deser (Jonas Schnelli) fa999af [QA] Allow addrman loopback tests (add debug option -addrmantest) (Jonas Schnelli) 6fe57bd Connect to peers signaling NODE_NETWORK_LIMITED when out-of-IBD (Jonas Schnelli) 31c45a9 Accept addresses with NODE_NETWORK_LIMITED flag (Jonas Schnelli) Pull request description: Eventually connect to peers signalling NODE_NETWORK_LIMITED if we are out of IBD. Accept and relay NODE_NETWORK_LIMITED peers in addrman. Tree-SHA512: 8a238fc97f767f81cae1866d6cc061390f23a72af4a711d2f7158c77f876017986abb371d213d1c84019eef7be4ca951e8e6f83fda36769c4e1a1d763f787037 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/init.cpp # src/protocol.h # test/functional/node_network_limited.py

…#3417) * Merge bitcoin#10387: Eventually connect to NODE_NETWORK_LIMITED peers eb91835 Add setter for g_initial_block_download_completed (Jonas Schnelli) 3f56df5 [QA] add NODE_NETWORK_LIMITED address relay and sync test (Jonas Schnelli) 158e1a6 [QA] fix mininode CAddress ser/deser (Jonas Schnelli) fa999af [QA] Allow addrman loopback tests (add debug option -addrmantest) (Jonas Schnelli) 6fe57bd Connect to peers signaling NODE_NETWORK_LIMITED when out-of-IBD (Jonas Schnelli) 31c45a9 Accept addresses with NODE_NETWORK_LIMITED flag (Jonas Schnelli) Pull request description: Eventually connect to peers signalling NODE_NETWORK_LIMITED if we are out of IBD. Accept and relay NODE_NETWORK_LIMITED peers in addrman. Tree-SHA512: 8a238fc97f767f81cae1866d6cc061390f23a72af4a711d2f7158c77f876017986abb371d213d1c84019eef7be4ca951e8e6f83fda36769c4e1a1d763f787037 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/init.cpp # src/protocol.h # test/functional/node_network_limited.py * remove witness Signed-off-by: Pasta <pasta@dashboost.org> * fix test expecting witness flag Signed-off-by: Pasta <pasta@dashboost.org> Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>

…ers) *signaling only* de74c62 [Doc] Update bip.md, add support for BIP 159 (Jonas Schnelli) e054d0e [QA] Add node_network_limited test (Jonas Schnelli) bd09416 Avoid leaking the prune height through getdata (fingerprinting countermeasure) (Jonas Schnelli) 27df193 Always set NODE_NETWORK_LIMITED bit (Jonas Schnelli) 7caba38 Add NODE_NETWORK_LIMITED flags and min block amount constants (Jonas Schnelli) Pull request description: Extracted from bitcoin#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 bitcoin#10387). Tree-SHA512: e3218eb4789a9320b0f42dc10f62d30c13c49bdef00443fbe653bee22933477adcfc1cf8f6a95269324560b5721203ed41f3c5e2dd8a98ec2791f6a9d8346b1a

…dashpay#3417) * Merge bitcoin#10387: Eventually connect to NODE_NETWORK_LIMITED peers eb91835 Add setter for g_initial_block_download_completed (Jonas Schnelli) 3f56df5 [QA] add NODE_NETWORK_LIMITED address relay and sync test (Jonas Schnelli) 158e1a6 [QA] fix mininode CAddress ser/deser (Jonas Schnelli) fa999af [QA] Allow addrman loopback tests (add debug option -addrmantest) (Jonas Schnelli) 6fe57bd Connect to peers signaling NODE_NETWORK_LIMITED when out-of-IBD (Jonas Schnelli) 31c45a9 Accept addresses with NODE_NETWORK_LIMITED flag (Jonas Schnelli) Pull request description: Eventually connect to peers signalling NODE_NETWORK_LIMITED if we are out of IBD. Accept and relay NODE_NETWORK_LIMITED peers in addrman. Tree-SHA512: 8a238fc97f767f81cae1866d6cc061390f23a72af4a711d2f7158c77f876017986abb371d213d1c84019eef7be4ca951e8e6f83fda36769c4e1a1d763f787037 Signed-off-by: Pasta <pasta@dashboost.org> # Conflicts: # src/init.cpp # src/protocol.h # test/functional/node_network_limited.py * remove witness Signed-off-by: Pasta <pasta@dashboost.org> * fix test expecting witness flag Signed-off-by: Pasta <pasta@dashboost.org> Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>

…ers) *signaling only* de74c62 [Doc] Update bip.md, add support for BIP 159 (Jonas Schnelli) e054d0e [QA] Add node_network_limited test (Jonas Schnelli) bd09416 Avoid leaking the prune height through getdata (fingerprinting countermeasure) (Jonas Schnelli) 27df193 Always set NODE_NETWORK_LIMITED bit (Jonas Schnelli) 7caba38 Add NODE_NETWORK_LIMITED flags and min block amount constants (Jonas Schnelli) Pull request description: Extracted from bitcoin#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 bitcoin#10387). Tree-SHA512: e3218eb4789a9320b0f42dc10f62d30c13c49bdef00443fbe653bee22933477adcfc1cf8f6a95269324560b5721203ed41f3c5e2dd8a98ec2791f6a9d8346b1a

jonasschnelli added the P2P label May 11, 2017

jonasschnelli force-pushed the 2017/05/node_network_limited branch from b06145f to 9bacfb6 Compare May 12, 2017 07:38

jonasschnelli force-pushed the 2017/05/node_network_limited branch from 9bacfb6 to c8a7182 Compare August 8, 2017 20:45

jonasschnelli changed the title ~~[WIP] Define and signal NODE_NETWORK_LIMITED (pruned peers)~~ Implement BIP159, define and signal NODE_NETWORK_LIMITED (pruned peers) Aug 8, 2017

jonasschnelli added this to the 0.16.0 milestone Aug 8, 2017

jonasschnelli force-pushed the 2017/05/node_network_limited branch 4 times, most recently from cf5fbac to e80484e Compare August 11, 2017 19:09

jonasschnelli force-pushed the 2017/05/node_network_limited branch 2 times, most recently from be2b61d to 9729092 Compare August 12, 2017 09:25

jonasschnelli force-pushed the 2017/05/node_network_limited branch 2 times, most recently from 995f717 to 3fedeac Compare September 23, 2017 02:07

jonasschnelli mentioned this pull request Sep 23, 2017

Remove second service bit from NODE_NETWORK_LIMITED/BIP159 bitcoin/bips#592

Merged

jonasschnelli added the Needs release note label Sep 23, 2017

fanquake reviewed Sep 26, 2017

View reviewed changes

jonasschnelli force-pushed the 2017/05/node_network_limited branch from 3fedeac to b09695d Compare September 28, 2017 04:08

TheBlueMatt reviewed Sep 28, 2017

View reviewed changes

jonasschnelli force-pushed the 2017/05/node_network_limited branch 2 times, most recently from a716bfb to abd60f6 Compare October 1, 2017 04:20

TheBlueMatt reviewed Oct 3, 2017

View reviewed changes

theuni reviewed Oct 3, 2017

View reviewed changes

jimpo reviewed Oct 3, 2017

View reviewed changes

jonasschnelli added 5 commits February 9, 2018 19:56

Accept addresses with NODE_NETWORK_LIMITED flag

31c45a9

Connect to peers signaling NODE_NETWORK_LIMITED when out-of-IBD

6fe57bd

[QA] Allow addrman loopback tests (add debug option -addrmantest)

fa999af

[QA] fix mininode CAddress ser/deser

158e1a6

[QA] add NODE_NETWORK_LIMITED address relay and sync test

3f56df5

jonasschnelli force-pushed the 2017/05/node_network_limited branch from fca33fc to 3f56df5 Compare February 9, 2018 09:02

laanwj reviewed Feb 14, 2018

View reviewed changes

Add setter for g_initial_block_download_completed

eb91835

laanwj merged commit eb91835 into bitcoin:master Mar 1, 2018

maflcko mentioned this pull request Mar 1, 2018

TODO for release notes 0.17.0 #12391

Closed

fanquake removed the Needs release note label Mar 20, 2019

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Eventually connect to NODE_NETWORK_LIMITED peers #10387

Eventually connect to NODE_NETWORK_LIMITED peers #10387

Uh oh!

Conversation

jonasschnelli commented May 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli commented Aug 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli commented Aug 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Sep 28, 2017

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Sep 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli commented Oct 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jimpo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jimpo Oct 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Feb 9, 2018

Uh oh!

laanwj commented Feb 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Feb 17, 2018

Uh oh!

jonasschnelli commented May 11, 2017 •

edited

Loading

jonasschnelli commented Aug 8, 2017 •

edited

Loading

jonasschnelli commented Aug 11, 2017 •

edited

Loading

jonasschnelli commented Sep 29, 2017 •

edited

Loading

jimpo Oct 3, 2017 •

edited

Loading