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

addrman: ensure old versions don't parse peers.dat #20284

Merged
merged 1 commit into from
Nov 12, 2020

Conversation

vasild
Copy link
Contributor

@vasild vasild commented Nov 2, 2020

Even though the format of peers.dat was changed in a backwards
incompatible way, it is not guaranteed that old versions will fail to
parse it. There is a chance that old versions parse its contents as
garbage and use it.

Old versions expect the "key size" field to be 32 and fail the parsing
if it is not. Thus, we put something other than 32 in it. This will make
versions between 0.11.0 and 0.20.1 deterministically fail on the new
format. Versions prior to #5941
will still parse it as garbage.

Also, introduce a way to increment the peers.dat format in a way that
does not necessary make older versions refuse to read it.

@fanquake fanquake added the P2P label Nov 2, 2020
@vasild
Copy link
Contributor Author

vasild commented Nov 2, 2020

@jnewbery spotted the potential problem here: #19954 (comment)

@jnewbery
Copy link
Contributor

jnewbery commented Nov 2, 2020

Concept ACK.

I think this requires a bit more discussion. I'm going to raise it as a meeting topic for the next p2p meeting (Nov 3).

Marking this as a 0.21 milestone, since we'll need to make a decision before that release.

@jnewbery jnewbery added this to the 0.21.0 milestone Nov 2, 2020
@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 2, 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:

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.

