Allow outbound & block-relay-only connections in functional tests (p2p, tests)

Host: jnewbery  -  PR author: amitiuttarwar

The PR branch HEAD was f51006b818 at the time of this review club meeting.


  • There are a variety of different types of P2P connections that Bitcoin Core nodes can have with peers. See the ConnectionType enum in src/net.h for the different types.

  • Until now, the Bitcoin Core functional test framework allowed only 2 types of P2P connections:
    • manual connections from one bitcoind instance to another. See the BitcoinTestFramework.connect_nodes() method, which calls the addnode RPC.
    • inbound connections made from the test framework to the bitcoind instance. See the TestNode.add_p2p_connection() method.
  • This week’s PR adds the ability to create 2 additional types of regtest P2P connections from the bitcoind instances to the functional test framework:
    • full outbound connections
    • block-relay-only outbound connections
  • The PR adds functionality to both the bitcoind C++ code and to the functional test framework Python code:
    • In bitcoind, a test-only addconnection RPC is added to allow opening full outbound or block-relay-only outbound connections in regtest.
    • In the functional test framework, TestNode.add_outbound_p2p_connection() is added to enable making either full outbound or block-relay-only outbound connections, along with P2PInterface.peer_accept_connection() to allow the test framework to accept and listen to incoming connections.

    We’ll focus mainly on the C++ code in this review club.


  1. What are Bitcoin Core’s different connection types?

  2. In the test framework, what is the difference between connecting two bitcoind nodes using connect_nodes() and using a P2PConnection object?

  3. Why would it be useful to create outbound or block-relay-only connections in the test framework?

  4. How does this PR enable creating outbound and block-relay-only connections in the test framework?

  5. Why do we make some RPC methods hidden, or “test only”?

  6. For each different type of connection, what is the maximum number of connections that Bitcoin Core will have by default?

  7. What are CSemaphore and CSemaphoreGrant used for in CConnman?

