Make all networking code mockable #21878

pull vasild wants to merge 9 commits into bitcoin:master from vasild:Sock_all_over_the_place changing 11 files +144 −47
  1. vasild commented at 4:04 pm on May 7, 2021: contributor

    This is a roadmap PR. It can be merged, but it can also be split into separate PRs and to get proper thorough review it is split.

    Add wrapper methods to the syscalls accept(), setsockopt(), getsockname(), bind(), listen() in the Sock class (e.g. Sock::Accept()). Those methods can be overriden (mocked) by unit tests (existent example in master) and by fuzz tests (existent example in master).

    Change everybody to use Sock instead of SOCKET.

    Move the functionality of CConnman::SocketEvents() to a mockable method of the Sock class.

    Already merged pieces of this PR

    Current pieces of this for review

  2. vasild renamed this:
    fubar everything
    Make all networking code mockable
    on May 7, 2021
  3. DrahtBot added the label P2P on May 7, 2021
  4. DrahtBot added the label Utils/log/libs on May 7, 2021
  5. DrahtBot commented at 9:41 pm on May 7, 2021: 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.

    Type Reviewers
    Concept ACK practicalswift, jonatack, promag, jnewbery, jamesob, josibake

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #27375 (net: support unix domain sockets for -proxy and -onion by pinheadmz)
    • #26812 (test: add end-to-end tests for CConnman and PeerManager by vasild)
    • #26312 (Remove Sock::Get() and Sock::Sock() by vasild)

    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.

  6. practicalswift commented at 1:04 pm on May 8, 2021: contributor
    Strong Concept ACK: mockable means fuzzable.
  7. jonatack commented at 10:37 am on May 10, 2021: contributor
    Concept ACK
  8. in src/net.h:1297 in 96bcc72b5d outdated 
    1292+        }
