Changes to make MaybeSendAddr simpler and easier to maintain/update:
- assert invariant that node.vAddrToSend.size() can never exceed
MAX_ADDR_TO_SEND
- erase known addresses from vAddrToSend in one pass
- no check for (vAddr.size() >= MAX_ADDR_TO_SEND) during iteration,
since vAddr can never exceed MAX_ADDR_TO_SEND.
After reading
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.70/autoconf.html#Specifying-Target-Triplets,
my understanding is that this change should mostly be a no-op, as
--build defaults to the output of config.guess, however, this may be
slightly more correct
> For historical reasons, whenever you specify --host, be sure to
> specify --build too; this will be fixed in the future.
and will quell some warnings in depends (#16354). If anything, this
also explicitly enables cross-compilation mode when `--host` differs
from `--build`.
As for "fixed in the future", this is the case for Autoconf 2.70+.
4783115fd4 net: add ifaddrs.h include (fanquake)
879215e665 build: check if -lsocket is required with *ifaddrs (fanquake)
87deac66aa rand: only try and use freeifaddrs if available (fanquake)
Pull request description:
Fixes#21485 by linking against `-lsocket` when it's required for using `*ifaddrs` functions.
ACKs for top commit:
laanwj:
Code review ACK 4783115fd4
hebasto:
ACK 4783115fd4, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 4542e036e9b029de970eff8a9230fe45d9204bb22313d075f474295d49bdaf1f1cbb36c0c6e2fa8dbbcdba518d8d3a68a6116ce304b82414315f333baf9af0e4
b8e76479ce ci: Bump macOS VM image to the latest version (Hennadii Stepanov)
Pull request description:
On Cirrus CI bump macOS VM from Catalina to Big Sur.
ACKs for top commit:
fanquake:
ACK b8e76479ce - I'm always going to question bumping things for the sake of it/when there's no reasoning given. In this case, moving from building on macOS 10.x to 11.x shouldn't really lose us any coverage, and may turn up potential macOS quirks a bit earlier.
Tree-SHA512: ab2daee194683ab0553328020fd2fcb918160f466cd380c542e1a9b44f5bea3664fb40b032f1b611ee0107b0efbe278230e067316e2373c3cb0470b205dd2f9d
765e0be534 build: split native_cctools (fanquake)
Pull request description:
This splits our native cctools package into two additional packages: `native_clang` and `native_libtapi`. This is in an effort to not only make our mac toolchain more understandable, but also to reduce duplication, and as a nice side-effect, fix the issue mentioned [here](https://github.com/bitcoin/bitcoin/pull/19817#issuecomment-741763289).
Everything about the current build process should remain the same. For gitian, that is:
* Download LLVM Clang 8.0.0 binary.
* Build libtapi using downloaded Clang.
* Build cctools using downloaded Clang and libtapi.
* Build the rest of depends..
For Guix (`FORCE_USE_SYSTEM_CLANG=1`):
* Build libtapi using using system Clang.
* Build cctools using system Clang and libtapi.
* Build the rest of depends..
After this has been merged, my plan is to combine a modified version of #20454 and #21414 with #19817, and from what I understand that will be enough to support Apple Arm cross compilation.
Builds at 477ed59f49f39ae4272219a1216ee19fb72fdde5:
Guix:
```bash
find output -type f -name *$(git rev-parse --short HEAD)*.* -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
7e57b9e5a2109d1a35f0091d86f975c1b1d73ac70ac2609cefbe1134efbf2c87 output/bitcoin-477ed59f49f3-osx-unsigned.dmg
dd11e71c2634ac2fa883d1e45cbd6de194fad37624bb56b8b8a6213fd40d6050 output/bitcoin-477ed59f49f3-osx-unsigned.tar.gz
64384eaa2fd9768992d86a06a1414c9e92e84ba21a875696483df2bb5828e2a2 output/bitcoin-477ed59f49f3-osx64.tar.gz
8a889e88db952d2c82ef44713c04aba95b777441f578738ff6d8a0d251e51da3 output/src/bitcoin-477ed59f49f3.tar.gz
```
Gitian:
```bash
d0eee8542d5f3d662555ad7218d2dc9f3f862656e65bcb2f01f256bfa0deead2 bitcoin-477ed59f49f3-osx-unsigned.dmg
ba7bc94897e42e7a037e352c4e4e1730f181c6d76b6d6a2785bbd7bf85614c83 bitcoin-477ed59f49f3-osx-unsigned.tar.gz
c4426d1d310a2fbffcaf2b7df0da4ec97bd11aab07085006dae68777b03f6372 bitcoin-477ed59f49f3-osx64.tar.gz
8a889e88db952d2c82ef44713c04aba95b777441f578738ff6d8a0d251e51da3 src/bitcoin-477ed59f49f3.tar.gz
a746831467dc8ff17ec5df06fc9288a859c1961d8c0b632d97b42f080dbd825d bitcoin-core-osx-22-res.yml
```
ACKs for top commit:
dongcarl:
reACK 765e0be534
hebasto:
ACK 765e0be534, verified building of the `native_cctools` package step-by-step.
Tree-SHA512: 61cf2b092fb8b9724adda1084e0cac9db889cd5e391914b43592aebc470fae3c1cbabc8b59a0abce6e7bad8c646694fe927f26f4deb18b60d7fd92f374f62feb
fa4926cca6 fuzz: [refactor] Use IsValidFlagCombination in signature_checker fuzz target (MarcoFalke)
eeee8f5be1 fuzz: Removed unused try-catch in coins_view (MarcoFalke)
fa98f3f66e fuzz: [refactor] Use ConsumeScript in signature_checker fuzz target (MarcoFalke)
Pull request description:
Some small refactors to remove unused and redundant fuzz code
ACKs for top commit:
practicalswift:
cr re-ACK fa4926cca6
Tree-SHA512: eb07a2140caad7b31495b76385fc7634cf5b6daa4947f430ebb127eb1375583dc11e541a0a42d0e5d93d430480b8a815b93974450fd5ed897528a2d47c752f86
40316a37cb test: add I2P test for a runaway SAM proxy (Vasil Dimov)
2d8ac77970 fuzz: add tests for the I2P Session public interface (Vasil Dimov)
9947e44de0 i2p: use pointers to Sock to accommodate mocking (Vasil Dimov)
82d360b5a8 net: change ConnectSocketDirectly() to take a Sock argument (Vasil Dimov)
b5861100f8 net: add connect() and getsockopt() wrappers to Sock (Vasil Dimov)
5a887d49b2 fuzz: avoid FuzzedSock::Recv() repeated errors with EAGAIN (Vasil Dimov)
3088f83d01 fuzz: extend FuzzedSock::Recv() to support MSG_PEEK (Vasil Dimov)
9b05c49ade fuzz: implement unimplemented FuzzedSock methods (Vasil Dimov)
Pull request description:
Change the networking code and the I2P code to be fully mockable and use `FuzzedSocket` to fuzz the I2P methods `Listen()`, `Accept()` and `Connect()`.
Add a mocked `Sock` implementation that returns a predefined data on reads and use it for a regression unit test for the bug fixed in https://github.com/bitcoin/bitcoin/pull/21407.
ACKs for top commit:
practicalswift:
Tested ACK 40316a37cb
MarcoFalke:
Concept ACK 40316a37cb
jonatack:
re-ACK 40316a37cb reviewed `git range-diff 01bb3afb 23c861d 40316a3` and the new unit test commit, debug built, ran unit tests, ran bitcoind with an I2P service and network operation with seven I2P peers (2 in, 5 out) is looking nominal
laanwj:
Code review ACK 40316a37cb
Tree-SHA512: 7fc4f129849e16e0c7e16662d9f4d35dfcc369bb31450ee369a2b97bdca95285533bee7787983e881e5a3d248f912afb42b4a2299d5860ace7129b0b19623cc8
0cca08a8ee Add unit test coverage for our onion peer eviction protection (Jon Atack)
caa21f586f Protect onion+localhost peers in ProtectEvictionCandidatesByRatio() (Jon Atack)
8f1a53eb02 Use EraseLastKElements() throughout SelectNodeToEvict() (Jon Atack)
8b1e156143 Add m_inbound_onion to AttemptToEvictConnection() (Jon Atack)
72e30e8e03 Add unit tests for ProtectEvictionCandidatesByRatio() (Jon Atack)
ca63b53ecd Use std::unordered_set instead of std::vector in IsEvicted() (Jon Atack)
41f84d5ecc Move peer eviction tests to a separate test file (Jon Atack)
f126cbd6de Extract ProtectEvictionCandidatesByRatio from SelectNodeToEvict (Jon Atack)
Pull request description:
Now that #19991 and #20210 have been merged, we can determine inbound onion peers using `CNode::m_inbound_onion` and add it to the localhost peers protection in `AttemptToEvictConnection`, which was added in #19670 to address issue #19500.
Update 28 February 2021: I've updated this to follow gmaxwell's suggestion in https://github.com/bitcoin/bitcoin/pull/20197#issuecomment-713865992.
This branch now protects up to 1/4 onion peers (connected via our tor control service), if any, sorted by longest uptime. If any (or all) onion slots remain after that operation, they are then allocated to protect localhost peers, or a minimum of 2 localhost peers in the case that no onion slots remain and 2 or more onion peers were protected, sorted as before by longest uptime.
This patch also adds test coverage for the longest uptime, localhost, and onion peer eviction protection logic to build on the welcome initial unit testing of #20477.
Suggest reviewing the commits that move code with `colorMoved = dimmed-zebra` and `colorMovedWs = allow-indentation-change`.
Closes#11537.
ACKs for top commit:
laanwj:
Code review ACK 0cca08a8ee
vasild:
ACK 0cca08a8ee
Tree-SHA512: 2f5a63f942acaae7882920fc61f0185dcd51da85e5b736df9d1fc72343726dd17da740e02f30fa5dc5eb3b2d8345707aed96031bec143d48a2497a610aa19abd
3fc06d3d7b [net] remove fUpdateConnectionTime from FinalizeNode (John Newbery)
7c4cc67c0c [net] remove CConnman::AddNewAddresses (John Newbery)
bcd7f30b79 [net] remove CConnman::MarkAddressGood (John Newbery)
8073673dbc [net] remove CConnman::SetServices (John Newbery)
392a95d393 [net_processing] Keep addrman reference in PeerManager (John Newbery)
1c25adf6d2 [net] Construct addrman outside connman (John Newbery)
Pull request description:
Addrman is currently a member variable of connman. Make it a top-level component with lifetime owned by node.context, and add a reference to addrman in peerman. This allows us to eliminate some functions in connman that are simply forwarding requests to addrman, and simplifies the connman-peerman interface.
By constructing the addrman in init, we can also add parameters to the ctor, which allows us to test it better. See #20233, where we enable consistency checking for addrman in our functional tests.
ACKs for top commit:
MarcoFalke:
re-ACK 3fc06d3d7b only change is squash 🏀
vasild:
ACK 3fc06d3d7b
Tree-SHA512: 17662c65cbedcd9bd1c194914bc4bb4216f4e3581a06222de78f026d6796f1da6fe3e0bf28c2d26a102a12ad4fbf13f815944a297f000e3acf46faea42855e07
This changes all context dependent checks in the parser to be
disjunctions of equality checks, rather than also including inequalities.
This makes sure that adding a new context enum in the future won't change
semantics for existing checks.
The error messages are also made a bit more consistent.
This is a preparation for parsing xonly pubkeys, which will complicate
this logic. It's cleaner to put the decision logic close to the public
key parsing itself.
7c543bc27b build: Enable safe warnings for msvc builds (Hennadii Stepanov)
Pull request description:
Disabling of some specific warnings no longer needed.
ACKs for top commit:
MarcoFalke:
ACK 7c543bc27b if green
practicalswift:
ACK 7c543bc27b if green: compiler warnings are good, compiler safety warnings are great
sipsorcery:
tACK 7c543bc27b.
Tree-SHA512: a8253a5a0ccc22b8622ae66efae77a163c54bbd6dbd7e28b043ad4eb474b2c8e5c7b22d3870b15a2b9b91bade0a5ae5bfc5ee765d132dc32ddbbe690fb0146d9
This has no effect for now, as the only fragments with sub-script
expressions (sh, wsh) only allow one, and don't have key expressions
in them.
A future Taproot descriptor will however violate both, and we want
the keys in different sub-scripts to be assigned non-overlapping
cache indices.
So far, no descriptor exists that supports more than one sub-script
descriptor. This will change with taproot, so prepare for this by
changing the m_subdescriptor_arg from a unique_ptr to a vector of
unique_ptr's.
There are currently two DescriptorImpl subclasses that rely on the functionality
that ExpandHelper automatically adds subscripts to the output SigningProvider.
Taproot descriptors will have subscripts, but we don't want them in the
SigningProvider's bare script field. To avoid them ending up there, move this
functionality into the specific classes' MakeScripts implementation.
90ae3d8ca6 doc: Add release notes for -deprecatedrpc=addresses and bitcoin-tx (Michael Dietz)
085b3a7299 rpc: deprecate `addresses` and `reqSigs` from rpc outputs (Michael Dietz)
Pull request description:
Considering the limited applicability of `reqSigs` and the confusing output of `1` in all cases except bare multisig, the `addresses` and `reqSigs` outputs are removed for all rpc commands.
1) add a new sane "address" field (for outputs that have an identifiable address, which doesn't include bare multisig)
2) with -deprecatedrpc: leave "reqSigs" and "addresses" intact (with all weird/wrong behavior they have now)
3) without -deprecatedrpc: drop "reqSigs" and "addresses" entirely always.
Note: Some light refactoring done to allow us to very easily delete a few chunks of code (marked with TODOs) when we remove this deprecated behavior.
Using `IsDeprecatedRPCEnabled` in core_write.cpp caused some circular dependencies involving core_io
Circular dependencies were caused by rpc/util unnecessarily importing node/coinstats and node/transaction. Really what rpc/util needs are some fundamental type/helper-function definitions. So this was cleaned up to make more sense.
This fixes#20102.
ACKs for top commit:
MarcoFalke:
re-ACK 90ae3d8ca6📢
Tree-SHA512: 8ffb617053b5f4a8b055da17c06711fd19632e0037d71c4c8135e50c8cd7a19163989484e4e0f17a6cc48bd597f04ecbfd609aef54b7d1d1e76a784214fcf72a
Add early exit guard clauses if node.RelayAddrsWithConn() is false or if
current_time < node.m_next_addr_send. Add comments.
This commit leaves some lines over-indented. Those will be fixed in a
subsequent whitespace-only commit.
beead33a21 [test] no send feefilters when txrelay is turned off (glozow)
18a9b27dd6 p2p: Don't send FEEFILTER in blocksonly mode (Martin Zumsande)
Pull request description:
The purpose of FEEFILTER messages (BIP 133) is to inform our peers that we do not want transactions below a specified fee rate.
In blocksonly mode, we do not want our peer to send us any transactions at all (and will disconnect if a peer still sends a transaction INV or TX).
Therefore, I don't think that it makes sense to send FEEFILTER messages every 10 minutes on average in blocksonly mode - this PR disables it.
Note that on block-relay-only connections, FEEFILTER is already disabled, just not in blocksonly mode.
ACKs for top commit:
glozow:
re ACK beead33a21🙂 thanks for adding the test!
amitiuttarwar:
reACK beead33a21
MarcoFalke:
review ACK beead33a21
jnewbery:
reACK beead33a21
Tree-SHA512: e748cd52fe23d647fa49008b020389956ac508e16ce9fd108d8afb773bff95788298ae229162bd70215d7246fc25c796484966dc05890b0b4ef601f9cd35628b
9ac86bcc0d test: remove qt byteswap compattests (fanquake)
Pull request description:
These were added as part of #9366 when with fixing issues with Protobuf.
Now that we no-longer use Protobuf, there's no reason to maintain a duplicate set of byteswap tests in the qt tests. Our other set of byteswap tests are here: https://github.com/bitcoin/bitcoin/blob/master/src/test/bswap_tests.cpp.
ACKs for top commit:
laanwj:
Code review ACK 9ac86bcc0d
Tree-SHA512: 72ba131a5f8fbd9fdbbc4e1f95baa794496c960b12e0271700c632c6511b7e1b331e8db07a201838b4d56b2aeeb43d4de4e10265ea07ab14241307fa14d3342e
dc4551c22c remove incompatibility release note for darkmode on macos (Sylvain Goumy)
303cfc6227 allow darkmode on macos build (Sylvain Goumy)
78f75a2d60 Allow icon colorization on mac os to better support dark mode (Uplab)
Pull request description:
Allow icons to be colorized on macOS to support native Dark mode color scheme.
Rendering on macOS Big Sur before PR:
Rendering on macOS Big Sur after PR:
Light mode stay visually unchanged.
<del>Note, that this currently only affect the build from source, as the macos dmg includes an attributes to force light color scheme on macos windows (see https://github.com/bitcoin/bitcoin/pull/14593). </del>
<del>But once all glitches are fixed, we will be able to remove this temporary fix. </del>
Edit: this PR is know including the removal of `NSRequiresAquaSystemAppearance` on Info.plist file so that the color fix is apply to every build.
Linked issues: #68#136
ACKs for top commit:
hebasto:
re-ACK dc4551c22c
jarolrod:
ACK dc4551c22c
Tree-SHA512: 1c3a4dec796063e61fcaf80112afc2b15c8669a1cd30ebd537cea96647c20215f8f80289719f905820bb0c490c8c1f94bfae4bb32f9c6d1fdd4e8f199ebb559f
732c7bddeb tests: Add test for CNetAddr::ToString IPv6 address formatting (RFC 5952) (practicalswift)
Pull request description:
Test that `CNetAddr::ToString` formats IPv6 addresses with zero compression and canonicalisation as described in [RFC 5952 ("A Recommendation for IPv6 Address Text Representation")](https://tools.ietf.org/html/rfc5952).
Solving #21466 will hopefully be trivial with the ability to check zero compression correctness against these tests.
ACKs for top commit:
vasild:
ACK 732c7bddeb
Tree-SHA512: 31a1378aa435ba4171490a2e15d7280a175292270eb001b47d367e010c6ac9b83420b82bbeab22211f8f500c69e21878047c87adf216263b3420b6bb2a5d2bfb
These were added as part of #9366 to fix issues with Protobuf.
Now that we no-longer use Protobuf, there's no reason to maintain a
duplicate set of byteswap tests for qt.