Meeting Log

  117:00 <jnewbery> #startmeeting
  217:00 <amiti> hi
  317:00 <emzy> hi
  417:00 <jnewbery> what time is it?
  517:00 <sergei-t> hi
  617:00 <willcl_ark> hi
  717:00 <elle> hi!
  817:00 <jnewbery> Review Club time!!!
  917:00 <stacie> hi
 1017:00 <andozw> hi
 1117:00 <ariard> hello
 1217:00 <troygiorshev> hi
 1317:00 <jnewbery> welcome all. Feel free to say hi to let everyone know you're here
 1417:00 <pinheadmz> hi
 1517:01 <lightlike> hi
 1617:01 <jnewbery> anyone here for the first time?
 1717:01 <nehan> hi
 1817:01 <murch> hi
 1917:01 <dappdever> hi
 2017:01 <jnewbery> anyone here for the 83rd time?
 2117:02 <pinheadmz> hm maybe ?
 2217:02 <willcl_ark> alas, not quite
 2317:02 <pinheadmz> wait how many have there been?
 2417:02 <murch> jnewbery: Have there been that many?
 2517:02 <pinheadmz> is this an overflow attack?
 2617:02 <pinheadmz> whats the modulus
 2717:02 <jnewbery> 83 so far
 2817:02 <murch> wow, neat.
 2917:02 <jnewbery> ok, notes and questions are in the normal place:
 3017:02 <pinheadmz> i bet only jnewbery can claim that record then
 3117:03 <jnewbery> pinheadmz: I've missed a couple
 3217:03 <emzy> I try to be here every time... :)
 3317:03 <jnewbery> There are changes to the C++ and Python code in that PR. We'll concentrate on the C++ changes in this meeting, but it's important to review the Python code too
 3417:04 <jnewbery> so if any of you love Python asyncio and networking code, that PR is your chance to shine
 3517:04 <jnewbery> who had time to review the C++ code this week? (y/n)
 3617:04 <sergei-t> y (resulting in a bunch of dumb questions!)
 3717:04 <emzy> n
 3817:04 <willcl_ark> A light review; most time was spent trying to familiarise with the existing code in this area...
 3917:05 <nehan> y lightly
 4017:05 <jnewbery> sergei-t: there are no dumb questions in Review Club
 4117:05 <murch> n
 4217:05 <amiti> the test framework additions are decently complex. I've tried to make the changes clear, but definitely welcome any feedback !
 4317:05 <troygiorshev> y light
 4417:05 <stacie> +1 for light review. Focused more on the concept of the PR as a whole
 4517:05 <jnewbery> ok, any initial thoughts on the PR? Concept ACKs/NACKs? Thoughts about the approach?
 4617:05 <lightlike> y (1 month ago though)
 4717:06 <dappdever> y
 4817:06 <sergei-t> if we add new connection types to the tests, why not feeler connections and addr_fetch connections?
 4917:07 <willcl_ark> Increasing test coverage is good?
 5017:07 <glozow> hi, sorry i'm late, y
 5117:07 <jnewbery> sergei-t: great question. Anyone else have thoughts about that? Perhaps also explain what feeler and addr_fetch connections are?
 5217:07 <troygiorshev> huge concept ACK! our test framework should try and match things as closely and completely as possible imo
 5317:07 <glozow> there's outbound-specific behavior we want to test, like banning
 5417:07 <stacie> Seems like a big win. Is there ever a reason to concept NACK increased test coverage?
 5517:07 <elle> cool pr! i was surprised that an rpc endpoint for adding specific connection types didnt already exist
 5617:08 <dappdever> jnewberry: does enabling these connections types open up a need for more functional tests to take advantage of this?
 5717:08 <amiti> sergei-t: good question, since outbound-full-relay & block-relay-only are the most common connections, I focused on those for this PR, but went a route that would allow for adding more connection types in the future
 5817:08 <jnewbery> dappdever: these connection types already existed, but we just had no way of testing them in our functional test framework
 5917:09 <ariard> sergei-t: intuitvely as those connections (feeler/addr_fetch) are short-lived we have less logic around them (e.g we don't have eviction for them)
 6017:09 <jnewbery> This question links in with the first questions from the review club notes: What are Bitcoin Core’s different connection types?
 6117:09 <dappdever> so perhaps a good follow-up activity would be authoring those functional tests?
 6217:10 <lightlike> stacie: i think there is - sometimes one would need invasive changes to the main code for better testing - that could be a NACK..
 6317:10 <dappdever> inbound, outbound_full_relay, manual, feeler, block_relay, addr_fetch :)
 6417:10 <stacie> I have a q about the manual connection type. Are these connections restricted to being full relay or block relay only? I’m assuming they default to full relay but could they be block relay only?
 6517:10 <jnewbery> dappdever: yes. This PR includes basic tests for outbound full relay and outbound block relay only tests, but more testing of our p2p functionality would definitely be useful
 6617:10 <stacie> lightlike - that's a good point!
 6717:10 <pinheadmz> jnewbery are anchor connections a thing yet? was that merged?
 6817:11 <willcl_ark> stacie: I think manual connections are only full_relay
 6917:11 <amiti> stacie: currently there are no manual block relay only connections, but I know this is something ariard is interested in :)
 7017:11 <ariard> pinheadmz: yeah it was merged but still appear as block-relay-only
 7117:11 <sergei-t> is it true that the only difference between full relay conns and manual conns is the reaction to their misbehavior?
 7217:11 <stacie> thanks for the clarification! willcl_ark & amiti
 7317:11 <jnewbery> pinheadmz: anchor connections are a thing, but that's not a distinct connection type. We save all our block-relay-only connections on shutdown
 7417:12 <ariard> amiti: because no addr-relay interferences :)
 7517:13 <jnewbery> stacie: test code needs to be maintained just the same as all other code, so I'd NACK it if it doesn't meet the quality bar or doesn't add enough value to justify the ongoing maintainance cost
 7617:14 <jnewbery> dappdever: yes, those are currently the six connection types. There's some very nice documentation for them here:
 7717:14 <stacie> jnewbery I hadn't thought of that, and it makes a lot of sense
 7817:15 <jnewbery> sergei-t: I think there are quite a few subtle behavior differences between manual connections and full relay connections, but I'd need to dig around in the code to find them
 7917:16 <jnewbery> but in their high-level behavior, they're the same: they both relay txs, blocks and addrs
 8017:16 <amiti> sergei-t: I don't think thats the only difference. for example, if you restart your node, you'll reconnect to your manually added peers. whereas you probably wont reconnect to the same full-relay outbound peers.
 8117:16 <jnewbery> Next question! In the test framework, what is the difference between connecting two bitcoind nodes and using a P2P connection?
 8217:16 <troygiorshev> sergei-t: how and when they are connectd is probably too important to ignore iirc
 8317:17 <sergei-t> amiti: this reconnection policy resembles LN node behavior regarding nodes I share channels with :)
 8417:17 <pinheadmz> i think the add_p2p_connection method uses a pyhton "fake" node wheras connecting two bitcoind nodes.... is connecting two bitcoind nodes
 8517:17 <willcl_ark> Connecting two nodes directly connects them via their listening p2p ports whereas using passes the connection through the P2PConnection class, which lets you instrument and act upon each message exchanged.
 8617:18 <sergei-t> jnewbery: I don't quite understand this question :( "using a P2P connection" is not connecting two nodes?
 8717:19 <sergei-t> I guess, the testing mechanics is just unclear to me: when we say "connect from testing framework to the node"... what does actually happen?
 8817:19 <jnewbery> sergei-t: P2PConnection is a Python class that we use in the test framework:
 8917:19 <glozow> pinheadmz add_p2p_connection is connecting a test framework object, not connecting bitcoind nodes
 9017:19 <glozow> test framework p2p object*
 9117:19 <ariard> sergei-t: I wouldn't confuse the p2p layer with LN link-layer (`channel_reestablish`)
 9217:19 <dappdever> Is there an example test for connecting two bitcoind nodes directly? I understood the P2P connection code fairly well but was unsure what to compare it to
 9317:20 <lightlike> sergei-t: The old terminology (which was changed recently) was "connect to a (python) mininode" instead of "Using a p2p connection".
 9417:20 <sergei-t> jnewbery: that class is "A low-level connection object to a node's P2P interface" - which node?
 9517:20 <pinheadmz> glozow thats what i meant! a "fake" node yeah
 9617:20 <willcl_ark> it's more like a passthrough/man-in-the-middle as I see it
 9717:21 <hmrawal> can we use the RPC connection in a test framework ? or are there any other connection methods which can be used in a test framework ?
 9817:21 <jnewbery> dappdever: Look for any tests that call connect_nodes()
 9917:21 <sergei-t> OK, so the testing framework maintains a "fake" node that connects to the "real" node? and there are two ways of doing so - "directly" or via P2P?
