--- Log opened Wed Oct 19 00:00:31 2022
--- Day changed Wed Oct 19 2022
00:00 -!- raj- [~raj@167.182.81.172.lunanode-rdns.com] has joined #bitcoin-core-pr-reviews
00:09 -!- raj- [~raj@167.182.81.172.lunanode-rdns.com] has quit [Quit: ZNC 1.7.5+deb4 - https://znc.in]
00:09 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
00:09 -!- raj- [~raj@2602:ffb6:4:e396:f816:3eff:fe47:ca2a] has joined #bitcoin-core-pr-reviews
01:12 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 260 seconds]
01:30 -!- TheRec [~toto@84-75-225-47.dclient.hispeed.ch] has joined #bitcoin-core-pr-reviews
01:30 -!- TheRec [~toto@84-75-225-47.dclient.hispeed.ch] has quit [Changing host]
01:30 -!- TheRec [~toto@user/therec] has joined #bitcoin-core-pr-reviews
01:34 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
01:39 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 264 seconds]
02:09 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
02:55 -!- __gotcha [~Thunderbi@94.105.119.240.dyn.edpnet.net] has joined #bitcoin-core-pr-reviews
03:08 -!- chipxxx [~chip@2001:8a0:f61c:9200:9c3d:d55a:c1aa:516e] has joined #bitcoin-core-pr-reviews
03:09 -!- chipxxx [~chip@2001:8a0:f61c:9200:9c3d:d55a:c1aa:516e] has quit [Read error: Connection reset by peer]
03:12 -!- chipxxx [~chip@2001:8a0:f61c:9200:9c3d:d55a:c1aa:516e] has joined #bitcoin-core-pr-reviews
03:14 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 264 seconds]
03:27 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
03:30 -!- jon_atack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
03:31 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 260 seconds]
03:32 -!- jonatack [~jonatack@user/jonatack] has quit [Ping timeout: 268 seconds]
03:59 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
04:16 -!- __gotcha [~Thunderbi@94.105.119.240.dyn.edpnet.net] has quit [Ping timeout: 268 seconds]
04:17 -!- helloee [~helloee@213.52.102.107] has joined #bitcoin-core-pr-reviews
04:23 -!- __gotcha [~Thunderbi@94.105.119.240] has joined #bitcoin-core-pr-reviews
04:28 -!- __gotcha [~Thunderbi@94.105.119.240] has quit [Ping timeout: 268 seconds]
04:34 -!- __gotcha [~Thunderbi@94.105.119.240.dyn.edpnet.net] has joined #bitcoin-core-pr-reviews
04:34 -!- __gotcha1 [~Thunderbi@94.105.119.240.dyn.edpnet.net] has joined #bitcoin-core-pr-reviews
04:34 -!- __gotcha [~Thunderbi@94.105.119.240.dyn.edpnet.net] has quit [Read error: Connection reset by peer]
04:34 -!- __gotcha1 is now known as __gotcha
04:34 -!- __gotcha [~Thunderbi@94.105.119.240.dyn.edpnet.net] has quit [Client Quit]
04:35 -!- __gotcha [~Thunderbi@94.105.119.240.dyn.edpnet.net] has joined #bitcoin-core-pr-reviews
04:38 -!- raj- [~raj@2602:ffb6:4:e396:f816:3eff:fe47:ca2a] has quit [Ping timeout: 264 seconds]
04:38 -!- __gotcha1 [~Thunderbi@cust-116-155-109-94.dyn.as47377.net] has joined #bitcoin-core-pr-reviews
04:39 -!- raj- [~raj@2602:ffb6:4:e396:f816:3eff:fe47:ca2a] has joined #bitcoin-core-pr-reviews
04:41 -!- __gotcha [~Thunderbi@94.105.119.240.dyn.edpnet.net] has quit [Ping timeout: 264 seconds]
04:46 -!- __gotcha1 [~Thunderbi@cust-116-155-109-94.dyn.as47377.net] has quit [Ping timeout: 264 seconds]
05:03 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 260 seconds]
05:05 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
05:38 -!- Bohruz [~Bohruz@mail2.khomp.com.br] has joined #bitcoin-core-pr-reviews
05:38 -!- Bohruz [~Bohruz@mail2.khomp.com.br] has quit [Client Quit]
05:59 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:f876:7470:b2c:dd9f] has joined #bitcoin-core-pr-reviews
06:07 -!- helloee [~helloee@213.52.102.107] has quit [Quit: Connection closed]
06:08 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 248 seconds]
06:23 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
06:27 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 260 seconds]
06:29 -!- furszy [~furszy@104.128.239.93] has quit [Changing host]
06:29 -!- furszy [~furszy@user/furszy] has joined #bitcoin-core-pr-reviews
06:40 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
06:45 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 264 seconds]
06:50 -!- chipxxx [~chip@2001:8a0:f61c:9200:9c3d:d55a:c1aa:516e] has quit [Ping timeout: 260 seconds]
06:51 -!- cstafford1717 [~cstafford@90.241.80.195] has joined #bitcoin-core-pr-reviews
06:59 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
07:20 -!- srihari [~srihari@49.206.59.22] has joined #bitcoin-core-pr-reviews
07:44 -!- srihari [~srihari@49.206.59.22] has quit [Remote host closed the connection]
08:01 -!- srihari [~srihari@49.206.59.22] has joined #bitcoin-core-pr-reviews
08:03 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 264 seconds]
08:09 -!- jon_atack [~jonatack@user/jonatack] has quit [Ping timeout: 264 seconds]
08:16 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
09:00 -!- srihari [~srihari@49.206.59.22] has quit [Remote host closed the connection]
09:10 -!- jesseposner_ [~jesse@c-24-5-105-39.hsd1.ca.comcast.net] has quit [Quit: Textual IRC Client: www.textualapp.com]
09:11 -!- jesseposner [~jesse@user/jesseposner] has joined #bitcoin-core-pr-reviews
09:19 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 264 seconds]
09:20 -!- hernanmarino [~hernanmar@181.99.169.107] has joined #bitcoin-core-pr-reviews
09:39 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
09:44 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 264 seconds]
09:50 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
09:53 -!- pablomartin [~pablomart@185.199.100.60] has joined #bitcoin-core-pr-reviews
10:01 -!- Bohruz [~Bohruz@mail2.khomp.com.br] has joined #bitcoin-core-pr-reviews
10:01 < pablomartin> hello
10:01 < brunoerg> hi
10:02 < glozow> hi
10:02 < amovfx> Good morning!
10:02 < lightlike> hi
10:02 < cstafford1717> hi
10:02 < kouloumos> hi
10:02 < raj-> hi..
10:03 < pablomartin> amovfx it's the afternoon in some places haha
10:03 < glozow> pinging larry, let's get started tho
10:03 < glozow> #startmeeting
10:03 < LarryRuane> hi everyone! yes sorry, let's get started!
10:03 < hernanmarino> Hello 
10:03 < LarryRuane> today we're reviewing PR 25515 https://bitcoincore.reviews/25515
10:04 < amovfx> pablomartin: yea, I just realized that after I typed it, brain is still gearing up
10:04 < amovfx> :)
10:04 < LarryRuane> anyone new joining us today? feel free to say hi
10:04 -!- srihari [~srihari@49.206.59.22] has joined #bitcoin-core-pr-reviews
10:04 < LarryRuane> don't hesitate to ask questions! Even if we've already moved on to the next topic, feel free to ask questions about something we've already covered
10:05 < cstafford1717> Me I am new
10:05 < andrewtoth> hi
10:05 < LarryRuane> welcome @cstafford1717 !
10:05 < brunoerg> cstafford1717: welcome! :)
10:05 < Bohruz> I am new too
10:05 < Bohruz> My first time here
10:05 < LarryRuane> Let's see.. Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK?
10:06 < brunoerg> Bohruz: welcome! 
10:06 < Bohruz> Thanks
10:06 < brunoerg> Concept ACK
10:06 < amovfx> welcome!
10:06 < srihari> approac ACK
10:06 < raj-> tested ACK.. 
10:06 < hernanmarino> Yes, tested ACK here
10:06 < pablomartin> tested ACK, didnt add the comments into the pr yet
10:06 < LarryRuane> (for anyone new, the general format is to go through the questions and discuss.. feel free to ask your own questions)
10:07 < LarryRuane> Good, several people have at least taken a look... Can anyone summarize the PR?
10:08 -!- pgs [~pgs@ip72-197-225-40.sd.sd.cox.net] has joined #bitcoin-core-pr-reviews
10:08 < raj-> Make a new interface so we can mock networking stuffs in unit tests.. 
10:09 < LarryRuane> raj-: Yes, and for anyone to answer, what is mocking in general, as a concept? Why is it useful?
10:09 < LarryRuane> Bohruz: welcome!
10:10 < amovfx> isolate and focus on code being tested and not the behavior or state of external deps.
10:10 < brunoerg> mock is something used to simulate the behavior of other one
10:10 < kouloumos> especially useful for unit tests
10:10 < amovfx> yea faster more reliable tests
10:10 < amovfx> unit tests*
10:11 < raj-> My understanding is its useful for unit situations where we need to mimic some behaviour but we can't directly instantiate a concrete class.. Here we can't have real CConnMan as there is no real networking in unit tests..  
10:11 < brunoerg> raj: +1
10:11 < pablomartin> agree with brunoerg
10:11 < pablomartin> and raj yeah
10:12 < LarryRuane> amovfx: Yes excellent ... Here's how I think of it ... software, at least if it's well-written, is implemented in layers, and if you want to test a given layer, it will need the layers below it (call on its methods) ... but it may not be possible or practical to include all of that lower layer's functionality in the test setup
10:12 < LarryRuane> yes other great answers there (while I was typing all that!)
10:12 < LarryRuane> so what is the upper layer being test here, and what is the lower layer being mocked?
10:12 < LarryRuane> *tested
10:13 < raj-> Guess.. Tested: Peer Manager, Mocked: Connection manager..
10:14 < LarryRuane> raj-: yes exactly! what do these layers do, roughly?
10:14 -!- amovfx [~amovfx@node-1w7jr9yi65te65h9e1oy1bf3k.ipv6.telus.net] has quit [Remote host closed the connection]
10:14 -!- amovfx [~amovfx@node-1w7jr9yi65te7l6by99j1mo3v.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
10:14 -!- effexzi [uid474242@id-474242.ilkley.irccloud.com] has joined #bitcoin-core-pr-reviews
10:15 < amovfx> Connection manager operates on peers?
10:15 < raj-> I found it hard to summarize their "management".. Wouldn't it be useful to have some docs before these classes? 
10:15 < pablomartin> is used to manage connections and maintain statistics on each node connected
10:15 < pablomartin> *conman
10:16 -!- Talkless [~Talkless@mail.dargis.net] has joined #bitcoin-core-pr-reviews
10:16 < brunoerg> Peer Manager is used to manage connections to the peer and the application state of it
10:16 < pablomartin> peerman: manages peer state and interaction e.g. processing messages, fetching blocks & removing for misbehaviour
10:16 < LarryRuane> raj-: yes, good idea.. I think of it as, peer manager decides to send certain messages, and processes received ones ... connection manager is the low-level moving-bytes-over-the-wire kind of stuff
10:17 < LarryRuane> More basic question, Why do we need unit tests? Aren’t functional tests sufficient?
10:17 < LarryRuane> (that's question 2)
10:17 < brunoerg> basically Peer Manager depends on connection manager... we need a connection (more low level stuff) to a peer to get info from it
10:18 < raj-> LarryRuane : Ah that makes sense.. So the ConnMan is like per peer management, and PeerMan is like the global message handler??
10:18 < LarryRuane> well I think PeerManager also knows about the individual peers.. I think both layers do
10:19 < LarryRuane> it's just the PeerManager is high-level things (like sequencing the multiple messages in a version handshake), and connection manager is dealing with low level stuff (like the addresses of peers)
10:20 < brunoerg> I usually say we need unit tests to test "functions" and functional tests to test "features" 
10:21 < brunoerg> it's not possible to use functional tests to test specific behavior of some functions/methods
10:21 < glozow> functional tests have a ton of overhead that makes them ill-equipped to target specific pieces of internal logic, and really really slow. we can't test eviction logic in isolation; we have to spin up a bunch of nodes, wait for evictions to be triggered, mine real blocks, to feed to the node, etc. etc.
10:21 < LarryRuane> Both of these layers, and many more, are exercised in the functional (python) tests, so what are the advantages of (also) unit-testing?
10:21 < amovfx> yea I thought functional tests would operate on multiple parts of the system, esentially simulate the program running
10:21 < amovfx> unit testing is faster and more specific?
10:22 < stratospher[m]> functional tests are more time consuming and resource intensive to set up. unit tests would be easier to debug since behaviour is compartmentalised.
10:22 < LarryRuane> brunoerg: yes I like that, glozow: good points, also functional tests can't test as deeply because they're limited to the RPC and P2P interfaces
10:22 < LarryRuane> stratospher[m]: +1
10:23 < brunoerg> stratospher[m]: +1
10:23 < LarryRuane> I think we already covered question 3: Why do we need to mock CConnman to unit test PeerManager?
10:23 < amovfx> there are no connections?
10:23 < LarryRuane> (it's because PeerManager uses a connection manager)
10:24 < amovfx> or there are no peers
10:24 -!- Talkless [~Talkless@mail.dargis.net] has quit [Remote host closed the connection]
10:24 < LarryRuane> amovfx: in what context? the unit tests?
10:24 < amovfx> yes
10:24 < amovfx> I read a Node can be mocked though
10:24 < amovfx> err CNode
10:24 -!- Talkless [~Talkless@mail.dargis.net] has joined #bitcoin-core-pr-reviews
10:25 < ws11__> Because unit tests should not use network
10:25 < LarryRuane> in a unit test there are no real connections, but the mock connection manager simulates one
10:26 < LarryRuane> ws11__: yes, there isn't a real network in the unit test framework
10:26 < LarryRuane> Question 4, Do mock interfaces need to implement all of the real interfaces?
10:27 < amovfx> I think only functions that have = 0?
10:27 < brunoerg> we can implement only what we are going to use in the test i guess
10:27 < amovfx> Its that overloading feature that demands a function be implimented
10:27 < pablomartin> amovfx which are virtual
10:27 < amovfx> yes
10:27 < amovfx> forgot the term because I am noob
10:27 -!- Talkless [~Talkless@mail.dargis.net] has quit [Remote host closed the connection]
10:27 < raj-> Only the ones we need to mock.. Other ones can return some default values..
10:27 < lightlike> only those functions that are called by the code that we want to test?
10:28 < raj-> But I think all needs to be override though in order to implement the interface.. 
10:28 -!- jesseposner [~jesse@user/jesseposner] has quit [Ping timeout: 268 seconds]
10:28 < LarryRuane> raj-: lightlike: Exactly, that's what I was getting (question not worded well) .. All must be defined, but no, only the ones that the test causes the production code-under-test to invoke need to be (non-trivially) implemented
10:29 < LarryRuane> here you can see a bunch of functions that don't provide a non-trivial implementation: https://github.com/dergoegge/bitcoin/blob/f98a4e8d891dd7374ef7dc4c723797bf0705075f/src/test/peerman_tests.cpp#L50
10:29 < amovfx> I think I mixed up the definition of declare and define
10:30 < lightlike> LarryRuane: are you sure? E.g. the function AddNode() seems to be part of CConnman, but not of ConnectionsInterface
10:30 < LarryRuane> see how (at that link) `ForNode()` is just an empty function (`{}`) ... it must be defined but doesn't need to do anything because the test code doesn't cause the peer manager to call it!
10:32 -!- Talkless [~Talkless@mail.dargis.net] has joined #bitcoin-core-pr-reviews
10:32 < LarryRuane> lightlike: That's a good point, derived classes can provide *additional* methods beyond what the interface (base) class specifies
10:33 < LarryRuane> but then the other derived classes (like our mock, `ConnectionsInterfaceMock`) can't simulate it
10:34 < LarryRuane> But maybe `AddNode()` can be added to the interface class later, if that method needs to be simulated by a mock?
10:34 < kouloumos> Do we currently mock any other components? (I didn't find anything interesting with a quick search)
10:34 < raj-> Question: What guides the separation between interface functions and *additional* function of the derived type, while creating an interface like this?
10:35 < lightlike> yes, I think we'd get a compilation error if AddNode() was used in the Peerman code we are testing
10:35 -!- Bohruz [~Bohruz@mail2.khomp.com.br] has quit [Quit: Connection closed]
10:37 < LarryRuane> lightlike: +1 raj-: I think what's in the interface are at least those functions that need to currently be mocked, but more can be added later (as more tests are written)
10:38 < kouloumos> Does that mean that we could also omit (from the interface), those that are currently trivially implemented?
10:38 < raj-> LarryRuane, yes that makes sense in my head.. So we can say that this interface is "dynamic" and can be modified later depending upon test requirements? 
10:38 < LarryRuane> kouloumos: I'm not sure.. there was a bunch of PRs listed here: https://github.com/bitcoin/bitcoin/issues/19398
10:39 < brunoerg> Interesting
10:39 < LarryRuane> a big motivation for which is to enable testing more using unit tests
10:40 < LarryRuane> Hope that answers things, speak up if we missed anything ... question 5: Should we mock other components as well? (e.g.: `CTxMempool`, `ChainstateManager`)?
10:40 < amovfx> so an overall design goal is to identify where interfaces are needed, implement them, then unit test them?
10:41 < raj-> Sounds fun to me.. :D But probably increases code complexity with more abstractions?? 
10:41 -!- srihari [~srihari@49.206.59.22] has quit [Remote host closed the connection]
10:41 < LarryRuane> amovfx: Yes, more precisely, implement interfaces so that code that *uses those interfaces* can be unit tested
10:41 < amovfx> I would say yes, for 1 consistency, 2 for more robust testing
10:42 < brunoerg> raj: I think so, but it's worth
10:43 < LarryRuane> raj-: I'd say ideally it should decrease code complexity by separating the layers more clearly, instead of everything being spaghettied (?) together
10:43 < raj-> brunoerg, LarryRuane ya agreed on that.. 
10:44 < brunoerg> It would make the code more readable and testable but maybe could increase codebase (not saying this is totally bad)
10:44 < LarryRuane> So for question 5, I think we're on the same page that other classes should be mocked, but doing so would have to be driven by the tests that people are writing
10:44 < LarryRuane> (in other words, exactly what the mock should do!)
10:45 < LarryRuane> Question 6, Roughly, what is the overhead of using an interface class? (versus using the derived class directly)
10:46 < hernanmarino> LarryRuane: +1
10:46 -!- hernanmarino [~hernanmar@181.99.169.107] has quit [Remote host closed the connection]
10:46 < amovfx> I think it is pretty small, just adds a layer of indirection, so program size increases.
10:47 -!- hernanmarino [~hernanmar@181.99.169.107] has joined #bitcoin-core-pr-reviews
10:47 < amovfx> Wait, is this interface only used in tests?
10:47 -!- hernanmarino_ [~hernanmar@181.99.169.107] has joined #bitcoin-core-pr-reviews
10:47 < amovfx> No it isn't, it's used in the PeerManagerImpl
10:48 < LarryRuane> amovfx: Yes, slightly, calling methods involves dereferencing a function pointer (indirect function call), which has a very low overhead
10:49 < LarryRuane> amovfx: correct, the new interface class `ConnectionsInterface` must be used in production code (by the peer manager), but the interface class *allows* unit tests
10:50 < lightlike> maybe the compiler can't inline functions anymore when they are virtual, which could be a slight decrease in performance?
10:50 < raj-> Question: The `CConnMan` includes `ConnectionsInterfaceMock` as a `friend class`.. Why is that?? 
10:51 < LarryRuane> lightlike: that's a good point! I agree, decision of which function to call must happen at runtime
10:51 < raj-> Also seems to be the case for `CNode`.. 
10:52 < pablomartin> raj it's to access to the private definition of the class
10:53 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 260 seconds]
10:54 < pablomartin> i'm not very comfortable to add this kind of dependency but perhaps there's no other way to do this
10:54 < amovfx> wot
10:54 < LarryRuane> raj-: I hadn't noticed that, good question! pablomartin: That's my understanding, it would let the friend class's methods (the mock functions in this case) to access the private members of CConnman (for example)
10:54 < raj-> pablomartin, okay.. Ya that was clear but I was confused on why and had a hard time wrapping head around who depends on what.. 
10:55 < LarryRuane> I think the idea is that test code needs to be able to dig into data structures, violate the layering so to speak, that you don't want production code to be able to do
10:56 < kouloumos> Regarding a change like this that maybe results to the compiler not be able to inline some functions. How could someone measure the performance of such a change? 
10:57 < LarryRuane> We have a whole benchmarking framework
10:58 < LarryRuane> maybe that would make it possible to measure? (You might need to write a specific benchmark test)
10:58 < LarryRuane> Ok we're about out of time, let me skip ahead to question 10: What is the general approach of the version_handshake test? Why does it contain the series of calls to BOOST_CHECK_EQUAL()?
10:58 < LarryRuane> (we sort of discussed this earlier)
10:59 < brunoerg> To check if the messages are being sent correctly?
10:59 < raj-> to make sure we are correctly sending all the `NetMsgType`s correctly at version handshake..  
11:00 < LarryRuane> Yes those are good answers, I was thinking it could maybe test that the p2p messages were being sent in the correct order (rather than just the number of times they were sent)
11:00 < raj-> LarryRuane, +1.. That would be cool.. 
11:01 < amovfx> makes sense, the handshake happens in a specific order correct?
11:01 < LarryRuane> Well we're out of time, sorry we didn't get to every question, thank you all for coming! feel free to stay here and continue the discussion!
11:01 < LarryRuane> #endmeeting
11:01 < raj-> Thansk LarryRuane for hosting.. Great one.. 
11:01 < hernanmarino_> Goodbye ! Thanks Larry for hosting 
11:02 < brunoerg> Thanks LarryRuane, learned a lot! 
11:02 < lightlike> Thanks LarryRuane! 
11:02 < amovfx> Thanks so much for hosting! learned a lot again :D
11:02 -!- pgs [~pgs@ip72-197-225-40.sd.sd.cox.net] has quit [Quit: Connection closed]
11:02 < pablomartin> thanks Larry!
11:02 < stratospher[m]> thank you Larry!
11:02 < andrewtoth> Thanks LarryRuane!
11:02 < kouloumos> Thank you Larry!
11:02 -!- hernanmarino_ [~hernanmar@181.99.169.107] has quit [Quit: Leaving]
11:02 -!- hernanmarino [~hernanmar@181.99.169.107] has quit [Quit: Leaving]
11:03 < LarryRuane> amovfx: yes ... One thing I've been guilty of myself when writing tests is to make the _overspecify_ behavior .. so if that version_handshake unit test insisted that the WTXIDRELAY message happened before the SENDADDRV2 message (which is currently true),
11:04 < LarryRuane> then if for some unrelated reason the order of those needed to be reversed, this test would break, but arguably there's nothing really wrong!
11:04 < amovfx> ah makes sense
11:05 < LarryRuane> So maybe it is better as it is .. I think it has to be decided on a case-by-case basis .. and also of course you don't want to make the test too complicated!
11:18 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
11:23 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 260 seconds]
11:23 -!- jesseposner [~jesse@user/jesseposner] has joined #bitcoin-core-pr-reviews
11:24 -!- amovfx [~amovfx@node-1w7jr9yi65te7l6by99j1mo3v.ipv6.telus.net] has quit []
11:25 -!- cstafford1717 [~cstafford@90.241.80.195] has quit [Quit: Connection closed]
11:26 -!- amovfx [~amovfx@d75-156-179-9.abhsia.telus.net] has joined #bitcoin-core-pr-reviews
11:29 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
11:38 < sipa> There is a place for tests that just test observed behavior, without concern for whether what they are testing for is "correct" behavior or not.
11:38 < sipa> But they should be clearly marked as such.
11:40 < sipa> In this example, it may be worthwhile to let a developer who is working on refactoring networking code know that they're (perhaps accidentally) causing WTXIDRELAY and SENDADDRV2 to be sent out in opposite order. Even if there is nothing wrong with that swapping directly, it shouldn't happen unless someone is intentionally making that change.
11:40 < instagibbs> lots and lots and lots of tests check things like "is block subsidy 50 coins", or at least used to
11:40 < instagibbs> maybe got better 
11:45 -!- amovfx [~amovfx@d75-156-179-9.abhsia.telus.net] has quit [Remote host closed the connection]
11:45 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
11:48 < lightlike> I find https://diyhpl.us/wiki/transcripts/greg-maxwell/greg-maxwell-bitcoin-core-testing/ by gmaxwell (paragraph in the middle) interesting in this regard
11:49 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 255 seconds]
11:50 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
11:58 -!- pablomartin [~pablomart@185.199.100.60] has quit [Ping timeout: 246 seconds]
12:12 -!- Talkless [~Talkless@mail.dargis.net] has quit [Quit: Konversation terminated!]
12:33 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5nvcpppuylib8.ipv6.telus.net] has quit [Ping timeout: 260 seconds]
12:34 -!- Peder123 [~Peder123@227.37-191-167.fiber.lynet.no] has joined #bitcoin-core-pr-reviews
12:36 -!- Peder123 [~Peder123@227.37-191-167.fiber.lynet.no] has quit [Client Quit]
12:39 -!- greypw2546002 [~greypw254@grey.pw] has quit [Remote host closed the connection]
12:40 -!- greypw2546002 [~greypw254@grey.pw] has joined #bitcoin-core-pr-reviews
12:43 -!- effexzi [uid474242@id-474242.ilkley.irccloud.com] has quit [Quit: Connection closed for inactivity]
12:53 < LarryRuane> sipa: instagibbs: lightlike: thanks, good points!
13:03 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
13:08 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 252 seconds]
13:12 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Remote host closed the connection]
13:13 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
13:15 -!- jon_atack [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
13:17 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 255 seconds]
13:20 -!- jonatack1 [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
13:21 -!- jon_atack [~jonatack@user/jonatack] has quit [Ping timeout: 255 seconds]
13:22 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
13:24 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
13:27 -!- jonatack1 [~jonatack@user/jonatack] has quit [Ping timeout: 255 seconds]
13:30 -!- jonatack1 [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
13:36 -!- jonatack1 [~jonatack@user/jonatack] has quit [Read error: Connection reset by peer]
13:44 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Remote host closed the connection]
13:45 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
13:49 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 246 seconds]
13:52 -!- jhon [~jhon@177.69.22.93] has joined #bitcoin-core-pr-reviews
13:53 -!- jhon [~jhon@177.69.22.93] has quit [Client Quit]
14:05 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
14:24 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 255 seconds]
14:29 -!- jesseposner [~jesse@user/jesseposner] has quit [Ping timeout: 246 seconds]
14:34 -!- jesseposner [~jesse@user/jesseposner] has joined #bitcoin-core-pr-reviews
14:54 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
15:00 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 252 seconds]
15:04 -!- NorrinRadd [~me@85.237.194.149] has quit [Ping timeout: 252 seconds]
15:08 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 252 seconds]
15:15 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
15:22 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
15:26 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 246 seconds]
15:45 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
15:49 -!- Norrin [~me@188.215.95.44] has joined #bitcoin-core-pr-reviews
15:52 -!- Norrin is now known as NorrinRadd
15:57 -!- brunoerg [~brunoerg@2804:14d:5281:8ae2:f876:7470:b2c:dd9f] has quit []
16:01 -!- greypw2546002 [~greypw254@grey.pw] has quit [Remote host closed the connection]
16:01 -!- greypw2546002 [~greypw254@grey.pw] has joined #bitcoin-core-pr-reviews
16:18 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 246 seconds]
16:45 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 255 seconds]
16:51 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
16:56 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
17:03 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 246 seconds]
17:10 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
17:15 -!- Zenton [~user@user/zenton] has quit [Ping timeout: 252 seconds]
17:42 -!- greypw2546002 [~greypw254@grey.pw] has quit [Remote host closed the connection]
17:42 -!- greypw2546002 [~greypw254@grey.pw] has joined #bitcoin-core-pr-reviews
17:58 -!- amovfx_ [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 246 seconds]
18:10 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 252 seconds]
18:21 -!- NorrinRadd [~me@188.215.95.44] has quit [Ping timeout: 272 seconds]
18:22 -!- NorrinRadd [~me@85.237.194.112] has joined #bitcoin-core-pr-reviews
18:26 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
18:32 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 246 seconds]
18:32 -!- greypw2546002 [~greypw254@grey.pw] has quit [Remote host closed the connection]
18:33 -!- greypw2546002 [~greypw254@grey.pw] has joined #bitcoin-core-pr-reviews
18:43 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
18:45 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
18:48 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 255 seconds]
18:50 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
18:55 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 246 seconds]
19:05 -!- jonatack1 [~jonatack@user/jonatack] has joined #bitcoin-core-pr-reviews
19:10 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
19:11 -!- Bohruz [~Bohruz@191.251.58.199] has joined #bitcoin-core-pr-reviews
19:12 -!- Bohruz [~Bohruz@191.251.58.199] has quit [Client Quit]
19:24 -!- greypw2546002 [~greypw254@grey.pw] has quit [Remote host closed the connection]
19:24 -!- greypw2546002 [~greypw254@grey.pw] has joined #bitcoin-core-pr-reviews
19:27 -!- jonatack1 [~jonatack@user/jonatack] has quit [Read error: Connection reset by peer]
19:48 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 255 seconds]
20:11 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 252 seconds]
20:42 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
20:56 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
21:05 -!- greypw2546002 [~greypw254@grey.pw] has quit [Remote host closed the connection]
21:06 -!- greypw2546002 [~greypw254@grey.pw] has joined #bitcoin-core-pr-reviews
21:09 -!- andrewtoth [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Remote host closed the connection]
21:10 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews
21:15 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Remote host closed the connection]
21:15 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews
21:26 -!- greypw2546002 [~greypw254@grey.pw] has quit [Remote host closed the connection]
21:26 -!- greypw2546002 [~greypw254@grey.pw] has joined #bitcoin-core-pr-reviews
21:43 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has quit [Ping timeout: 258 seconds]
21:46 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has joined #bitcoin-core-pr-reviews
21:47 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 246 seconds]
22:00 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 255 seconds]
22:02 -!- _andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews
22:04 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Remote host closed the connection]
22:29 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
22:31 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has quit [Remote host closed the connection]
22:31 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has joined #bitcoin-core-pr-reviews
22:34 -!- amovfx [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 246 seconds]
22:49 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
23:04 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
23:08 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has quit [Ping timeout: 246 seconds]
23:13 -!- amovfx_ [~amovfx@node-1w7jr9yi65te5ujpx628y9xci.ipv6.telus.net] has joined #bitcoin-core-pr-reviews
23:51 -!- greypw2546002 [~greypw254@grey.pw] has quit [Remote host closed the connection]
23:51 -!- greypw2546002 [~greypw254@grey.pw] has joined #bitcoin-core-pr-reviews
23:54 -!- amovfx [~amovfx@node-1w7jr9yi65te76avgv6tof5ba.ipv6.telus.net] has quit [Ping timeout: 272 seconds]
--- Log closed Thu Oct 20 00:00:52 2022