Base32/base64 are mechanisms for encoding binary data. That they'd
decode to a string is just bizarre. The fact that they'd do that
based on the type of input arguments even more so.
The new locale-independent atoi64 method introduced in #20452 parses
large integer values higher than maximum representable value as 0
instead of the maximum value, which breaks backwards compatibility.
This commit restores compatibility and adds test coverage for this case
in terms of the related GetIntArg and strtoll functions.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
faa3ec2304 span: Add std::byte helpers (MarcoFalke)
fa18038f51 refactor: Use ignore helper when unserializing an invalid pubkey (MarcoFalke)
fabe18d0b3 Use value_type in CDataStream where possible (MarcoFalke)
Pull request description:
This adds (currently unused) span std::byte helpers, so that they can be used in new code.
The refactors are also required for https://github.com/bitcoin/bitcoin/pull/23438, but they are split up because the other pull doesn't compile with msvc right now.
The third commit is not needed for the other pull, but still nice.
ACKs for top commit:
klementtan:
reACK faa3ec2. Verified that all the new `std::byte` helper functions are tested.
laanwj:
Code review ACK faa3ec2304
Tree-SHA512: b1f6af39f03ea4dfebf20d4a8538fa993a6104e7fc92ddf0c4606a7efc3ca9a8c1a4741d98a1418569c11bb9ce9258bf0c0c06d93d85ed7e208902a2db04e407
fa9d72a794 Remove unused ParseDouble and ParsePrechecks (MarcoFalke)
fa3cd28535 refactor: Remove unused ParsePrechecks from ParseIntegral (MarcoFalke)
Pull request description:
All of the `ParsePrechecks` are already done by `ToIntegral`, so remove them from `ParseIntegral`.
Also:
* Remove redundant `{}`. See https://github.com/bitcoin/bitcoin/pull/20457#discussion_r720116866
* Add missing failing c-string test case
* Add missing failing test cases for non-int32_t integral types
ACKs for top commit:
laanwj:
Code review ACK fa9d72a794, good find on ParseDouble not being used at all, and testing for behavior of embedded NULL characters is always a good thing.
practicalswift:
cr ACK fa9d72a794
Tree-SHA512: 3d654dcaebbf312dd57e54241f9aa6d35b1d1d213c37e4c6b8b9a69bcbe8267a397474a8b86b57740fbdd8e3d03b4cdb6a189a9eb8e05cd38035dab195410aa7
Also:
* Remove redundant {} from return statement
* Add missing failing c-string test case and "-" and "+" strings
* Add missing failing test cases for non-int32_t integral types
util: Avoid locale dependent functions strtol/strtoll/strtoul/strtoull in ParseInt32/ParseInt64/ParseUInt32/ParseUInt64
fuzz: Assert equivalence between new and old Parse{Int,Uint}{8,32,64} functions
test: Add unit tests for ToIntegral<T>(const std::string&)
Recognizing addresses from those networks allows us to accept and gossip
them, even though we don't know how to connect to them (yet).
Co-authored-by: eriknylund <erik@daychanged.com>
102867c587 net: change CNetAddr::ip to have flexible size (Vasil Dimov)
1ea57ad674 net: don't accept non-left-contiguous netmasks (Vasil Dimov)
Pull request description:
(chopped off from #19031 to ease review)
Before this change `CNetAddr::ip` was a fixed-size array of 16 bytes,
not being able to store larger addresses (e.g. TORv3) and encoded
smaller ones as 16-byte IPv6 addresses.
Change its type to `prevector`, so that it can hold larger addresses and
do not disguise non-IPv6 addresses as IPv6. So the IPv4 address
`1.2.3.4` is now encoded as `01020304` instead of
`00000000000000000000FFFF01020304`.
Rename `CNetAddr::ip` to `CNetAddr::m_addr` because it is not an "IP" or
"IP address" (TOR addresses are not IP addresses).
In order to preserve backward compatibility with serialization (where
e.g. `1.2.3.4` is serialized as `00000000000000000000FFFF01020304`)
introduce `CNetAddr` dedicated legacy serialize/unserialize methods.
Adjust `CSubNet` accordingly. Still use `CSubNet::netmask[]` of fixed 16
bytes, but use the first 4 for IPv4 (not the last 4). Do not accept
invalid netmasks that have 0-bits followed by 1-bits and only allow
subnetting for IPv4 and IPv6.
Co-authored-by: Carl Dong <contact@carldong.me>
ACKs for top commit:
sipa:
utACK 102867c587
MarcoFalke:
Concept ACK 102867c587
ryanofsky:
Code review ACK 102867c587. Just many suggested updates since last review. Thanks for following up on everything!
jonatack:
re-ACK 102867c587 diff review, code review, build/tests/running bitcoind with ipv4/ipv6/onion peers
kallewoof:
ACK 102867c587
Tree-SHA512: d60bf716cecf8d3e8146d2f90f897ebe956befb16f711a24cfe680024c5afc758fb9e4a0a22066b42f7630d52cf916318bedbcbc069ae07092d5250a11e8f762
Before this change `CNetAddr::ip` was a fixed-size array of 16 bytes,
not being able to store larger addresses (e.g. TORv3) and encoded
smaller ones as 16-byte IPv6 addresses.
Change its type to `prevector`, so that it can hold larger addresses and
do not disguise non-IPv6 addresses as IPv6. So the IPv4 address
`1.2.3.4` is now encoded as `01020304` instead of
`00000000000000000000FFFF01020304`.
Rename `CNetAddr::ip` to `CNetAddr::m_addr` because it is not an "IP" or
"IP address" (TOR addresses are not IP addresses).
In order to preserve backward compatibility with serialization (where
e.g. `1.2.3.4` is serialized as `00000000000000000000FFFF01020304`)
introduce `CNetAddr` dedicated legacy serialize/unserialize methods.
Adjust `CSubNet` accordingly. Still use `CSubNet::netmask[]` of fixed 16
bytes, but use the first 4 for IPv4 (not the last 4). Only allow
subnetting for IPv4 and IPv6.
Co-authored-by: Carl Dong <contact@carldong.me>
Problem:
- Nothing uses the `fspaces` argument to `HexStr()` besides unit
tests. This argument results in extra complexity and a small
performance decrease within the function for branch evalulation.
Solution:
- Remove unused `fspaces` option.
Creates new files util/bip32.h and util/bip32.cpp for containing
BIP 32 stuff.
Moves FormatKeyPath from descriptor.cpp to util/bip32.
Adds a wrapper around it to prepent the 'm' for when just the
BIP 32 style keypath is needed.
Add support for the optional "pf_invalid" out parameter (which allows the caller
to detect decoding failures) to the std::string versions of DecodeBase32 and
DecodeBase64. The char* versions already have this feature.
Also, rename all uses of pfInvalid to pf_invalid to match style guidelines.
Unfortunately, `std::string` elements are (bare) chars. As these
are the most likely type to be passed to these functions, make them use
char instead of unsigned char. This avoids some casts.
