refactor: Move sock from util to common #27989

1. 
   hebasto commented at 10:24 am on June 28, 2023: member

   Networking code should not be required by the kernel.
2. refactor: Move sock from util to common

   Networking code should not be required by the kernel.

   54fd52da65
3. 
   DrahtBot commented at 10:24 am on June 28, 2023: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #27861 (kernel: Rm ShutdownRequested and AbortNode from validation code. by TheCharlatan)
   • #27534 (rpc: add ‘getnetmsgstats’, new rpc to view network message statistics by satsie)
   • #27385 (net, refactor: extract Network and BIP155Network logic to node/network by jonatack)
   • #26812 (test: add end-to-end tests for CConnman and PeerManager by vasild)
   • #25572 (refactor: Introduce EvictionManager and use it for the inbound eviction logic by dergoegge)
   • #25268 (refactor: Introduce EvictionManager by dergoegge)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. DrahtBot added the label Refactoring on Jun 28, 2023
5. 
   TheCharlatan commented at 2:46 pm on June 28, 2023: contributor

   What about all the other networking and wallet related files in /util? E.g. we should probably also move asmap.cpp, bip32.cpp, bytevectorhash.cpp, error.cpp, fees.cpp, syserror.cpp, message.cpp, and readwritefile.cpp out of /util. These pure moves of entire files are very straight forward, so I feel like when we do this, we should do all of them at once as part of integrating the kernel library into the internal compilation units. My fear is that this will otherwise create too much rebase churn for the open PRs if it is done piecemeal.
6. 
   ryanofsky commented at 0:47 am on June 30, 2023: contributor

   Concept -0. I don’t think it’s a good thing to remove everything from util that isn’t used by the kernel. I think util/ is a good home for general purpose code providing basic data structures and cross platform capabilities, and I think common/ is good home for project-specific code that needs to be shared between bitcoind, bitcoin-cli, and bitcoin-wallet.

   It can make sense to move code from util/ to common/ if the code relies on an external dependency, or has global variables, or something else we don’t want in the kernel. It might also make sense to move code to common/ if it is very specific to our own applications and doesn’t belong in a library accessible to other applications.

   But this networking code doesn’t seem much different than the filesystem code in util/fs_helpers.cpp or pipe code added in util/signalinterrupt.cpp from #27861 so I think it fits better in util/ than common/ organizationally. There may be other reasons to move things, but I don’t think “not currently required by kernel code” is enough reason by itself.

7. hebasto closed this on Jun 30, 2023

   ---

8. bitcoin locked this on Jun 29, 2024

Contributors
hebasto DrahtBot TheCharlatan ryanofsky

Labels
Refactoring