10017:21 <jnewbery> hmrawal: Yes, we can use the RPC interface in the tests
10117:21 <glozow> omg i misunderstood this question. we're talking about connecting TestNodes vs connecting P2PConnections?
10217:22 <jnewbery> sergei-t: I think you're getting your terminology mixed up. A test contains one or more bitcoind instances under test. We can interact with those instances from the test framework using the RPC interface or the P2P interface. We can also get the different bitcoind instances to connect to each other using p2p.
10317:23 <willcl_ark> glozow: yeah in tests you can either directly connect them, or connect via
10417:23 <jnewbery> P2PConnection is a low-level class that the Python framework uses to connect to the bitcoind node (and after this PR is merged, it will be able to accept an outbound connection _from_ the bitcoind node)
10517:24 <jnewbery> willcl_ark: what do you mean 'directly' connect to them?
10617:24 <willcl_ark> jnewbery: well using their RPC to connect to each other
10717:24 <troygiorshev> lightlike: thanks, mininode has been renamed to p2p, got it
10817:24 <sergei-t> jnewbery: is it correct that the test framework spins up one or more "real" nodes that it tests and an additionally a "fake" node that can connect to "real" nodes?
10917:25 <jnewbery> willcl_ark: RPC is a different interface. We're just talking about P2P connections here
11017:25 <hmrawal> in the test framework, is the bitcoind instance treated as a node ?
11117:26 <jnewbery> sergei-t: there's no "fake" node. The test framework can open P2P connections to the bitcoind instances. You could call the test framework a "fake" node if you wanted to, but I don't think it makes things any clearer (in fact, I think it's confusing terminology)
11217:26 <jnewbery> hmrawal: the bitcoind instance is a node, yes
11317:26 <troygiorshev> hmrawal: yep exactly. Note that we can have multiple nodes (aka multiple bitcoind instances)
11417:26 <willcl_ark> jnewbery: Ok I think i see now
11517:27 <hmrawal> so n number of instances = n number of nodes right!
11617:27 <glozow>
11717:27 <sergei-t> jnewbery: fair enough! I just perceive anything that can connect to a node as a node itself, now I see this isn't always true
11817:28 <amiti> for people interested in learning more: when you are writing a functional test, in `set_test_params` you have to set `self.num_nodes`. this creates `TestNode` objects that wrap a bitcoind instance (and does things like forward along RPC calls)
11917:28 <amiti> if you have multiple test nodes initialized, they get connected to one another in `setup_nodes` in ``
12017:29 <sergei-t> so in question 2 "connecting two bitcoind nodes" refers to calling connect_nodes()?
12117:29 <glozow> if i might plug my test framework intro doc
12217:30 <glozow> sergei-t i believe so yes
12317:30 <jnewbery> glozow: thank you. That's a great doc!
12417:30 <sergei-t> glozow: great page, will read, thanks!
12517:30 <troygiorshev> glozow: amazing!
12617:30 <jnewbery> Next question. Why would it be useful to create outbound or block-relay-only connections in the test framework?
12717:30 <willcl_ark> glozow: very nice!
12817:31 <amiti> +1 awesome writeup glozow
12917:31 <sergei-t> and the second option in question 2 is connecting the nodes... how if not wth connect_nodes()? sorry for jumping back to q2
13017:31 <lightlike> sergei-t: I think it is a matter of terminology. I still think of the P2PConnection instances as objects, mock/fake nodes that we control and use to send the real node whatever p2p messages we wish.
13117:31 <dappdever> jnewberry: perhaps it is useful to test non-manual functionality, such as automatic disconnect or discouragement filter?
13217:31 <elle> useful since there are a few places in the code where there is a switch on connection path. so we want to test these different code paths
13317:32 <willcl_ark> jnewbery: presumably without being able to set up those types of nodes, then we can't test their codepaths
13417:32 <elle> *connection type
13517:32 <stacie> glozow that pastebin link you shared earlier is also really helpful!
13617:32 <glozow> thank you everyone i feel very appreciated today ^_^
13717:32 <troygiorshev> jnewbery: to test misbehaving outbound connections?
13817:32 <jnewbery> yay. We appreciate you glozow!
13917:34 <jnewbery> sergei: question 2 should say `P2PConnection`, not "P2P Connection". The difference is that the second option is creating a p2p connection using the python P2PConnection object, which as lightlike says, is a mock/fake to test the p2p interface
14017:34 <glozow> +1 to troygiorshev, limits us greatly in the testing department
14117:35 <jnewbery> dappdever elle troygiorshev: exactly, so we can test the code paths where we treat outbound and block-relay-only connections differently from manual connections
14217:35 <glozow> i tried to write a banning test once, and spent 2 days tracing why it didn't work - this is the line hahaha
14317:35 <pinheadmz> elle +1 for example we "prefer" not to download blocks from inbound peers
14417:36 <pinheadmz> and the version handshake is different of course
14517:36 <pinheadmz> (just grepping net_processing for IsInboundConn)
14617:36 <jnewbery> Q4. How does this PR enable creating outbound and block-relay-only connections in the test framework?
14717:37 <pinheadmz> add_p2p_connection() creates an inbound, and new method add_outbound_p2p_connection() ...
14817:37 <willcl_ark> It adds `add_outbound_p2p_connection()` to test/functional/test_framework/ which can be added in "outbound" or "blockrelay" mode
14917:38 <jnewbery> Yes, and what changes in the C++ allow us to create those outbound connections?
15017:39 <sergei-t> AddConnection() function added to CConnman ?
15117:39 <stacie> The new addconnection RPC
15217:39 <willcl_ark> I think thats CConnman::AddConnection
15317:40 <jnewbery> that's right! There's a new RPC called addconnection, which calls through to a new CConnman method called AddConnection(). Good team effort :)
15417:41 <jnewbery> The RPC method is hidden. Why do we make some RPC methods hidden, or “test only”?
15517:42 <glozow> users would get confooz about addconnection
15617:42 <pinheadmz> jnewbery to protect curious meddling users!
15717:42 <sergei-t> because the behavior of these RPCs is not triggered by an RPC call in "real world"? I.e., full relay connections only appear to randomly chosen nodes, it the node is specified, it's a manual connection by definition
15817:42 <emzy> They are not stable, can change in the future.
15917:43 <troygiorshev> emzy: +1
16017:43 <pinheadmz> although this RPC is also disabled on test and mainnet
16117:43 <jnewbery> emzy: I think that's the most important reason. As soon as you make an interface public, then it becomes part of some user's workflow and you can never change it!
16217:44 <jnewbery> pinheadmz: yes. Good observation.
16317:44 <sergei-t> jnewbery: so in principle this RPC may become public if it's considered stable?
16417:44 <hmrawal> I think it should
16517:44 <glozow> i don't see why users should be able to use this
16617:45 <dappdever> the connection type should be discerned from the messages exchanged, I think
16717:45 <willcl_ark> Specifying a blocksonly peer could be useful, in some niche?
16817:45 <sergei-t> it would be like manual connection but with behavior of full relay connection... _if_ we were to add it, would it make more sense to parameterize manual connection instead?
16917:45 <jnewbery> sergei-t: it's possible, but I think a good general principle is that new features/interfaces should be as tightly restricted as possible to begin with. It's much easier to give users features than take them away.
17017:46 <amiti> when implementing, I initially tried adding the functionality to the `addnode`RPC, but didn't like adding yet another definition to an already overloaded endpoint, so separated it out into a new RPC endpoint. then made it test only for mostly these reasons, like more flexibility & its easier to isolate test needs vs trying to encapsulate all possible uses on mainnet
17117:46 <amiti> (addnode RPC adds manual connections)
17217:46 <jnewbery> Normal users should never need this functionality though. I could only imagine it being useful for testing or niche use-cases
17317:46 <pinheadmz> and we already have good rpc to create outbound conenction
17417:47 <pinheadmz> if you want to accept a specific inbound you could use whitebind or somthing maybe
17517:47 <jnewbery> peer selection in general is something that should be automated and hidden from the vast majority of users
17617:47 <jnewbery> 6. For each different type of connection, what is the maximum number of connections that Bitcoin Core will have by default?
17817:48 <ariard> jnewbery: disagree, manual peering can be really useful
18017:48 <hmrawal> MAX_FEELER_CONNECTIONS = 1
18117:48 <hmrawal> MAX_ADDNODE_CONNECTIONS = 8
18217:48 <emzy> 8 full peers, 2 blocks only, one deeler.
18317:48 <stacie>
18417:48 <sergei-t> inbound = 125 - (all of the above)?
18517:48 <emzy> *feeler
18617:49 <willcl_ark> INBOUND: 10, OUTBOUND_FULL_RELAY: 8, MANUAL: ?, FEELER: 1, BLOCK_RELAY: 2, ADDR_FETCH: 1 (at a time)
18717:49 <hmrawal> willcl_ark: where did you find the inbound and addr_fetch connections ?
18817:49 <stacie> I don't know if those constants are the correct answer (MAX_OUTBOUND_FULL_RELAY_CONNECTIONS, MAX_BLOCK_RELAY_ONLY_CONNECTIONS, etc.) but that link to net.h is where I found them
18917:49 <sergei-t> willcl_ark: where's the max for addr_fetch connections defined?
19017:49 <jnewbery> ariard: if the majority of users have to manually configure their peer connections, then I don't think the developers have done their job
19117:50 <willcl_ark> hmrawal: sergei-t: I felt reading net.cpp that only one at a time was going to be allowed to happen?
19217:51 <sergei-t> I just see constants explicitly defined in net.h for all except addr_fetch
19317:51 <glozow> ariard manual peering is fine, but should be done using the interface for creating manual connections, which already exists
19417:51 <ariard> jnewbery: well just opening few manual connections might protect you against potential peer selection vulns, or settup some multihoming
19517:51 <hmrawal> even I read it in net.h but didn't get the inbound and addrfetch
19617:51 <jnewbery> willcl_ark: if you allow inbound connections, then by default you'll allow up to 125 connections in total
19717:51 <amiti> inbound is implicit, max connections is 125, so subtract the outbounds we have open
19817:52 <ariard> glozow: but you can't open block-relay-only manual connections with current interface?
19917:52 <hmrawal> jnewberry: exactly
20017:52 <imprecise> Am I late or early?
20117:52 <jnewbery> 8 of those will be for full outbound and 2 for block-relay outbound, so generally 115 would be left for inbound
20217:52 <glozow> ariard why do you need manual block-relay-only?
20317:52 <sergei-t> amiti: subtract the outbound including addr_fetch? (supposedly 1, but not defined in net.h)
20417:52 <ariard> there is a difference between a default setting for a wide majority of users and fine-setting its peering according to your network deployment
20517:52 <willcl_ark> jnewbery: ah whoops, I see that now...
20617:52 <amiti> imprecise: we are 52 minutes into the session :)
20717:53 <ariard> glozow: your current manuals are full-relay, thus I can guess their presence through tx/addr relay
20817:53 <imprecise> ':0 amiti thanks
20917:53 <amiti> addr_fetch connections are short-lived connections only (possibly) made on startup, I don't remember exactly how the counting works but for the majority of the time running a node, we won't have any open
21017:53 <jnewbery> imprecise: your timeliness could use some work, but your nick suggests an admirable level of self-awareness
21117:54 <jnewbery> ok, final question: What are CSemaphore and CSemaphoreGrant used for in CConnman?
21217:54 * imprecise chef kiss to fabulous British humor
21317:54 <jnewbery> :)
21417:55 <hmrawal> jnewbery: so I get the outbound but how's addr_fetch defined ?
21517:55 <glozow> ariard maybe a txrelay param in `addnode`... manual conn should be manual
21617:56 <willcl_ark> hmrawal: As I read it its implicit; you don't keep them open so there's not many (maybe only one at a time?)
21717:57 <hmrawal> why is that not subtracted from max peers (125) ?
21817:57 <jnewbery> willcl_ark: I believe that's right. ADDR_FETCH connections can only be opened from the ProcessAddrFetch() function
21917:57 <jnewbery> hmrawal: they're very short-lived. You open a connection, send a getaddr message, wait for the response and close the connection
22017:58 <sergei-t> Semaphores are used to not exceed the max allowed number of connections (but I'm not exactly sure how they do it)... what does "grant" mean in this context?
22117:58 <pinheadmz> jnewbery maybe i missed this but why do we need special connections just for addr fetch? do we not trust our exisintg peers? (something seomthing eclipse attack?)
22217:59 <amiti> pinheadmz: its if you don't have any existing peers yet
22317:59 <pinheadmz> amiti aha thank you - we have dns seeds for this as well though right?
22417:59 <jnewbery> sergei-t: exactly. It's used to not exceed the maximum number of outbound connections. If you can't get a semaphore grant it means that all outbound connection slots are filled
22518:00 <amiti> so, if you're starting up a node for the first time. you connect to the DNS seeds with addr fetch connections. they send you back addresses of nodes to populate your addrman, you close those connections and start opening connections to the general network
22618:00 <ariard> glozow: yeah IMO it should be both block-relay and manual something not captured by our current connection type struct
22718:00 <willcl_ark> amiti: do we ever "reload" on addresses after we've been running for a while?
22818:00 <jnewbery> ding ding ding. That's time
22918:00 <kanzure> timezones
23018:00 <jnewbery> #endmeeting
23118:00 <glozow> AW MAN
23218:00 <sipa> hi!
23318:00 <kanzure> hi
23418:00 <amiti> there's also a fallback set of seeds incase there's a crazy infrastructure attack or you don't want to connect to DNS, but I believe these are the only two groups that we automatically connect to via addrfetch
23518:00 <imprecise> hi
23618:00 <pedr0fr> I have a question. I want to try the ZMQ sequence notifier (merged on the 23rd of September - PR #19572).I am willing to wait for bitcoin core 0.21.0rc1, which, if I understand correctly, was planned for the 1st of November ( When is 0.21.0rc1 expected to be released? Thank you.
23718:01 <glozow> thank u jnewbery and amiti for the fabulous content
23818:01 <troygiorshev> thanks jnewbery and amiti !
23918:01 <dappdever> thank you!
24018:01 <willcl_ark> yes thanks jnewbery!
24118:01 <lightlike> thanks!
24218:01 <elle> thanks guys! very informative session!
24318:01 <emzy> Thank you!
24418:01 <imprecise> *pound* start meeting for people who got mistaken on time zone issues
24518:01 <amiti> willcl_ark: what do you mean reload?
24618:01 <hmrawal> thank you guys
24718:01 <sipa> pedr0fr: after 0.21 forks off, which is still waiting on a few issues
24818:01 <fodediop1> thank you everybody
24918:01 <sergei-t> thank you!
25018:01 <amiti> thanks all for coming! thanks jnewbery for hosting :)
25118:01 <jnewbery> thanks for coming everyone!
25218:02 <imprecise> ty for hosting jnewbery and amiti, apologies for not making the meeting
25318:02 <willcl_ark> amiti: well you say we only do it at fresh startup; what about a nodes that's starting up again after being offline for a while, or a node that's been running for some time without being turned off
25418:02 <hmrawal> asking silly questions isn't against the rules right ? :-p
25518:02 <stacie> Thanks jnewbery and amiti!
25618:02 <glozow> you probably don't need that, because you can read from peers.dat
25718:02 <pedr0fr> sipa: Thank you. I see that it is waiting for three issues.
25818:02 <dappdever> where is the code for handling misbehaving outbound connections? it seems like that will be testable after this PR, correct?
25918:03 <glozow> dappdever grep for Misbehaving()
26018:03 <willcl_ark> glozow: sure, I'm just wondering if we ever make those connections if we have a "healthy" peers.dat
26118:03 <glozow> in net_processing.cpp
26218:03 <fodediop1> exit
26318:03 <amiti> willcl_ark: if the node has been running for a while, you'd expect the addrman to be populated with addresses from normal address relay behaviors
26418:03 <willcl_ark> ok
26518:04 <amiti> willcl_ark: I don't believe so. if you were to shut down the node, wipe your `peers.dat` and then restart, you might create addr fetch connections again
26618:04 <sipa> addr_fetch was originally introduced for situations where we can't just resolve DNS as a means of getting IP addresses (specifically when only connecting through tor)
26718:06 <sipa> so instead of asking a DNS seed, we *connect* to one (which means the exit node we're using will resolve for us, and open a connection to one of the resolved IPs); we still want a lot of IPs though, so we ask that node
26818:07 <sipa> it's essentially adding a redirection step, to work around the fact that DNS seeds can't be queried directly through tor