src/addrman.h Outdated
const uint8_t lowest_compatible = compat - m_incompatibility_base;
if (lowest_compatible > maximum_supported_format) {
throw std::ios_base::failure(
strprintf("Unsupported format of addrman database: %u. It is compatible with "
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding an actual versioning mechanism, at least next time we won't have to resort to a hack like this!

@laanwj
Copy link
Member

laanwj commented Nov 2, 2020

Concept ACK.

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.

Concept/light code review ACK. I'll try to test this soon.

@sdaftuar
Copy link
Member

sdaftuar commented Nov 3, 2020

Concept ACK.

Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

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

Concept ACK, could test compatibility check with a fake peers.dat file.

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

review ACK 5cde641

just some style questions

src/addrman.h Outdated
@@ -332,7 +339,12 @@ friend class CAddrManTest;
OverrideStream<Stream> s(&s_, s_.GetType(), s_.GetVersion() | ADDRV2_FORMAT);

s << static_cast<uint8_t>(Format::V3_BIP155);
Copy link
Member

Choose a reason for hiding this comment

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

unrelated style nit: Any reason why this can't use CustomUintFormatter (and its run-time sanity checks)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is possible to use CustomUintFormatter here. This line is not otherwise modified by the patch, so it would be sneaking an unnecessary change, so I left it as is to ease reviewers and future history observers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I changed this line for other reasons and switched to CustomUintFormatter :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

re #20284 (comment). I don't understand why we want this change. The comment for CustomUintFormatter says "This is only intended to implement serializers that are compatible with existing formats, and its use is not recommended for new data structures."

Copy link
Member

Choose a reason for hiding this comment

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

The comment only applies to endianness, see #18317 (comment).
Generally, I think serialize/deserialze should use the same helpers, so either CustomUintFormatter should be used for both paths or for none.

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

@jnewbery jnewbery left a comment

Choose a reason for hiding this comment

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

Concept ACK

The commit log should s/backwards compatible/forwards compatible/. The change in v0.21 is backwards compatible, since the new code can read old files. The change breaks forwards compatibility since old code will not be able to read new files.

This isn't just a pedantic nit. It's the whole point of this PR, so let's get the terminology right.

src/addrman.h Outdated Show resolved Hide resolved
src/addrman.h Outdated Show resolved Hide resolved
src/addrman.h Show resolved Hide resolved
src/addrman.h Outdated Show resolved Hide resolved
src/addrman.h Outdated
//! change is made. This is 32 instead of 0 because we overtook the "key size" field
//! which was 32 historically and we don't want to start with 0 and bump into 32 at
//! some point in the future.
static constexpr uint8_t m_incompatibility_base = 32;
Copy link
Member

Choose a reason for hiding this comment

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

nit: Any value equal to or greater than 32 should be fine here. As this is a breaking change (old versions can't read the peers.dat created by this version of Bitcoin Core), might as well bump to 33 right away to avoid confusion with the "magic value" 32.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or perhaps use 0x80 as a very obvious offset. We're unlikely to have 128 file versions so we're not going to run out of bits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it should be one of 29, 30, 31 or 32:

// Unserialize in 0.21
lowest_compatible = byte_read_from_disk - INCOMPATIBILITY_BASE;

0.20 write 32 to disk

So

  • If INCOMPATIBILITY_BASE is 28 or less then a file written by 0.20 will be rendered unreadable by 0.21 as having too high lowest_compatible: 4 or higher.
  • If INCOMPATIBILITY_BASE is one of 29, 30, 31 or 32 then a file written by 0.20 will appear to have lowest_compatible 3, 2, 1 or 0 which is ok for 0.21.
  • If INCOMPATIBILITY_BASE is 33 or greater then a file written by 0.20 will cause 0.21 to calculate a negative lowest_compatible.

So I left it as 32.

@vasild
Copy link
Contributor Author

vasild commented Nov 10, 2020

The commit log should s/backwards compatible/forwards compatible/.

I think the confusion with forward vs backward compatible 1, 2, 3 is because forward/backward is relative - from the point of view of the new software the old one is backwards and from the point of view of the old software the new one is forwards (in time). Also, there is software version and file format version which makes it further confusing when we consider old software and new file format and new software with old file format.

For example, Wikipedia contains this:

A data format is said to be backward compatible with its predecessor if every message or file that is valid under the old format is still valid, retaining its meaning under the new format.

Which means that our peers.dat format=3 is not backward compatible with its predecessor format=2.

The current master contains this:

0x20 + nKey (serialized as if it were a vector, for backward compatibility)

Do you think that should be s/backward/forward/?

I have reworded the commit message to explain the matters without using the words "backward" or "forward".

@vasild
Copy link
Contributor Author

vasild commented Nov 10, 2020

Addressed suggestions - reworded comments, a log message and renamed two variables.

src/addrman.h Outdated Show resolved Hide resolved
@jnewbery
Copy link
Contributor

jnewbery commented Nov 10, 2020

I think the confusion with forward vs backward compatible...

The fact that there's confusion is exactly the reason we should precise with our language. We have exact terminology for these concepts, so we should use it consistently to avoid future confusion:

  • software is backward-compatible if it can read old input formats
  • software is forward-compatible if it can gracefully read new input formats (ignoring the new parts it can't understand)
    => a change to an input format breaks forward-compatibility if the new file format can't be read by old software.

The current master contains this:

0x20 + nKey (serialized as if it were a vector, for backward compatibility)

Do you think that should be s/backward/forward/?

No, this is for backwards compatibility. That format change broke forward-compatibility explicitly by XOR'ing the number of new buckets with 2**30. Explicitly having the new software write the same value to this field means that the software can read both v0 and v1 without any branching logic at that point. That's the easiest way to achieve backward-compatibility.

@vasild
Copy link
Contributor Author

vasild commented Nov 10, 2020

Addressed suggestions - reworded a comment and moved a local variable as a member of the class.

@jnewbery
Copy link
Contributor

utACK eaa04be

Functionally this is fine and can be merged for 0.21. However, I think that the comments are still very confused/confusing and could be tidied up with precise terminology and clearer explanations.

@maflcko
Copy link
Member

maflcko commented Nov 10, 2020

looks like this doesn't compile:

/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/./addrman.h:438: undefined reference to `CAddrMan::FILE_FORMAT'

@vasild
Copy link
Contributor Author

vasild commented Nov 10, 2020

looks like this doesn't compile:

/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/./addrman.h:438: undefined reference to `CAddrMan::FILE_FORMAT'

Pushed a blind attempt to fix it (it does compile locally).

@sipa
Copy link
Member

sipa commented Nov 10, 2020

  • software is backward-compatible if it can read old input formats
  • software is forward-compatible if it can gracefully read new input formats (ignoring the new parts it can't understand) => a change to an input format breaks forward-compatibility if the new file format can't be read by old software.

Ok, so backward and forward compatibility is a property of the software, not of the formats.

No, this is for backwards compatibility. That format change broke forward-compatibility explicitly by XOR'ing the number of new buckets with 2**30.

I'm not sure that's true according to your definition above. The writing of the 0x20 prefix is there such that old software can still read the nKey (as it used to be serialized as a vector) - the new software doesn't care about the prefix. So I think your definition above says this is forward compatibility: old software reading the new format gracefully.

The same is true about the XORing of the bucket count with 2**30. It's there to trigger a re-bucketing if read by old software, which means it deals with the new format, albeit with reduced functionality (the bucket assignments aren't retained).

V3 is the first actual incompatibility that was introduced: old software cannot read addrv2 encoding at all. It's arguably still "graceful" in that old Bitcoin Core keeps working when it finds a future peers.dat file, but as it means the same as starting with no peers.dat I think it's fair to call that incompatible.

I think the compatibility table is something like this:

Format version Software V0 Software V1 Software V2 Software V3
V0 R R R
V1 R R R
V2 R R
V3

Where "R" means "compatible but may trigger rebucketing".

The upper right triangle represents backward compatibility (and is all R/✓, so we're good). The bottom left triangle represents forward compatibility, and V3 breaks that.

@jnewbery
Copy link
Contributor

So I think your definition above says this is forward compatibility: old software reading the new format gracefully.

The same is true about the XORing of the bucket count with 2**30. It's there to trigger a re-bucketing if read by old software, which means it deals with the new format, albeit with reduced functionality (the bucket assignments aren't retained).

Ah! Those both make sense. Thanks!

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

review ACK b303dc0 🛫

Show signature and timestamp

Signature:

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

review ACK b303dc0d1d487f68220a3a7ba0fea656a69a9b13 🛫
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUj2gQv+JfptLiFyooG83QVtzuWDPAgHEXj2K5OWuSZtd60CFQADbrFYY45LoJvk
Flp9gnZpMS9t5Zg3qJ3WplN+BrOTzIrDZ8e0cENGBxHTlP3KQE0l5sUylOkGBAr0
932meJSIz9S2419dq8D0YaOEMFQo6uckFL378ETPBT4MUtCOViAJ88JJgHzZ7JHj
T8NDkRlO4PAlS3UsylZzboEWqp3s5PhNopLnSvMYIune/ITKOumOWkfhkvHGFnLC
RoD5+HDrHO1yo12oofgrQvxVO5CYszBcdIjFIu+9BiUT3j+oDDvUPa1m0vVFpFjf
4SD5QQ8hiVmwp5SNsgATh8hBwvpd9bKlorH8yI7/J9zwKtKH/cKFN+zsh3UMAvT2
ps8gH2aRgbbpbqUCJGuDw83hSHxe5NFhJgKlBpjc1tWh+1sFb03g40O2n34bi3Nz
/qyBJkpO/gtM2J1UFrk5oDA/bUsETFSJ0++EHj6dsW/qnlZNN2bNqufyZpaS2nxY
LDP1QOux
=MqbG
-----END PGP SIGNATURE-----

Timestamp of file with hash 4c934b14bbb3c840a64b1314b685fd79bf97c3e4a990a9fcec19dbb0b7b0ea8d -

@jnewbery
Copy link
Contributor

Pushed a blind attempt to fix it

I think it'd be better to fix this by just casting the enum to an int in the unserialize exception:

diff --git a/src/addrman.cpp b/src/addrman.cpp
index 0067a32994..7636c6bad2 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -71,8 +71,6 @@ double CAddrInfo::GetChance(int64_t nNow) const
     return fChance;
 }
 
-constexpr CAddrMan::Format CAddrMan::FILE_FORMAT;
-
 CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId)
 {
     std::map<CNetAddr, int>::iterator it = mapAddr.find(addr);
diff --git a/src/addrman.h b/src/addrman.h
index 8c9ed3f563..a18124dcad 100644
--- a/src/addrman.h
+++ b/src/addrman.h
@@ -437,7 +437,7 @@ public:
             throw std::ios_base::failure(
                 strprintf("Unsupported format of addrman database: %u. It is compatible with "
                           "formats >=%u, but the maximum supported by this version of %s is %u.",
-                          format, lowest_compatible, PACKAGE_NAME, FILE_FORMAT));
+                          format, lowest_compatible, PACKAGE_NAME, static_cast<uint8_t>(FILE_FORMAT)));
         }
 
         s >> nKey;

@jnewbery
Copy link
Contributor

utACK 2ed531d

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.

Testing the latest push.

doc/release-notes.md Show resolved Hide resolved
src/addrman.h Outdated Show resolved Hide resolved
@vasild
Copy link
Contributor Author

vasild commented Nov 11, 2020

I think it'd be better to fix this by just casting the enum to an int in the unserialize exception:

That did not compile. Went back to the previous version and addressed Saturn V.

This should be the final version of it.

Even though the format of `peers.dat` was changed in an incompatible
way (old software versions <0.21 cannot understand the new file format),
it is not guaranteed that old versions will fail to parse it. There is a
chance that old versions parse its contents as garbage and use it.

Old versions expect the "key size" field to be 32 and fail the parsing
if it is not. Thus, we put something other than 32 in it. This will make
versions between 0.11.0 and 0.20.1 deterministically fail on the new
format. Versions prior to bitcoin#5941
(<0.11.0) will still parse it as garbage.

Also, introduce a way to increment the `peers.dat` format in a way that
does not necessary make older versions refuse to read it.
@vasild
Copy link
Contributor Author

vasild commented Nov 11, 2020

Nothing is final!

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

re-ACK 38ada89 🥐

Show signature and timestamp

Signature:

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

re-ACK 38ada892ed0ed9aaa46b1791db12a371a3c0c419 🥐
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgATgv9FIjdDlv/Mm+CVB+6f87JOytWGl0Hm74LOEK3aod5FkeO8DTT/sitNJLR
zEW4QEs9aj5nNTSe4+M15cjp2u2pR7BrvhE5HPxkVBZMwSCBDQZecILbsv/CBw8H
/Ku2IJPO5inSaPA9yVa1CF5GAr/o51UdjL97f6MXG/oP6eBUiMau2Lp8v2W+p5Qc
bXmwFcSROMVrTL6+BWP8Z57GOhDwymL9V4ZW4bN0Drm9hpyM2jysKylzRthSdrTo
ZAVTHjc4TIfhJ7PVDltkoWOcTFFVzVde/fVPVnF3nFcwxWjKpeT4d6iKHlzJpDkV
IzQhLg60D+1+tJQINfOfnTvBn9AzUwQT9WXAKgwLqbmCDxHnbPPOiH3aTii8/1Jp
t2xpvoXtz/8lV001nR5lES1XTlSx8M51Iqsz+u/ZhIPPqJIFND4ry19ceYH5OL7S
Qrn0UgAgrmaOOA0vAxLlyfpgCyCh+KhYV3Pp/6CjhsnzCco3jkTIitj6bISEWHwT
lgngxoOH
=eg4T
-----END PGP SIGNATURE-----

Timestamp of file with hash 5da911553b7e1b622981b630b830f42113eaf33856aeae205d8a024c377ec42e -

@jnewbery
Copy link
Contributor

ACK 38ada89

Thanks @vasild!

@maflcko
Copy link
Member

maflcko commented Nov 12, 2020

(the timed-out test run can be ignored)

@laanwj
Copy link
Member

laanwj commented Nov 12, 2020

Code review ACK 38ada89

@laanwj laanwj merged commit 0bd4929 into bitcoin:master Nov 12, 2020
@vasild vasild deleted the peers_dat_format branch November 12, 2020 16:07
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 12, 2020
38ada89 addrman: ensure old versions don't parse peers.dat (Vasil Dimov)

Pull request description:

  Even though the format of `peers.dat` was changed in a backwards
  incompatible way, it is not guaranteed that old versions will fail to
  parse it. There is a chance that old versions parse its contents as
  garbage and use it.

  Old versions expect the "key size" field to be 32 and fail the parsing
  if it is not. Thus, we put something other than 32 in it. This will make
  versions between 0.11.0 and 0.20.1 deterministically fail on the new
  format. Versions prior to bitcoin#5941
  will still parse it as garbage.

  Also, introduce a way to increment the `peers.dat` format in a way that
  does not necessary make older versions refuse to read it.

ACKs for top commit:
  jnewbery:
    ACK 38ada89
  laanwj:
    Code review ACK 38ada89
  MarcoFalke:
    re-ACK 38ada89 🥐

Tree-SHA512: 550bd660c5019dba0f9c334aca8a11c4a0463cfddf11efe7a4a5585ffb05549c82b95066fba5d073ae37893e0eccc158a7ffea9b33ea031d9be4a39e44f6face
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Feb 11, 2021
Summary:
```
Even though the format of `peers.dat` was changed in an incompatible
way (old software versions <0.21 cannot understand the new file format),
it is not guaranteed that old versions will fail to parse it. There is a
chance that old versions parse its contents as garbage and use it.

Old versions expect the "key size" field to be 32 and fail the parsing
if it is not. Thus, we put something other than 32 in it. This will make
versions between 0.11.0 and 0.20.1 deterministically fail on the new
format. Versions prior to bitcoin/bitcoin#5941
(<0.11.0) will still parse it as garbage.

Also, introduce a way to increment the `peers.dat` format in a way that
does not necessary make older versions refuse to read it.
```

Backport of core [[bitcoin/bitcoin#20284 | PR20284]].

Depends on D9201.

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Subscribers: majcosta

Differential Revision: https://reviews.bitcoinabc.org/D9202
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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet