refactor(wire): split node.rs by behavior#790
Conversation
|
I also think we may want to move to |
Davidson-Souza
added
code quality
|
So this is how it would look like if we move contexts into 
|
|
ACK 27bd2a0 Nice! This refactoring makes it much clearer to me what each part is doing. |
|
Strong concept ACK. Let the clean up session lead us to a brighter future 🚀
I think it makes sense
Looks great! I can even see how this will make testing more approachable. |
JoseSK999
force-pushed
the
refactor-node-impl
branch
2 times, most recently
from
7580cbb to
4b9a4cf
Compare
In terms of 
|
So far `node.rs` has been a catch-all file, since it was containing the definition for types, macros, and methods for anything needed during the `UtreexoNode` operation. It was already on the 2,250 lines, which makes it hard to reason about and test. I think there are roughly four kind of behaviors: - Peer management (including inflight requests, addnode, message times, etc.) - Connectivity (opening connections, bootstrapping via DNS/seeds, and maintenance) - Block pipeline (requesting blocks/proofs and processing them) - Handling user requests So I created four files under the new `node/` folder for these behaviors, along with the three node contexts. The only code changes were: - Making `broadcast_to_peers`, `resolve_connect_host`, `get_port` and `to_addr_v2` `pub(crate)` - Moving the `Default` impl for `RunningNode` to its own file - Renaming `maybe_use_hadcoded_addresses` to `maybe_use_hardcoded_addresses` (typo). Hence, this change is move-only in terms of the method bodies (should be easy to verify).
JoseSK999
force-pushed
the
refactor-node-impl
branch
from
4b9a4cf to
375e166
Compare
|
Removed unused |
|
ACK 375e166 |
…ave enough c34c500 feat(wire)!: skip fetching seeds from DNS if the `AddressMan` is well populated (Luis Schwab) f2034a8 feat(wire): add `get_addresses_by_service` and `enough_addresses` to `AddressMan` (Luis Schwab) Pull request description: Closes #784. This PR adds some logic to skip fetching peers from DNS if we have enough in the `AddressMan`. The actual values are still up for discussion. ~~I'll rebase once #790 is merged.~~ ACKs for top commit: jaoleal: ACK c34c500 Davidson-Souza: ACK c34c500 JoseSK999: ACK c34c500 Tree-SHA512: 0416ca3b0a790d60c8827ae3188dc269bfdc13584ea2c21b04d8d7a949f69c918499f5c89bca28886f068fcc4eeb32b1b3cebeee40a10bd2dbab4bace0ec5131
3 participants
Description and Notes
So far
node.rshas been a catch-all file, since it was containing the definition for types, macros, and methods for anything needed during theUtreexoNodeoperation. It was already on the 2,250 lines, which makes it hard to reason about and test.I think there are roughly four kind of behaviors:
So I created four files under the new
node/folder for these behaviors.The only code changes were:
broadcast_to_peers,resolve_connect_host,get_portandto_addr_v2pub(crate)Defaultimpl forRunningNodeto its own filemaybe_use_hadcoded_addressestomaybe_use_hardcoded_addresses(typo).Hence, this change is move-only in terms of the method bodies (should be easy to verify).
How to verify the changes you have done?
Should be possible to check that method bodies are the exact same by checking git diffs or something.