net: improve encapsulation of CNetAddr #19360

pull vasild wants to merge 1 commits into bitcoin:master from vasild:improve_encapsulation_of_cnetaddr changing 2 files +9 −7
  1. vasild commented at 9:27 am on June 23, 2020: member

    Do not access CNetAddr::ip directly from CService methods.

    This improvement will help later when we change the type of CNetAddr::ip (in the BIP155 implementation).

    (chopped off from #19031 to ease review)

  2. fanquake added the label P2P on Jun 23, 2020
  3. vasild commented at 9:32 am on June 23, 2020: member
    All modified code is covered by existent tests. For example: CService::GetKey().
  4. DrahtBot commented at 9:50 am on June 23, 2020: member

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

    Conflicts

    No conflicts as of last run.

  5. naumenkogs commented at 12:32 pm on June 23, 2020: member
    utACK 567f98ea5d33d8cec699148be83b079dd5189101
  6. net: improve encapsulation of CNetAddr
    Do not access `CNetAddr::ip` directly from `CService` methods.
    
    This improvement will help later when we change the type of
    `CNetAddr::ip` (in the BIP155 implementation).
    
    Co-authored-by: Carl Dong <contact@carldong.me>
    bc74a40a56
  7. vasild force-pushed on Jul 10, 2020
  8. vasild commented at 2:02 pm on July 10, 2020: member

    Updated to use CNetAddr::GetAddrBytes(), see #19031 (comment) for longer explanation.

    Now this PR is even simpler.

  9. dongcarl commented at 4:27 pm on July 10, 2020: member

    ACK bc74a40a56128f81f11151d5966f53b82f19038c

    Restarted Travis job

  10. in src/netaddress.cpp:729 in bc74a40a56
    730-     vKey.resize(18);
    731-     memcpy(vKey.data(), ip, 16);
    732-     vKey[16] = port / 0x100; // most significant byte of our port
    733-     vKey[17] = port & 0x0FF; // least significant byte of our port
    734-     return vKey;
    735+    auto key = GetAddrBytes();
    


    jonatack commented at 12:42 pm on July 11, 2020:
    I like auto, but if this is critical code, maybe it would be prudent to use the explicit std::vector<unsigned char> type.
  11. jonatack commented at 12:43 pm on July 11, 2020: member
    ACK bc74a40a56128f81f11151d5966f53b82f19038c
  12. naumenkogs commented at 7:19 am on July 14, 2020: member
    ACK bc74a40
  13. fjahr commented at 12:11 pm on July 15, 2020: member
    Code review ACK bc74a40
  14. in src/netaddress.h:165 in bc74a40a56
    159@@ -160,7 +160,11 @@ class CService : public CNetAddr
    160         CService(const struct in6_addr& ipv6Addr, uint16_t port);
    161         explicit CService(const struct sockaddr_in6& addr);
    162 
    163-        SERIALIZE_METHODS(CService, obj) { READWRITE(obj.ip, Using<BigEndianFormatter<2>>(obj.port)); }
    164+        SERIALIZE_METHODS(CService, obj)
    165+        {
    166+            READWRITEAS(CNetAddr, obj);
    


    jnewbery commented at 4:55 pm on July 15, 2020:
    This should be updated to use the new AsBase<> syntax from #19503 if that PR gets merged first.
  15. jnewbery commented at 4:56 pm on July 15, 2020: member
    ACK bc74a40a5
  16. laanwj commented at 7:17 pm on July 15, 2020: member
    code review ACK bc74a40a56128f81f11151d5966f53b82f19038c
  17. laanwj merged this on Jul 15, 2020
  18. laanwj closed this on Jul 15, 2020

  19. sidhujag referenced this in commit effb941b99 on Jul 16, 2020
  20. vasild deleted the branch on Jul 16, 2020
  21. vasild commented at 10:16 am on July 16, 2020: member

    Thanks for reviewing!

    The reward for work well done is the opportunity to do more. – Jonas Salk

    So, the next one for review is at #19534 :-)

  22. PastaPastaPasta referenced this in commit e8c6989e95 on Jan 16, 2021
  23. UdjinM6 referenced this in commit bce94b7837 on Jan 18, 2021
  24. deadalnix referenced this in commit 2184cc687d on Feb 5, 2021
  25. UdjinM6 referenced this in commit 6e5210adeb on May 28, 2021
  26. random-zebra referenced this in commit b4751e10ce on Aug 11, 2021
  27. DrahtBot locked this on Feb 15, 2022

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-10-08 16:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me