Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of VARINT default argument #18087

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

sipa
Copy link
Member

@sipa sipa commented Feb 7, 2020

This removes the need for the non-strandard use of variadic macros.

@fanquake
Copy link
Member

fanquake commented Feb 7, 2020

Thanks. Concept ACK.

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 7, 2020

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #18112 (Serialization improvements step 5 (blockencodings) by sipa)
  • #18000 ([WIP] Coin Statistics Index by fjahr)
  • #17060 (Cache 26% more coins: Reduce CCoinsMap::value_type from 96 to 76 bytes by martinus)

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.

@maflcko
Copy link
Member

maflcko commented Feb 7, 2020

What is the risk or downside of using GNU extensions? Is this a stylistic issue?

@Empact
Copy link
Member

Empact commented Feb 7, 2020

Given the two current modes are:

  • DEFAULT -> unsigned
  • NONNEGATIVE_SIGNED -> signed

Assuming other modes aren't planned. How about using two separate macros to present them: VARINT and VARINT_U (or similar)?

Incidentally, I don't see anywhere we're currently checking that the NONNEGATIVE_SIGNED values are in fact nonnegative.

@sipa
Copy link
Member Author

sipa commented Feb 7, 2020

What is the risk or downside of using GNU extensions? Is this a stylistic issue?

Ping @fanquake, @theuni.

Assuming other modes aren't planned. How about using two separate macros to present them: VARINT and VARINT_U (or similar)?

@ryanofsky What do you think of this suggestion?

Incidentally, I don't see anywhere we're currently checking that the NONNEGATIVE_SIGNED values are in fact nonnegative.

Good point. They're only used in local disk formats, but perhaps an assert makes sense.

@fanquake
Copy link
Member

fanquake commented Feb 7, 2020

Is this a stylistic issue?

@MarcoFalke No. The better place for this discussion would be #18088.

@laanwj
Copy link
Member

laanwj commented Feb 7, 2020

What is the risk or downside of using GNU extensions? Is this a stylistic issue?

It means that a compiler "merely" implementing the C++11 standard could not compile our code.

Maybe all current C++ compilers implement these extensions at the moment, but how would we feel if this would be some proprietary MSVC extension used?

IMO, unless there is another strong reason that gives a clear advantage (say, in efficiency, or correctness/verify ability), it's preferable to not use special extensions.

@maflcko
Copy link
Member

maflcko commented Feb 9, 2020

Concept ACK per @laanwj

Slightly prefer the version by @Empact because it feels odd to explicitly pass in a value that is just called "default"

@practicalswift
Copy link
Contributor

Concept ACK: inside the standard is obviously better than outside the standard :)

Talking about standards: people interested in this PR might be interested in reviewing #17208 ("Make all tests pass UBSan without using any UBSan suppressions") which needs review :)

@practicalswift
Copy link
Contributor

ACK c2dc981 -- patch looks correct

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK c2dc981. Verified changes with

git checkout origin/pull/18087/head
git grep -l VARINT | xargs sed -i 's/VARINT(\([^,]\+\), VarIntMode::DEFAULT)/VARINT(\1\)/'
git diff HEAD^

src/serialize.h Outdated Show resolved Hide resolved
@sipa
Copy link
Member Author

sipa commented Feb 10, 2020

Rebased, and switched to @ryanofsky's VARINT/VARINT_MODE approach (smaller diff!).

src/serialize.h Outdated Show resolved Hide resolved
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK ace3c71. Changes since last review: rebase and adding VARINT_MODE

This removes the need for the GNU C++ extension of variadic macros.
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 0e0fa27. Only change since last review reverting outdated documentation change from earlier version of pr

@maflcko
Copy link
Member

maflcko commented Feb 10, 2020