1293+
1294+        std::vector<CNode*> m_nodes_copy;
1295+
1296+    private:
1297+        CConnman* m_connman;
    promag commented at 2:31 pm on May 12, 2021:

    96bcc72b5dc4f8ba5654d8ddeab844688507f1ce

    nit, * const

    MarcoFalke commented at 3:03 pm on May 12, 2021:
    & is even better
    promag commented at 9:55 pm on May 12, 2021:
    I did though of that but then the caller inits with {*this}?
    vasild commented at 11:58 am on May 13, 2021:
    After reading https://github.com/isocpp/CppCoreGuidelines/issues/222 and https://github.com/isocpp/CppCoreGuidelines/issues/1707 I decided to use a reference instead of a pointer because the member cannot/must not be nullable.
  9. in src/net.h:1294 in 96bcc72b5d outdated 
    1289+            for (auto& node : m_nodes_copy) {
1290+                node->Release();
1291+            }
1292+        }
1293+
1294+        std::vector<CNode*> m_nodes_copy;
    promag commented at 2:32 pm on May 12, 2021:

    96bcc72b5dc4f8ba5654d8ddeab844688507f1ce

    nit, make private and add const getter const std::vector<CNode*>& nodes() const { return m_nodes_copy; }

    vasild commented at 11:58 am on May 13, 2021:
    Done.
  10. promag commented at 2:32 pm on May 12, 2021: member
    Concept ACK.
  11. promag commented at 9:57 pm on May 12, 2021: member
    @vasild I think 96bcc72 and refactors around it could easily be merged on its own.
  12. vasild force-pushed on May 13, 2021
  13. vasild commented at 11:54 am on May 13, 2021: contributor
    0256cfd34...e68bc633f: address review suggestions
  14. vasild commented at 12:34 pm on May 13, 2021: contributor

    @vasild I think 96bcc72 and refactors around it could easily be merged on its own.

    Right! Submitted in #21943.

  15. jnewbery commented at 9:32 am on May 14, 2021: contributor
    Very nice. Strong concept ACK.
  16. vasild force-pushed on May 14, 2021
  17. vasild commented at 10:42 am on May 14, 2021: contributor
    e68bc633f...5772edb36: add an explicit commit that shows we intentionally don’t process newly accepted nodes, as discussed in #21943 (review)
  18. vasild force-pushed on May 17, 2021
  19. vasild commented at 3:22 pm on May 17, 2021: contributor
    5772edb...ec5f887: replicate changes to https://github.com/bitcoin/bitcoin/pull/21943
  20. DrahtBot added the label Needs rebase on May 19, 2021
  21. vasild force-pushed on May 19, 2021
  22. vasild commented at 11:49 am on May 19, 2021: contributor
    ec5f887543...a0b00304a8: rebase due to conflicts
  23. DrahtBot removed the label Needs rebase on May 19, 2021
  24. DrahtBot added the label Needs rebase on May 19, 2021
  25. vasild force-pushed on May 20, 2021
  26. vasild commented at 1:32 pm on May 20, 2021: contributor
    a0b00304a8...6bed261aaf: rebase due to conflicts, follow the trend from #21659 and flag newly added Sock methods with [[nodiscard]]
  27. DrahtBot removed the label Needs rebase on May 20, 2021
  28. vasild force-pushed on May 31, 2021
  29. vasild commented at 8:45 am on May 31, 2021: contributor

    6bed261aaf...272ecad4bf:

    • Rebase
    • Fix #21744
    • To denote an invalid socket: use an empty shared_ptr/unique_ptr to Sock (that owns nullptr) instead of a Sock object that contains m_socket equal to INVALID_SOCKET. This way high-level callers never need to lookup the internal file descriptor of Sock.
  30. vasild force-pushed on May 31, 2021
  31. vasild commented at 2:53 pm on May 31, 2021: contributor
    272ecad4bf...2833c71862: fix a compiler warning
  32. DrahtBot added the label Needs rebase on Jun 1, 2021
  33. vasild force-pushed on Jun 3, 2021
  34. vasild commented at 2:37 pm on June 3, 2021: contributor
    2833c71862...f029732b34: rebase due to conflicts
  35. DrahtBot removed the label Needs rebase on Jun 3, 2021
  36. DrahtBot added the label Needs rebase on Jun 16, 2021
  37. vasild force-pushed on Jun 22, 2021
  38. vasild commented at 8:12 am on June 22, 2021: contributor
    f029732b34...379e9e7279: rebase due to conflicts
  39. DrahtBot removed the label Needs rebase on Jun 22, 2021
  40. vasild force-pushed on Jun 30, 2021
  41. vasild commented at 1:20 pm on June 30, 2021: contributor
    379e9e7279...cdf537545a: during fuzzing, mock also DNS lookup; this is necessary since we now fuzz high level code which may end up doing DNS lookups, for example: CConnman::OpenNetworkConnection() -> CConnman::ConnectNode() -> Lookup()
  42. jamesob commented at 1:23 pm on June 30, 2021: member
    Concept ACK, excited to dig into this.
  43. DrahtBot added the label Needs rebase on Aug 13, 2021
  44. vasild force-pushed on Aug 25, 2021
  45. vasild commented at 11:11 am on August 25, 2021: contributor
    cdf537545a...c98c54e3e3: rebase due to conflicts
  46. DrahtBot removed the label Needs rebase on Aug 25, 2021
  47. DrahtBot added the label Needs rebase on Aug 27, 2021
  48. vasild force-pushed on Sep 28, 2021
  49. vasild commented at 7:37 am on September 28, 2021: contributor
    c98c54e3e3...f8d0ae7e5d: rebase due to conflicts and address review suggestions from child PRs: #21879 (comment), #21943 (comment).
  50. DrahtBot removed the label Needs rebase on Sep 28, 2021
  51. vasild force-pushed on Sep 28, 2021
  52. vasild commented at 1:21 pm on September 28, 2021: contributor
    f8d0ae7e5d...39476dc354: rebase and pick minor changes from child PR #21943 (comment).
  53. DrahtBot added the label Needs rebase on Oct 4, 2021
  54. vasild force-pushed on Oct 8, 2021
  55. vasild commented at 8:39 am on October 8, 2021: contributor
    39476dc354...02aeba0713: rebase due to conflicts
  56. DrahtBot removed the label Needs rebase on Oct 8, 2021
  57. vasild force-pushed on Oct 26, 2021
  58. vasild commented at 8:26 am on October 26, 2021: contributor
    02aeba0713...94f8543cda: rebase and pick changes from child PR: #21943 (comment)
  59. DrahtBot added the label Needs rebase on Nov 8, 2021
  60. vasild commented at 10:06 am on November 10, 2021: contributor
    94f8543cda...3673db7c0a: rebase due to conflicts
  61. vasild force-pushed on Nov 10, 2021
  62. DrahtBot removed the label Needs rebase on Nov 10, 2021
  63. vasild force-pushed on Nov 12, 2021
  64. vasild commented at 10:18 am on November 12, 2021: contributor
    3673db7c0a...ff3d6a5937: pick changes from child PR: #21943 (comment)
  65. DrahtBot added the label Needs rebase on Nov 18, 2021
  66. vasild commented at 4:28 pm on November 18, 2021: contributor
    ff3d6a5937...63280ff095: rebase due to conflicts and pick changes from child PR: #21943 (comment)
  67. vasild force-pushed on Nov 18, 2021
  68. DrahtBot removed the label Needs rebase on Nov 18, 2021
  69. laanwj referenced this in commit 64059b78f5 on Nov 24, 2021
  70. DrahtBot added the label Needs rebase on Nov 24, 2021
  71. vasild force-pushed on Nov 26, 2021
  72. vasild commented at 11:00 am on November 26, 2021: contributor
    63280ff095...8fa0821747: rebase due to conflicts. #21943 which was part of this PR was merged, thus remove its commits from this PR.
  73. DrahtBot removed the label Needs rebase on Nov 26, 2021
  74. MarcoFalke referenced this in commit f2074eeb2d on Dec 1, 2021
  75. DrahtBot added the label Needs rebase on Dec 1, 2021
  76. sidhujag referenced this in commit 7a5e16a331 on Dec 1, 2021
  77. vasild force-pushed on Dec 1, 2021
  78. vasild commented at 3:36 pm on December 1, 2021: contributor
    8fa0821747...9dd36c4a72: rebase due to conflicts
  79. DrahtBot removed the label Needs rebase on Dec 1, 2021
  80. DrahtBot added the label Needs rebase on Dec 6, 2021
  81. vasild force-pushed on Dec 7, 2021
  82. vasild commented at 9:52 am on December 7, 2021: contributor
    9dd36c4a72...ecd2afdbb8: rebase due to conflicts
  83. DrahtBot removed the label Needs rebase on Dec 7, 2021
  84. DrahtBot added the label Needs rebase on Dec 14, 2021
  85. vasild commented at 12:44 pm on December 15, 2021: contributor
    ecd2afdbb8...7e74e1f959: rebase due to conflicts
  86. vasild force-pushed on Dec 15, 2021
  87. DrahtBot removed the label Needs rebase on Dec 15, 2021
  88. josibake commented at 1:50 pm on December 15, 2021: contributor

    Concept ACK

    I see two open PR’s listed in the description as ready for review; are those the best places to start with reviewing this?

  89. vasild commented at 2:15 pm on December 15, 2021: contributor
    @josibake, yes. #21879 and #23604 contain the first commits from this PR. And if you are curious what’s coming afterwards - browse the subsequent commits in this PR. Thanks!
  90. laanwj referenced this in commit 8f1c28a609 on Jan 5, 2022
  91. DrahtBot added the label Needs rebase on Jan 5, 2022
  92. vasild force-pushed on Jan 6, 2022
  93. vasild commented at 10:03 am on January 6, 2022: contributor
    7e74e1f959...cae35129e6: rebase due to conflicts; remove 3 commits from this PR because they got merged via https://github.com/bitcoin/bitcoin/pull/21879
  94. DrahtBot removed the label Needs rebase on Jan 6, 2022
  95. DrahtBot added the label Needs rebase on Jan 17, 2022
  96. in src/net.cpp:431 in cae35129e6 outdated 
    396-            addr_bind.SetSockAddr((const struct sockaddr*)&sockaddr_bind);
