f52a72af569526f319d9f1480eb473b9570125c3 ci: Invalidate depends caches when sources have been changed (Hennadii Stepanov)
939640f87ec385d08f1b04bfd7f61c80038c4565 ci: Reorder scripts to make git available before depends_sources_cache (Hennadii Stepanov)
Pull request description:
On master (502d22ceed1f90ed41336260f8eb428d3acaf514) the Cirrus CI do _not_ invalidate depends caches when their sources has been changed, i.e. source version updates, a new dependency etc.
This behavior causes:
- breaking CI build for Android APK (see #22708)
- the cache sizes growing
It is assumed that caches could be invalidated via Cirrus CI API/GUI, but it is inconvenient and it does not work as expected in all cases.
The common part of `fingerprint_key`s for both `depends_sources_cache` and `depends_built_cache` is the recent commit that changes sources in the `src/depends` sub-directory.
For `depends_built_cache` additionally `CIRRUS_TASK_NAME` is used to avoid sharing of the same cache between tasks with different OSes and different `DEP_OPTS`.
---
The `depends_sources` cache:
- in master (502d22ceed1f90ed41336260f8eb428d3acaf514)
- with this PR
ACKs for top commit:
MarcoFalke:
cr ACK f52a72af569526f319d9f1480eb473b9570125c3
Zero-1729:
crACK f52a72af569526f319d9f1480eb473b9570125c3
Tree-SHA512: 0d14814ff308317c7c18cc725cf69055139030c207200af003872068f5567bd4b9c1f6783845d11742e314ebcc1dc5cae16aa86475376b623d921af5fcda14dd
Clear() is now only called from the ctor, so just inline the code into
that function.
The LOCK(cs) can be removed, since there can be no data races in the ctor.
Also move the function definition out of the header and into the cpp file.
Just use unique_ptr<CAddrMan>s and reset the pointer if a frest addrman is required.
Also make CAddrMan::Clear() private to ensure that no call sites are missed.
1ea11e10acd60807a06adea5ecf553974a1b0346 doc: link to managing-wallets from doc readme (fanquake)
Pull request description:
This was forgotten in #22523.
ACKs for top commit:
achow101:
ACK 1ea11e10acd60807a06adea5ecf553974a1b0346
jarolrod:
ACK 1ea11e10acd60807a06adea5ecf553974a1b0346
Tree-SHA512: b82664b282cc0fe733b752c011621593df0f846d2188f12dbc5fedb7ffed2bd161293ce2a369ca973926030795b5f7acde7a1cbf5e337042a6f665906069c656
62a09a30772141ef4add2f10d29927211abf57eb refactor: remove ::vpwallets and related global variables (Russell Yanofsky)
Pull request description:
Get rid of global wallet list variables by moving them to WalletContext struct
- [`cs_wallets`](e638acf697/src/wallet/wallet.cpp (L56)) is now [`WalletContext::wallet_mutex`](4be544c7ec/src/wallet/context.h (L37))
- [`vpwallets`](e638acf697/src/wallet/wallet.cpp (L57)) is now [`WalletContext::wallets`](4be544c7ec/src/wallet/context.h (L38))
- [`g_load_wallet_fns`](e638acf697/src/wallet/wallet.cpp (L58)) is now [`WalletContext::wallet_load_fns`](4be544c7ec/src/wallet/context.h (L39))
ACKs for top commit:
achow101:
ACK 62a09a30772141ef4add2f10d29927211abf57eb
meshcollider:
re-utACK 62a09a30772141ef4add2f10d29927211abf57eb
Tree-SHA512: 74428180d57b4214c3d96963e6ff43e8778f6f23b6880262d1272f2de67d02714fdc3ebb558f62e48655b221a642c36f80ef37c8f89d362e2d66fd93cbf03b8f
cd37356ff9a1a3c2365c4fe3c716d1ca74185d73 [crypto] Fix K1/K2 use in ChaCha20-Poly1305 AEAD (Dhruv Mehta)
Pull request description:
BIP324 mentions K1 is used for the associated data and K2 is used for the payload. The code does the opposite. This is not a security problem but will be a problem across implementations based on the HKDF key derivations.
BIP324 author Jonas Schnelli thinks a [code update will be better](https://github.com/bitcoin/bitcoin/pull/15649#discussion_r440780669) than a BIP update.
If this PR is merged:
- [ ] We need to update the test vector 3 in BIP324
ACKs for top commit:
jonasschnelli:
utACK cd37356ff9a1a3c2365c4fe3c716d1ca74185d73
Tree-SHA512: e2165117bfbf7a031060e7376912f9af1c1bfc57916383799a0fa2c040e2caaab0d6aafc3425c083a233b96c84fafec75c938e00ceb6bd7d52607d58607cb145
The SHA256SUMS file can be used in a sha256sum -c command to verify
downloaded binaries. However users are likely to download just a single
file and not place this file in the correct directory relative to the
SHA256SUMS file for the simple verification command to work. By not
including the directory name in the SHA256SUMS file, it will be easier
for users to verify downloaded binaries.
Co-authored-by: Carl Dong <contact@carldong.me>
f12fbad5a1852706184082c68655fafad4bb9fe1 windres: use PACKAGE_VERSION rather than building more version numbers (fanquake)
Pull request description:
Rather than defining more strings, reuse PACKAGE_VERSION, which is already available.
We also already use PACKAGE_VERSION for `ProductVersion` and `FileVersion` in setup.nsi.
ACKs for top commit:
MarcoFalke:
cr ACK f12fbad5a1852706184082c68655fafad4bb9fe1
laanwj:
Code review ACK f12fbad5a1852706184082c68655fafad4bb9fe1
Tree-SHA512: b74a37cbba105d208d4da9264d295d7e052009fdd6b0ed54a0d9968bbe2deeba1766d6d310438b2939a81555faa0cbd67d5e53f0c8a2de669ce56353c1c67d22
49ee2a0ad88e0e656234b769d806987784ff1e28 Avoid wallet code writing node settings file (Russell Yanofsky)
Pull request description:
Change wallet loading code to access settings through the Chain interface instead of writing settings.json directly. This is for running wallet and node in separate processes, since multiprocess code wouldn't easily work with different processes updating the same file.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102.
ACKs for top commit:
jamesob:
ACK 49ee2a0ad88e0e656234b769d806987784ff1e28 ([`jamesob/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co`](https://github.com/jamesob/bitcoin/tree/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co))
ryanofsky:
> ACK [49ee2a0](49ee2a0ad8) ([`jamesob/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co`](https://github.com/jamesob/bitcoin/tree/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co))
Zero-1729:
crACK 49ee2a0ad88e0e656234b769d806987784ff1e28
meshcollider:
Code review ACK 49ee2a0ad88e0e656234b769d806987784ff1e28
Tree-SHA512: a81c63b87816f739e02e3992808f314294d6c7213babaafdaaf3c4650ebc97ee4f98f9a4684ce4ff87372df59989b8ad5929159c5686293a7cce04e97e2fabba
68faa87881f5334b2528db4adc72ec19d94316a3 test: use f-strings in mining_*.py tests (fanquake)
c2a5d560df2824df5731100c2584e8ad7a3d7bc2 test: use f-strings in interface_*.py tests (fanquake)
86d958262dff43002820d58ccb8958e2dbfb9d5b test: use f-strings in feature_proxy.py (fanquake)
31bdb33dcb8345df1bb94b28e811252a918d7dcb test: use f-strings in feature_segwit.py (fanquake)
b166d54c3cbb0c028210cee977b3dcde5ac5474f test: use f-strings in feature_versionbits_warning.py (fanquake)
cf6d66bf941d946600047d712c7cd15d7605322e test: use f-strings in feature_settings.py (fanquake)
6651d77f22862716f5bd7d0b31cfbd3937ab7b1d test: use f-strings in feature_pruning.py (fanquake)
961f5813ba65b6a601081912c4ece96c2679794d test: use f-strings in feature_notifications.py (fanquake)
1a546e6f6ca95772f0d7dbc2792477becbb8ea63 test: use f-strings in feature_minchainwork.py (fanquake)
6679eceacc915a8ea7cd7063f103ffc5eb9da884 test: use f-strings in feature_logging.py (fanquake)
fb633933ab570e945d2a366f37eeff39f516c613 test: use f-strings in feature_loadblock.py (fanquake)
e9ca8b254d4b9567831c0e113ce1c0a2b4795a95 test: use f-strings in feature_help.py (fanquake)
ff7e3309995a8960ac371741b2b00c6da40f7490 test: use f-strings in feature_filelock.py (fanquake)
d5a6adc5e478fa5c6e562377eea873dc38e66578 test: use f-strings in feature_fee_estimation.py (fanquake)
a2de33cbdc79202bccddb4beadfde88266ac979f test: use f-strings in feature_dersig.py (fanquake)
a2502cc63fd308be8af840962da9c53339433fa6 test: use f-strings in feature_dbcrash.py (fanquake)
3e2f84e7a96cb4b97b609ac853f78edd0ed43f82 test: use f-strings in feature_csv_activation.py (fanquake)
e2f1fd8ee92fa421b6d293169044d6ddd5a9b8df test: use f-strings in feature_config_args.py (fanquake)
36d33d32b1b498b61f56d552f6e2c1d064f978c3 test: use f-strings in feature_cltv.py (fanquake)
dca173cc044270b30782b1e3355e9dcb8c534295 test: use f-strings in feature_blocksdir.py (fanquake)
5453e8706278918ac51a725e81599cfa18c8cdbc test: use f-strings in feature_backwards_compatibility.py (fanquake)
6f3d5ad67ac8e7b50abae1a2949898d858e38106 test: use f-strings in feature_asmap.py (fanquake)
Pull request description:
Rather than using 3 different ways to build/format strings (sometimes all in the same test, i.e [`feature_config_args.py`](https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_config_args.py)), consolidate to using [f-strings (3.6+)](https://docs.python.org/3/reference/lexical_analysis.html#f-strings), which are generally more concise / readable, as well as more performant than existing methods.
This deals with the `feature_*.py`, `interface_*.py` and `mining_*.py` tests.
See also: [PEP 498](https://www.python.org/dev/peps/pep-0498/)
ACKs for top commit:
mjdietzx:
reACK 68faa87881f5334b2528db4adc72ec19d94316a3
Zero-1729:
crACK 68faa87881f5334b2528db4adc72ec19d94316a3
Tree-SHA512: d4e1a42e07d96d2c552387a46da1534223c4ce408703d7568ad2ef580797dd68d9695b8d19666b567af37f44de6e430e8be5db5d5404ba8fcecf9f5b026a6efb
5449d44e37982fcd5251fd47873c5f7d34c39fc9 scripts: prevent GCC optimising test symbols in test-symbol-check (fanquake)
Pull request description:
I noticed in #22381 that when the test-symbol-check target was being built with Clang and run in the CI it would fail due to using a too-new version of `pow` (used [here](d67330d112/contrib/devtools/test-symbol-check.py (L85))). Our CIs use Focal (glibc 2.31) and the version of `pow` was the optimized version introduced in [glibc 2.29](https://lwn.net/Articles/778286/):
```bash
* Optimized generic exp, exp2, log, log2, pow, sinf, cosf, sincosf and tanf.
```
This made sense, except for that if it was failing when built using Clang, why hadn't it also been failing when being built with GCC?
Turns out GCC is optimizing away that call to `pow` at all optimization levels, including `-O0`, see: https://godbolt.org/z/53MhzMxT7, and this has been the case forever, or at least since GCC 5.x. Clang on the other hand, will only optimize away the `pow` call at `-O1` and `-O2`, not `-O0`: https://godbolt.org/z/Wbnqj3q6c. Thus when this test was built with Clang (we don't pass `-O` so we default to `-O0`) it was failing in the CI environment, because it would actually have a call to the "new" `pow`.
Avoid this issue by using a symbol that won't be optimized away, or that we are unlikely to ever have versioning issues with.
ACKs for top commit:
laanwj:
ACK 5449d44e37982fcd5251fd47873c5f7d34c39fc9
Tree-SHA512: 3a26c5c3a5f2905fd0dd90892470e241ba625c0af3be2629d06d5da3a97534c1d6a55b796bbdd41e2e6a26a8fab7d981b98c45d4238565b0eb7edf3c5da02007
`bool CAddrDB::Read(CAddrMan& addr, CDataStream& ssPeers)` is _only_
called from the tests, and the call to addr.Clear() only exists so that
a test that Clear() is called passes. Remove that test and the call.
Now that we manage the lifespan of node.addrman, we can just reset
node.addrman to a newly initialized CAddrMan if parsing peers.dat
fails, eliminating the possibility that Clear() leaves some old state
behind.
d8ba6327b21c755582ab3c83fc31bc6c523c41c3 scripted-diff: replace clientInterface with m_client_interface in net (fanquake)
f68c6cebe638cc3cf92f4e37650d37ea3899893a net: use clientInterface rather than uiInterface (fanquake)
Pull request description:
First commit fixes two cases where we were using `uiInterface`, rather than `clientInterface`.
Second commit renames `clientInterface` to `m_client_interface`, to match banman.
ACKs for top commit:
hebasto:
ACK d8ba6327b21c755582ab3c83fc31bc6c523c41c3, verified that `uiInterface` is replaced in all sites.
Zero-1729:
crACK d8ba6327b21c755582ab3c83fc31bc6c523c41c3
Tree-SHA512: 9c893d8799b0bc7737836c16e11b77b6f9dffa31041ec6678e63cad958ea06da09a841b99cc61c1b4d9f6f4f1be397ca5a8d2fb2fb7ab08c9437043f8a57c3dc
aaa6ad54554abd1666cb60285ed6f890698ca620 [MOVEONLY] [tests] Move addrman ser/deser tests to addrman_tests.cpp (John Newbery)
Pull request description:
Addrman serialization/deserialization tests are currently in net_tests.cpp.
Move them to addrman_tests.cpp with the rest of the addrman tests.
Reviewer hint: review using `git diff --color-moved=dimmed-zebra`
ACKs for top commit:
MarcoFalke:
review ACK aaa6ad54554abd1666cb60285ed6f890698ca620 📺
mjdietzx:
ACK aaa6ad54554abd1666cb60285ed6f890698ca620
Saviour1001:
Concept ACK <code>[aaa6ad5](aaa6ad5455)</code>
Tree-SHA512: 9a575e63f8830a9dfba30ef63c83ae4b45813add9973308f0d7a2463886438575a2e8a4a23af5b19ed977fbaab1a212ef832f89a25de03920d493e9ebafa9c30
5c5d0b62648e1b144b7b93c199f45265dac100e5 Add FoundBlock.found member (Russell Yanofsky)
Pull request description:
This change lets IPC serialization code handle FoundBlock arguments more simply and efficiently. Without this change there was no way to determine from a FoundBlock object whether a block was found or not. So in order to correctly implement behavior of leaving FoundBlock output variables unmodified when a block was not found, IPC code would have to read preexisting output variable values from the local process, send them to the remote process, receive output values back from the remote process, and save them to output variables unconditionally. With FoundBlock.found method, the process is simpler. There's no need to read or send preexisting local output variable values, just to read final output values from the remote process and set them conditionally if the block was found.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102.
ACKs for top commit:
fjahr:
Code review ACK 5c5d0b62648e1b144b7b93c199f45265dac100e5
theStack:
Concept and code review ACK 5c5d0b62648e1b144b7b93c199f45265dac100e5
jamesob:
ACK 5c5d0b62648e1b144b7b93c199f45265dac100e5 ([`jamesob/ackr/22215.1.ryanofsky.refactor_add_foundblock`](https://github.com/jamesob/bitcoin/tree/ackr/22215.1.ryanofsky.refactor_add_foundblock))
Zero-1729:
crACK 5c5d0b6
Tree-SHA512: d906e1b7100ff72c3aa06d80bd77673887b2db670ebd52dce7c4f6f557a23a1744c6109308228a37fda6c6ea74f05ba0efecff0ef235ab06ea8acd861fbb8675
Addrman serialization/deserialization tests are currently in net_tests.cpp.
Move them to addrman_tests.cpp with the rest of the addrman tests.
Reviewer hint: review using `git diff --color-moved=dimmed-zebra`
Rather than defining more strings, reuse PACKAGE_VERSION, which is
already available.
We already use PACKAGE_VERSION for `ProductVersion` and `FileVersion` in setup.nsi.