ACK 0e0fa27 📯

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 0e0fa27acb74b4f0075afcf59a0dff51a21baddb 📯
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgPJwv+JEf3TlslUjdcSz58oj7t8GSnSnNeIwcAh6zSPMC2xxFNDX7ISNolSOnM
Xw1ZFjx7XKxNsc1c/ZfCumK51H59fAXrDPGnXWST50W6lUMK7Ithr/AYtiviTbUL
xNeONWmAtjI4VjFRMtxyDJNJdkRdX7ex7P50av79lQ/DwKvIVcW4k4FPKHO183qr
FJKKPve6kCceMdufWaFHbHqTIUa1CtXOR1w3ZAfLH2X1SDxbSVTXwiJ3UlFwtpaI
jWF8Es9Jmhm3WEFTso+6Rzj61t0MNZa50CAdTV+gk/wptjgrG07ryqaHtCZLWJjf
LdZn0ZHiM5r0F+ArVzTGrtoqPZ2nWwRTaH5VxbK5UwfAe33woO2IFRdldlyJSaGN
cdlBEVYIZNxbgDehwJFytsnoDD+kGDb2bF9e202oJ6DQGP7uhb2vU8aUn7CYPa+X
gtA7UC41YvwVlFfURrOx7LYAFO6E/LChs3sJaZBdyhlrcETLTZxAhEphEHWqdMCZ
ecC5uhfq
=sVpp
-----END PGP SIGNATURE-----

Timestamp of file with hash fe1f05c2963a24a1253bb29b2e921091390831e2f2b6c031b2109dc1573157b6 -

Copy link
Contributor

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 0e0fa27 code review, built/ran tests/bitcoind

@practicalswift
Copy link
Contributor

ACK 0e0fa27 -- diff looks correct

fanquake added a commit that referenced this pull request Feb 11, 2020
0e0fa27 Get rid of VARINT default argument (Pieter Wuille)

Pull request description:

  This removes the need for the non-strandard use of variadic macros.

ACKs for top commit:
  ryanofsky:
    Code review ACK 0e0fa27. Only change since last review reverting outdated documentation change from earlier version of pr
  jonatack:
    ACK 0e0fa27 code review, built/ran tests/bitcoind
  practicalswift:
    ACK 0e0fa27 -- diff looks correct
  MarcoFalke:
    ACK 0e0fa27 📯

Tree-SHA512: 6e335e4b586d62112b7260a12481cd949d1b3bbdb83edf8db690348f0a01852e68504336ff3e072e5131a7c8cb404ef11a2f786f842b8d08bbf6ea0e688777b1
@fanquake fanquake merged commit 0e0fa27 into bitcoin:master Feb 11, 2020
fanquake added a commit that referenced this pull request May 4, 2020
0ae8f18 build: add -Wgnu to compile flags (fanquake)
3a0fd77 Remove use of non-standard zero variadic macros (Ben Woosley)
49f6178 Drop unused LOG_TIME_MICROS helper (Ben Woosley)
5d49999 prevector: Avoid unnamed struct, which is a GNU extension (DesWurstes)