397-        } else {
398-            LogPrint(BCLog::NET, "Warning: getsockname failed\n");
399-        }
400+    if (!sock.GetSockName((struct sockaddr*)&sockaddr_bind, &sockaddr_bind_len)) {
401+        addr_bind.SetSockAddr((const struct sockaddr*)&sockaddr_bind);
    PastaPastaPasta commented at 2:06 pm on February 1, 2022:
    please replace these c-style casts with reinterpret_casts
    vasild commented at 4:09 pm on February 11, 2022:
    The cast was not added in this PR. I am leaving it as is because it is unrelated to the change and without it, it is easier to see that only getsockname has been changed to sock.GetSockName.
  97. in src/net.cpp:2192 in cae35129e6 outdated 
    2235@@ -2358,22 +2236,36 @@ bool CConnman::BindListenPort(const CService& addrBind, bilingual_str& strError,
2236 
2237     // Allow binding if the port is still in TIME_WAIT state after
2238     // the program was closed and restarted.
2239-    setsockopt(sock->Get(), SOL_SOCKET, SO_REUSEADDR, (sockopt_arg_type)&nOne, sizeof(int));
2240+    if (sock->SetSockOpt(SOL_SOCKET, SO_REUSEADDR, (sockopt_arg_type)&nOne, sizeof(int)) == SOCKET_ERROR) {
    PastaPastaPasta commented at 2:07 pm on February 1, 2022:
    please replace this c-style cast with a reinterpret_cast (I think)
    vasild commented at 4:09 pm on February 11, 2022:
    See #21878 (review)
  98. in src/net.cpp:2201 in cae35129e6 outdated 
    2247     // some systems don't have IPV6_V6ONLY but are always v6only; others do have the option
2248     // and enable it by default or not. Try to enable it, if possible.
2249     if (addrBind.IsIPv6()) {
2250 #ifdef IPV6_V6ONLY
2251-        setsockopt(sock->Get(), IPPROTO_IPV6, IPV6_V6ONLY, (sockopt_arg_type)&nOne, sizeof(int));
2252+        if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_V6ONLY, (sockopt_arg_type)&nOne, sizeof(int)) == SOCKET_ERROR) {
    PastaPastaPasta commented at 2:08 pm on February 1, 2022:
    same (reinterpret_cast)
    vasild commented at 4:09 pm on February 11, 2022:
    See #21878 (review)
  99. in src/net.cpp:2208 in cae35129e6 outdated 
    2257+        }
2258 #endif
2259 #ifdef WIN32
2260         int nProtLevel = PROTECTION_LEVEL_UNRESTRICTED;
2261-        setsockopt(sock->Get(), IPPROTO_IPV6, IPV6_PROTECTION_LEVEL, (const char*)&nProtLevel, sizeof(int));
2262+        if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_PROTECTION_LEVEL, (const char*)&nProtLevel, sizeof(int)) == SOCKET_ERROR) {
    PastaPastaPasta commented at 2:08 pm on February 1, 2022:
    same
    vasild commented at 4:09 pm on February 11, 2022:
    See #21878 (review)
  100. in src/netbase.cpp:480 in cae35129e6 outdated 
    510@@ -511,19 +511,23 @@ std::unique_ptr<Sock> CreateSockTCP(const CService& address_family)
511     int set = 1;
512     // Set the no-sigpipe option on the socket for BSD systems, other UNIXes
513     // should use the MSG_NOSIGNAL flag for every send.
514-    setsockopt(hSocket, SOL_SOCKET, SO_NOSIGPIPE, (void*)&set, sizeof(int));
515+    if (sock->SetSockOpt(SOL_SOCKET, SO_NOSIGPIPE, (void*)&set, sizeof(int)) == SOCKET_ERROR) {
    PastaPastaPasta commented at 2:09 pm on February 1, 2022:
    reinterpret_cast
    vasild commented at 4:09 pm on February 11, 2022:
    See #21878 (review)
  101. in src/netbase.cpp:517 in cae35129e6 outdated 
    546@@ -543,10 +547,6 @@ bool ConnectSocketDirectly(const CService &addrConnect, const Sock& sock, int nT
547     // Create a sockaddr from the specified service.
548     struct sockaddr_storage sockaddr;
549     socklen_t len = sizeof(sockaddr);
550-    if (sock.Get() == INVALID_SOCKET) {
551-        LogPrintf("Cannot connect to %s: invalid socket\n", addrConnect.ToString());
552-        return false;
553-    }
554     if (!addrConnect.GetSockAddr((struct sockaddr*)&sockaddr, &len)) {
    PastaPastaPasta commented at 2:09 pm on February 1, 2022:
    reinterpret_cast
    vasild commented at 4:09 pm on February 11, 2022:
    See #21878 (review)
  102. laanwj referenced this in commit e8a3882e20 on Feb 4, 2022
  103. vasild force-pushed on Feb 11, 2022
  104. vasild commented at 4:53 pm on February 11, 2022: contributor
    cae35129e6...4a26bcd3fe: rebase due to conflicts, remove 3 commits from this PR because they got merged via https://github.com/bitcoin/bitcoin/pull/23604
  105. DrahtBot removed the label Needs rebase on Feb 11, 2022
  106. vasild force-pushed on Feb 15, 2022
  107. vasild commented at 4:33 pm on February 15, 2022: contributor
    4a26bcd3fe...bca585381d: rebase and minor improvements after self-review and before chopping off the next piece into its own PR: rename Sock::WaitEvents to Sock::Events and make it possible to define an instance of it without specifying the “occurred” event, which does not make sense anyway.
  108. DrahtBot added the label Needs rebase on Apr 14, 2022
  109. vasild force-pushed on Apr 15, 2022
  110. vasild commented at 11:21 am on April 15, 2022: contributor
    bca585381d...b73dcbc24e: rebase due to conflicts and pick changes from child PRs
  111. DrahtBot removed the label Needs rebase on Apr 15, 2022
  112. laanwj referenced this in commit 6300b9556e on Apr 19, 2022
  113. DrahtBot added the label Needs rebase on Apr 19, 2022
  114. vasild force-pushed on Apr 19, 2022
  115. vasild commented at 4:42 pm on April 19, 2022: contributor
    b73dcbc24e...a8f889d5e2: rebase due to conflicts
  116. DrahtBot removed the label Needs rebase on Apr 19, 2022
  117. sidhujag referenced this in commit f41c649dc6 on Apr 19, 2022
  118. DrahtBot added the label Needs rebase on Apr 22, 2022
  119. vasild commented at 4:12 pm on April 27, 2022: contributor
    a8f889d5e2...e834d610ff: rebase due to conflicts
  120. vasild force-pushed on Apr 27, 2022
  121. DrahtBot removed the label Needs rebase on Apr 27, 2022
  122. DrahtBot added the label Needs rebase on May 16, 2022
  123. vasild force-pushed on May 19, 2022
  124. vasild commented at 3:35 pm on May 19, 2022: contributor
    e834d610ff...f3e35c4d6c: rebase due to conflicts and pick changes from child PR #24356 (comment)
  125. DrahtBot removed the label Needs rebase on May 19, 2022
  126. DrahtBot added the label Needs rebase on May 24, 2022
  127. vasild force-pushed on May 27, 2022
  128. vasild commented at 9:05 am on May 27, 2022: contributor
    f3e35c4d6c...0aae576f68: rebase due to conflicts
  129. DrahtBot removed the label Needs rebase on May 27, 2022
  130. laanwj referenced this in commit 0ea92cad52 on Jun 16, 2022
  131. DrahtBot added the label Needs rebase on Jun 16, 2022
  132. sidhujag referenced this in commit 10043f624c on Jun 17, 2022
  133. vasild commented at 4:20 pm on June 17, 2022: contributor
    0aae576f68...6077fac5ef: rebase due to conflicts
  134. vasild force-pushed on Jun 17, 2022
  135. DrahtBot removed the label Needs rebase on Jun 17, 2022
  136. laanwj referenced this in commit a085a55491 on Jun 22, 2022
  137. DrahtBot added the label Needs rebase on Jun 22, 2022
  138. vasild force-pushed on Jun 22, 2022
  139. vasild commented at 12:22 pm on June 22, 2022: contributor
    6077fac5ef...82a6d4576e: rebase due to conflicts
  140. DrahtBot removed the label Needs rebase on Jun 22, 2022
  141. laanwj referenced this in commit ba29911e21 on Jun 28, 2022
  142. DrahtBot added the label Needs rebase on Jun 28, 2022
  143. vasild force-pushed on Jun 28, 2022
  144. vasild commented at 12:31 pm on June 28, 2022: contributor
    82a6d4576e...a6424d5023: rebase due to conflicts
  145. laanwj referenced this in commit 55c9e2d790 on Jun 28, 2022
  146. vasild force-pushed on Jun 28, 2022
  147. vasild commented at 1:39 pm on June 28, 2022: contributor
    a6424d5023...12f848b5cd: rebase due to conflicts
  148. DrahtBot removed the label Needs rebase on Jun 28, 2022
  149. sidhujag referenced this in commit 9957e2cd9f on Jun 28, 2022
  150. sidhujag referenced this in commit 968d887367 on Jun 28, 2022
  151. DrahtBot added the label Needs rebase on Jul 20, 2022
  152. vasild commented at 2:28 pm on July 20, 2022: contributor
    12f848b5cd...f07b89e917: rebase due to conflicts
  153. vasild force-pushed on Jul 20, 2022
  154. DrahtBot removed the label Needs rebase on Jul 20, 2022
  155. DrahtBot added the label Needs rebase on Jul 27, 2022
  156. achow101 commented at 6:28 pm on October 12, 2022: member
    Should this be marked as draft since it is a parent PR?
  157. glozow referenced this in commit 7e1007a3c6 on Oct 12, 2022
  158. jonatack commented at 8:02 pm on October 12, 2022: contributor
    @vasild this may be able to be closed now that all of the child PRs have been merged.
  159. fanquake commented at 2:57 am on October 13, 2022: member

    vasild this may be able to be closed now that all of the child PRs have been merged.

    Given that the majority of commits in this pull haven’t been merged, I don’t think so.

  160. vasild force-pushed on Oct 14, 2022
  161. vasild commented at 8:38 am on October 14, 2022: contributor

    f07b89e917...9a27807453: rebase due to conflicts

    Should this be marked as draft since it is a parent PR?

    Could be, I am not sure. The reason I did not make it a draft is because it can be merged as it is - there is no danger that something bad will happen if it gets merged (assuming there are reviewers who ACK it). The reason I split it into smaller PRs is solely to ease review. Should I make it draft?

    @vasild this may be able to be closed now that all of the child PRs have been merged.

    Given that the majority of commits in this pull haven’t been merged, I don’t think so.

    Right, there are two pieces left - #26312 which I just opened and the tests themselves (which depend on that PR, thus no separate PR for them yet).

  162. DrahtBot removed the label Needs rebase on Oct 14, 2022
  163. sidhujag referenced this in commit f73aef5a64 on Oct 23, 2022
  164. DrahtBot added the label Needs rebase on Nov 25, 2022
  165. vasild force-pushed on Dec 12, 2022
  166. vasild commented at 10:44 am on December 12, 2022: contributor
    9a27807453...77ab5e8e98: rebase due to conflicts
  167. DrahtBot removed the label Needs rebase on Dec 12, 2022
  168. DrahtBot added the label Needs rebase on Feb 1, 2023
  169. vasild force-pushed on Feb 9, 2023
  170. vasild commented at 2:04 pm on February 9, 2023: contributor
    77ab5e8e98...b497200c7a: rebase due to conflicts
  171. DrahtBot removed the label Needs rebase on Feb 9, 2023
  172. DrahtBot added the label Needs rebase on Feb 17, 2023
  173. achow101 marked this as a draft on Apr 25, 2023
  174. i2p: avoid using Sock::Get() for checking for a valid socket 
    Peeking at the underlying socket file descriptor of `Sock` and checkig
if it is `INVALID_SOCKET` is bad encapsulation and stands in the way of
testing/mocking/fuzzing.

Instead use an empty unique_ptr to denote that there is no valid socket.
    1a23f0cfd6
  175. net: don't check if the socket is valid in GetBindAddress() 
    The socket is always valid (the underlying file descriptor is not
`INVALID_SOCKET`) when `GetBindAddress()` is called.
    f5c57839ea
  176. net: don't check if the socket is valid in ConnectSocketDirectly() 
    The socket is always valid (the underlying file descriptor is not
`INVALID_SOCKET`) when `ConnectSocketDirectly()` is called.
    7426958ad4
  177. net: remove now unnecessary Sock::Get() 
    `Sock::Get()` was used only in `sock.{cpp,h}`. Remove it and access
`Sock::m_socket` directly.

Unit tests that used `Get()` to test for equality still verify that the
behavior is correct, indirectly, by testing whether the socket is closed
or not.
    b740d39dd4
  178. net: remove Sock default constructor, it's not necessary 2bcbe87424
  179. fuzz: add CConnman::OpenNetworkConnection() to the tests 
    Now that all network calls done by `CConnman::OpenNetworkConnection()`
are done via `Sock` they can be redirected (mocked) to `FuzzedSocket`
for testing.
    618b3ec0b2
  180. fuzz: add CConnman::CreateNodeFromAcceptedSocket() to the tests 1545b55517
  181. fuzz: add CConnman::InitBinds() to the tests bb98574e4b
  182. fuzz: add CConnman::SocketHandler() to the tests 80d9dfd0f0
  183. vasild force-pushed on Aug 16, 2023
  184. vasild commented at 1:27 pm on August 16, 2023: contributor
    b497200c7a...80d9dfd0f0: rebase due to conflicts
  185. DrahtBot removed the label Needs rebase on Aug 16, 2023
  186. ryanofsky referenced this in commit d0b928b29d on Oct 3, 2023
  187. DrahtBot added the label Needs rebase on Oct 3, 2023
  188. DrahtBot commented at 3:03 pm on October 3, 2023: contributor

    🐙 This pull request conflicts with the target branch and needs rebase.

  189. vasild commented at 11:34 am on October 4, 2023: contributor

    All the functional bits of this are now merged. Thank you all!

    This enables extending the test framework to be able to call any method, without worries that it may end up doing OS syscalls (try to create sockets, open connections, etc). A PR that does some of that is in #28584 (= last 4 commits from this PR).

    Closing as complete.

  190. vasild closed this on Oct 4, 2023
  191. vasild deleted the branch on Oct 4, 2023


vasild DrahtBot practicalswift jonatack promag MarcoFalke jnewbery jamesob josibake PastaPastaPasta achow101 fanquake

Labels
P2P Utils/log/libs Needs rebase

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-03 10:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me