Pull request description:

  Since we [started using](#7165) the `ax_cxx_compile_stdcxx.m4` macro we've been passing `[noext]` to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn _off_ extension support so they would fail to compile".

  However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former.

  #### anonymous structs
  ```bash
  ./prevector.h:153:9: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct]
          struct {
  ```

  This is fixed in b849212.

  #### variadic macros

  ```bash
  ./undo.h:57:50: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]
              ::Unserialize(s, VARINT(nVersionDummy));
  ```

  This is taken care of in #18087.

  The `LOG_TIME_*` macros introduced in #16805 make use of a [GNU extension](https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html).

  ```bash
  In file included from validation.cpp:22:
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
      BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
                                                                                                    ^
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:101:92: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
      BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
                                                                                             ^
  6 warnings generated.
  ```

  This is fixed in 081a0ab and 612e8e1.

  #### prevention
  To ensure that usage doesn't creep back in we can add [`-Wgnu`](https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu) to our compile time flags, which will make Clang warn whenever it encounters GNU extensions.

  This would close #14130.
  Also related to #17230, where it's suggested we use a GNU extension, the `gnu::pure` attribute.

ACKs for top commit:
  practicalswift:
    ACK 0ae8f18 -- diff looks correct
  MarcoFalke:
    ACK 0ae8f18
  vasild:
    utACK 0ae8f18
  dongcarl:
    ACK 0ae8f18

Tree-SHA512: c517404681ef8edf04c785731d26105bac9f3c9c958605aa24cbe399c649e7c5ee0c4aa8e714fd2b2d335e2fbea4d571e09b0dec36678ef871f0a6683ba6bb7f
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 5, 2020
0ae8f18 build: add -Wgnu to compile flags (fanquake)
3a0fd77 Remove use of non-standard zero variadic macros (Ben Woosley)
49f6178 Drop unused LOG_TIME_MICROS helper (Ben Woosley)
5d49999 prevector: Avoid unnamed struct, which is a GNU extension (DesWurstes)

Pull request description:

  Since we [started using](bitcoin#7165) the `ax_cxx_compile_stdcxx.m4` macro we've been passing `[noext]` to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn _off_ extension support so they would fail to compile".

  However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former.

  #### anonymous structs
  ```bash
  ./prevector.h:153:9: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct]
          struct {
  ```

  This is fixed in bitcoin@b849212.

  #### variadic macros

  ```bash
  ./undo.h:57:50: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]
              ::Unserialize(s, VARINT(nVersionDummy));
  ```

  This is taken care of in bitcoin#18087.

  The `LOG_TIME_*` macros introduced in bitcoin#16805 make use of a [GNU extension](https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html).

  ```bash
  In file included from validation.cpp:22:
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
      BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
                                                                                                    ^
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:101:92: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
      BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
                                                                                             ^
  6 warnings generated.
  ```

  This is fixed in 081a0ab and 612e8e1.

  #### prevention
  To ensure that usage doesn't creep back in we can add [`-Wgnu`](https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu) to our compile time flags, which will make Clang warn whenever it encounters GNU extensions.

  This would close bitcoin#14130.
  Also related to bitcoin#17230, where it's suggested we use a GNU extension, the `gnu::pure` attribute.

ACKs for top commit:
  practicalswift:
    ACK 0ae8f18 -- diff looks correct
  MarcoFalke:
    ACK 0ae8f18
  vasild:
    utACK 0ae8f18
  dongcarl:
    ACK 0ae8f18

Tree-SHA512: c517404681ef8edf04c785731d26105bac9f3c9c958605aa24cbe399c649e7c5ee0c4aa8e714fd2b2d335e2fbea4d571e09b0dec36678ef871f0a6683ba6bb7f
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jan 2, 2021
Summary:
> This removes the need for the GNU C++ extension of variadic macros.

This is a backport of Core [[bitcoin/bitcoin#18087 | PR18087]]

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D8763
furszy added a commit to PIVX-Project/PIVX that referenced this pull request Jul 5, 2021
bd4b846 Remove old serialization primitives (Pieter Wuille)
060d62b Convert the last, non-trivial, serialization functions to the new form (furszy)
8c74c09 Convert LimitedString to formatter (Pieter Wuille)
f021897 Fix CDiskBlockIndex serialization of dummy fields for old DB versions (random-zebra)
1ee0cb2 Convert CDiskBlockIndex to new serialization. (furszy)
221bf49 Convert wallet to new serialization (furszy)
cf06950 Convert to new serialization (step 3) (furszy)
dc0fc95 Remove old MESS_VER_STRMESS message version try-catch. (furszy)
35fca11 Convert Qt to new serialization (Pieter Wuille)
3f7826e Add comments to CustomUintFormatter (Pieter Wuille)
eccd473 Convert to new serialization (step 2). Focused on object's serializations that doesn't require an special treatment. (furszy)
0f15784 Convert everything except wallet/qt to new serialization (step 1) (Pieter Wuille)
3d3ee64 Convert merkleblock to new serialization (Pieter Wuille)
13577fb Add SER_READ and SER_WRITE for read/write-dependent statements (Russell Yanofsky)
7344c1a Extend CustomUintFormatter to support enums (Russell Yanofsky)
c4d6228 Merge BigEndian functionality into CustomUintFormatter (Pieter Wuille)
3765d6c Add static_asserts to ser_X_to_Y() methods (Samer Afach)
806213a Fix a violation of C++ standard rules that unions cannot be switched. (Samer Afach)
d6380c4 Add CustomUintFormatter (Pieter Wuille)
fd29a50 Make VectorFormatter support stateful formatters (Russell Yanofsky)
4e2afad Convert CCompactSize to proper formatter (Pieter Wuille)
bb99030 Get rid of VARINT default argument (Pieter Wuille)
e107a0c Convert undo.h to new serialization framework (Pieter Wuille)
a926ba3 Make std::vector and prevector reuse the VectorFormatter logic (Pieter Wuille)
1dfddce Add custom vector-element formatter (Pieter Wuille)
df4e1ba Add a constant for the maximum vector allocation (5 Mbyte) (Pieter Wuille)
c2fdeaf Convert compression.h to new serialization framework (Pieter Wuille)
aa35991 Add FORMATTER_METHODS, similar to SERIALIZE_METHODS, but for formatters (Pieter Wuille)
3e38199 Move compressor utility functions out of class (Pieter Wuille)
7376a95 Convert chain to new serialization (Pieter Wuille)
bbfc55c Convert VARINT to the formatter/Using approach (Pieter Wuille)
39c58a1 Add a generic approach for (de)serialization of objects using code in other classes (Pieter Wuille)
ace3895 Convert addrdb/addrman to new serialization (Pieter Wuille)
6bb135e Introduce new serialization macros without casts (Pieter Wuille)
ace7d14 Drop minor GetSerializeSize template (Ben Woosley)
f05e692 Drop unused GetType() from CSizeComputer (furszy)
5c36b3d Introduce BigEndian wrapper and use it for netaddress ports (Pieter Wuille)
fb3c646 Migrate last FLATDATA calls to use Span. (furszy)
1ef2d90 Support serializing Span<unsigned char> and use that instead of FLATDATA (Pieter Wuille)
8fef544 Add Slice: a (pointer, size) array view that acts like a container (Pieter Wuille)

Pull request description:

  Decoupled from #2411, built on top of #2359.

  Focused on creating the Span class and updating the serialization framework and every object using it up to latest upstream structure (3-4 years ahead of what we currently are in master). We will be up-to-date with them in the area after finishing with #2411 entirely (there are few more updates to the serialization code that comes down #2411 commits line that cannot cherry-pick here).

  Adapted the following PRs:

  *  bitcoin#12886.
  *  bitcoin#12916.
  *  bitcoin#13558.
  *  bitcoin#17850.
  *  bitcoin#17896.
  *  bitcoin#12752.
  *  bitcoin#17957.
  *  bitcoin#18021.
  *  bitcoin#18087.
  *  bitcoin#18112 (only from 353f376 that we don't support).
  *  bitcoin#18167.
  *  bitcoin#18317.
  *  bitcoin#19032.

ACKs for top commit:
  random-zebra:
    ACK bd4b846
  Fuzzbawls:
    ACK bd4b846

Tree-SHA512: fe1b31d0976dff97bfeed0f9efeeb4c6c161277529880ede990c9b3fb0ea680f25b4be01b739f6bf7eeca79fa7687c9c2146c403c96e86bc6b052c6dd88e6930
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Aug 11, 2021
ecde04a [Consensus] Bump Active Protocol version to 70923 for v5.3 (random-zebra)
b63e4f5 Consensus: Add v5.3 enforcement height for testnet. (furszy)
f44be94 Only relay IPv4, IPv6, Tor addresses (Pieter Wuille)
015298c fix: tor: Call event_base_loopbreak from the event's callback (furszy)
34ff7a8 Consensus: Add mnb ADDRv2 guard. (furszy)
b4515dc GUI: Present v3 onion addresses properly in MNs list. (furszy)
337d43d tests: don't export in6addr_loopback (Vasil Dimov)
2cde8e0 GUI: Do not show the tor v3 onion address in the topbar. (furszy)
0b5f406 Doc: update tor.md with latest upstream information. (furszy)
89df7f2 addrman: ensure old versions don't parse peers.dat (Vasil Dimov)
bb90c5c test: add getnetworkinfo network name regression tests (Jon Atack)
d8e01b5 rpc: update GetNetworksInfo() to not return unsupported networks (Jon Atack)
57fc7b0 net: update GetNetworkName() with all enum Network cases (Jon Atack)
647d60b tests: Modify rpc_bind to conform to bitcoin#14532 behaviour. (Carl Dong)
d4d6729 Allow running rpc_bind.py --nonloopback test without IPv6 (Kristaps Kaupe)
4a034d8 test: Add rpc_bind test to default-run tests (Wladimir J. van der Laan)
61a08af [tests] bind functional test nodes to 127.0.0.1  Prevents OSX firewall (Sjors Provoost)
6a4f1e0 test: Add basic addr relay test (furszy)
78aa61c net: Make addr relay mockable (furszy)
ba954ca Send and require SENDADDRV2 before VERACK (Pieter Wuille)
61c2ed4 Bump net protocol version + don't send 'sendaddrv2' to pre-70923 software (furszy)
ccd508a tor: make a TORv3 hidden service instead of TORv2 (Vasil Dimov)
6da9a14 net: advertise support for ADDRv2 via new message (furszy)
e58d5d0 Migrate to test_large_inv() to Misbehaving logging. (furszy)
d496b64 [QA] fix mininode CAddress ser/deser (Jonas Schnelli)
cec9567 net: CAddress & CAddrMan: (un)serialize as ADDRv2 Change the serialization of `CAddrMan` to serialize its addresses in ADDRv2/BIP155 format by default. Introduce a new `CAddrMan` format version (3). (furszy)
b8c1dda streams update: get rid of nType and nVersion. (furszy)
3eaa273 Support bypassing range check in ReadCompactSize (Pieter Wuille)
a237ba4 net: recognize TORv3/I2P/CJDNS networks (Vasil Dimov)
8e50853 util: make EncodeBase32 consume Spans (Sebastian Falbesoner)
1f67e30 net: CNetAddr: add support to (un)serialize as ADDRv2 (Vasil Dimov)
2455420 test: move HasReason so it can be reused (furszy)
d41adb4 util: move HasPrefix() so it can be reused (Vasil Dimov)
f6f86af Unroll Keccak-f implementation (Pieter Wuille)
45222e6 Implement keccak-f[1600] and SHA3-256 (Pieter Wuille)
08ad06d net: change CNetAddr::ip to have flexible size (furszy)
3337219 net: improve encapsulation of CNetAddr. (furszy)
910d5c4 test: Do not instantiate CAddrDB for static call (Hennadii Stepanov)
6b607ef Drop IsLimited in favor of IsReachable (Ben Woosley)
a40711b IsReachable is the inverse of IsLimited (DRY). Includes unit tests (marcaiaf)
8839828 net: don't accept non-left-contiguous netmasks (Vasil Dimov)
5d7f864 rpcbind: Warn about exposing RPC to untrusted networks (Luke Dashjr)
2a6abd8 CNetAddr: Add IsBindAny method to check for INADDR_ANY (Luke Dashjr)
4fdfa45 net: Always default rpcbind to localhost, never "all interfaces" (Luke Dashjr)
31064a8 net: Minor accumulated cleanups (furszy)
9f9c871 tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift)
f6c52a3 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift)
a751b9b net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (furszy)
f30869d test: add IsRFC2544 tests (Mark Tyneway)
ed5abe1 Net: Proper CService deserialization + GetIn6Addr return false if addr isn't an IPv6 addr (furszy)
86d73fb net: save the network type explicitly in CNetAddr (Vasil Dimov)
ad57dfc net: document `enum Network` (Vasil Dimov)
cb160de netaddress: Update CNetAddr for ORCHIDv2 (Carl Dong)
c3c04e4 net: Better misbehaving logging (furszy)
3660487 net: Use C++11 member initialization in protocol (Marco)
082baa3 refactor: Drop unused CBufferedFile::Seek() (Hennadii Stepanov)
e2d776a util: CBufferedFile fixes (Larry Ruane)
6921f42 streams: backport OverrideStream class (furszy)

Pull request description:

  Conjunction of a large number of back ports, updates and refactorings that made with the final goal of implementing v3 Onion addresses support (BIP155 https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki) before the tor v2 addresses EOL, scheduled, by the Tor project, for (1) July 15th: v2 addr support removal from the code base, and (2) October 15th: v2 addr network disable, where **every peer in our network running under Tor will loose the connection and drop the network**.

  As BIP155 describes, this is introducing a new P2P message to gossip longer node addresses over the P2P network. This is required to support new-generation Onion addresses, I2P, and potentially other networks that have longer endpoint addresses than fit in the 128 bits of the current addr message.

  In order to achieve the end goal, had to:
  1.  Create Span class and push it up to latest Bitcoin implementation.
  2.  Update the whole serialization framework and every object using it up to latest Bitcoin implementation (3-4 years ahead of what we currently are in master).
  3.  Update the address manager implementing ASN-based bucketing of the network nodes.
  4.  Update and refactor the netAddress and address manager tests to latest Bitcoin implementation (4 years ahead of what we currently are in master).
  5.  Several util string, vector, encodings, parsing, hashing backports and more..

  Important note:
  This PR it is not meant to be merged as a standalone PR, will decouple smaller ones moving on. Adding on each sub-PR its own description isolated from this big monster.

  Second note:
  This is still a **work-in-progress**, not ready for testing yet. I'm probably missing to mention few PRs that have already adapted to our sources. Just making it public so can decouple the changes, we can start merging them and i can continue working a bit more confortable (rebase a +170 commits separate branch is not fun..).

  ### List of back ported and adapted PRs:

  Span and Serialization:
  ----------------
  *  bitcoin#12886.
  *  bitcoin#12916.
  *  bitcoin#13558.
  *  bitcoin#13697. (Only Span's commit 29943a9)
  *  bitcoin#17850.
  *  bitcoin#17896.
  *  bitcoin#12752.
  *  bitcoin#16577.
  *  bitcoin#16670. (without faebf62)
  *  bitcoin#17957.
  *  bitcoin#18021.
  *  bitcoin#18087.
  *  bitcoin#18112 (only from 353f376 that we don't support).
  *  bitcoin#18167.
  *  bitcoin#18317.
  *  bitcoin#18591 (only Span's commit 0fbde48)
  *  bitcoin#18468.
  *  bitcoin#19020.
  *  bitcoin#19032.
  *  bitcoin#19367.
  *  bitcoin#19387.

  Net, NetAddress and AddrMan:
  ----------------

  *  bitcoin#7932.
  *  bitcoin#10756.
  *  bitcoin#10765.
  *  bitcoin#12218.
  *  bitcoin#12855.
  *  bitcoin#13532.
  *  bitcoin#13575.
  *  bitcoin#13815.
  *  bitcoin#14532.
  *  bitcoin#15051.
  *  bitcoin#15138.
  *  bitcoin#15689.
  *  bitcoin#16702.
  *  bitcoin#17243.
  *  bitcoin#17345.
  *  bitcoin#17754.
  *  bitcoin#17758.
  *  bitcoin#17812.
  *  bitcoin#18023.
  *  bitcoin#18454.
  *  bitcoin#18512.
  *  bitcoin#19314.
  *  bitcoin#19687

  Keys and Addresses encoding:
  ----------------
  * bitcoin#11372.
  * bitcoin#17511.
  * bitcoin#17721.

  Util:
  ----------------
  * bitcoin#9140.
  * bitcoin#16577.
  * bitcoin#16889.
  * bitcoin#19593.

  Bench:
  ----------------
  * bitcoin#16299.

  BIP155:
  ----------------
  *  bitcoin#19351.
  *  bitcoin#19360.
  *  bitcoin#19534.
  *  bitcoin#19628.
  *  bitcoin#19841.
  *  bitcoin#19845.
  *  bitcoin#19954.
  *  bitcoin#19991 (pending).
  *  bitcoin#19845.
  *  bitcoin#20000 (pending).
  *  bitcoin#20120.
  *  bitcoin#20284.
  *  bitcoin#20564.
  *  bitcoin#21157 (pending).
  *  bitcoin#21564 (pending).
  *  Fully removed v2 onion addr support.
  *  Add hardcoded seeds.
  *  Add release-notes, changes to files.md and every needed documentation.

  I'm currently working on the PRs marked as "pending", this isn't over, but I'm pretty pretty close :). What a long road..

ACKs for top commit:
  random-zebra:
    utACK ecde04a
  Fuzzbawls:
    ACK ecde04a

Tree-SHA512: 82c95fbda76fce63f96d8a9af7fa9a89cb1e1b302b7891e27118a6103af0be23606bf202c7332fa61908205e6b6351764e2ec23d753f1e2484028f57c2e8b51a
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants