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

BITCOIN CORE QUEEN #1

Merged
merged 142 commits into from
Dec 13, 2024
Merged

BITCOIN CORE QUEEN #1

merged 142 commits into from
Dec 13, 2024

Conversation

Randy808 and others added 30 commits April 27, 2024 23:50
`IF_CHECK_PASSED` is a multi-value keyword, resulting in a list value.
Convert it to a string before applying any `string()` command.
- Some test methods in the functional test framework are independent
  and do not require any previous context or setup defined in `run_test`.
- This commit adds a new option for running these specific methods within a test file,
  allowing them to be executed individually without running the entire test suite.

- running test methods that require an argument or context will fail.
The previous error message for non-existent wallets of "Already a
descriptor wallet" is misleading. Return a more specific error when a
non-existent wallet is specified.
This passes the return value of _() directly to strprintf so the format
string can be checked at compile time in a future commit.
This is required for a future commit. Can be reviewed via the git
options --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

Also move util::detail::Hex to a proper namespace instead of an inline
namespace so it doesn't conflict with the new util::detail namespace, and
won't create other problems for callers trying to use the inline namespaces.

Also fix a misleading comment in util_string_tests.cpp.

Co-Authored-By: Ryan Ofsky <[email protected]>
Requested by clang-tidy:

src/wallet/salvage.cpp:119:18: error: use emplace_back instead of push_back [modernize-use-emplace,-warnings-as-errors]
   119 |         warnings.push_back(Untranslated("Salvage: Database salvage found errors, all data may not be recoverable."));
       |                  ^~~~~~~~~~
       |                  emplace_back(
This ensures the options are applied consistently from contexts where
they might not pass through the args manager, such as in some tests, or
when used through the kernel library.

This is similar to the patch applied in 09ef322.
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended 's/\b(gArgs|args)\.IsArgSet\("-version"\)/\1.GetBoolArg("-version", false)/g' $(git grep -l '-version')
-END VERIFY SCRIPT-
Does not make sense to run without a datadir.

Prior to this change it would be interpreted as a mix of unset and as a relative path of "0".
The coinbase check is repeated again early during ProcessNewBlock.
Pre-checking it may also shadow more fundamental problems with a block.
In most cases the block header is checked first, before validating the
transactions. Checking the coinbase first therefore masks potential
issues with the header. Fix this by removing the pre-check.

The pre-check was likely introduced on top of
ada0caa to fix UB in
GetWitnessCommitmentIndex in case a block's transactions are empty. This
code path could only be reached because of the call to
UpdateUncommittedBlockStructures in submitblock, but cannot be reached
through net_processing.

Add some functional test cases to cover the previous conditions that
lead to a "Block does not start with a coinbase" json rpc error being
returned.

---

With the introduction of a mining ipc interface and the potential future
introduction of a kernel library API it becomes increasingly important
to offer common behaviour between them. An example of this is
ProcessNewBlock, which is used by ipc, rpc, net_processing and
(potentially) the kernel library. Having divergent behaviour on
suggested pre-checks and checks for these functions is confusing to both
developers and users and is a maintenance burden.

The rpc interface for ProcessNewBlock (submitblock) currently pre-checks
if the block has a coinbase transaction and whether it has been
processed before. While the current example binary for how to use the
kernel library, bitcoin-chainstate, imitates these checks, the other
interfaces do not.
ProcessNewBlock fails if an invalid duplicate block is passed in through
its call to AcceptBlock and AcceptBlockHeader. The failure in
AcceptBlockHeader makes AcceptBlock return early. This makes the
pre-check in submitblock redundant.

---

With the introduction of a mining ipc interface and the potential future
introduction of a kernel library API it becomes increasingly important
to offer common behaviour between them. An example of this is
ProcessNewBlock, which is used by ipc, rpc, net_processing and
(potentially) the kernel library. Having divergent behaviour on
suggested pre-checks and checks for these functions is confusing to both
developers and users and is a maintenance burden.

The rpc interface for ProcessNewBlock (submitblock) currently pre-checks
if the block has a coinbase transaction and whether it has been
processed before. While the current example binary for how to use the
kernel library, bitcoin-chainstate, imitates these checks, the other
interfaces do not.
The duplicate checks are repeated early in the contextual checks of
ProcessNewBlock. If duplicate blocks are detected much of their
validation is skipped. Depending on the constitution of the block,
validating the merkle root of the block is part of the more intensive
workload when validating a block. This could be an argument for moving
the pre-checks into block processing. In net_processing this would have
a smaller effect however, since the block mutation check, which also
validates the merkle root, is done before.

A side effect of this change is that a duplicate block is persisted
again on disk even when pruning is activated. This is similar to the
behaviour with getblockfrompeer. Add a release note for this change in
behaviour.

Testing spamming a node with valid, but duplicate unrequested blocks
seems to exhaust a CPU thread, but does not seem to significantly impact
keeping up with the tip. The benefits of adding these checks to
net_processing are questionable, especially since there are other ways
to trigger the more CPU-intensive checks without submitting a duplicate
block. Since these DOS concerns apply even less to the RPC interface,
which does not have banning mechanics built in, remove them too.

---

With the introduction of a mining ipc interface and the potential future
introduction of a kernel library API it becomes increasingly important
to offer common behaviour between them. An example of this is
ProcessNewBlock, which is used by ipc, rpc, net_processing and
(potentially) the kernel library. Having divergent behaviour on
suggested pre-checks and checks for these functions is confusing to both
developers and users and is a maintenance burden.

The rpc interface for ProcessNewBlock (submitblock) currently pre-checks
if the block has a coinbase transaction and whether it has been
processed before. While the current example binary for how to use the
kernel library, bitcoin-chainstate, imitates these checks, the other
interfaces do not.
This tests the new submitblock behaviour that is introduced in the
previous commit: Submitting a previously pruned block should persist the
block's data again.
The behaviour of submitblock was changed in the previous commit, so
change it here too.
Can be tested by running

```
$ sudo tcpdump -i eth0 host 11.22.33.44
```

and verifying that no packets appear in the tcpdump output.

Co-authored-by: Vasil Dimov <[email protected]>
This change resolves build issues that occur when the source or build
directory is symlinked.
And under the hood suppoert single transactions
in AcceptPackage. This simplifies user experience
and paves the way for reducing number of codepaths
for transaction acceptance in the future.

Co-Authored-By: instagibbs <[email protected]>
The "SDK Extraction" section is what users presumably are looking for
when they follow these links.
The "Codesigning" section is what users presumably are looking for when
they follow this link.
The `src/config` directory has not been used since the migration to
CMake, which disables in-source builds.
These are required when cross-compiling shared libraries such as the
kernel library.
hodlinator and others added 26 commits December 6, 2024 21:56
Proves tinyformat doesn't trigger an exception for \0 characters.

Co-Authored-By: Lőrinc <[email protected]>
31e59d9 iwyu: Drop backported mapping (Hennadii Stepanov)
fe9bc5a ci: Update Clang in "tidy" job (Hennadii Stepanov)

Pull request description:

  This PR switches to the latest [IWYU 0.23](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.23), which is compatible with Clang 19.

  New "bugprone-use-after-move" and "modernize-use-starts-ends-with" warnings that emerged have been addressed.

ACKs for top commit:
  maflcko:
    lgtm ACK 31e59d9
  l0rinc:
    ACK 31e59d9
  theuni:
    ACK 31e59d9

Tree-SHA512: ae0ca150673e1bfa78664f2ef35dbc965094b32374cafeeae390c6d368c28169a7f7790debe9a6eeb5efc39c9a468f5032d92f30cc4032b09d8265f6a75de882
41d934c chore: Typo Overriden -> Overridden (Hodlinator)
c9fb38a refactor test: Cleaner combine_logs.py logic (Hodlinator)

Pull request description:

  - Fixes typo caught by spelling linter (https://github.com/bitcoin/bitcoin/runs/33979284676).
  - Minor but nice refactoring of *combine_logs.py* change that was suggested late: #31212 (comment)

ACKs for top commit:
  l0rinc:
    ACK 41d934c
  maflcko:
    lgtm ACK 41d934c
  theStack:
    ACK 41d934c
  BrandonOdiwuor:
    Code Review ACK 41d934c
  tdb3:
    ACK 41d934c

Tree-SHA512: cf8ecc070d0b01df9c4e57a75820e17d4535591e85bf9d271c7b8f60875f7e04b9978c56e9b88c10e89e69ff755c35b23ed291949c32c875a91c3317105a3c79
In a `x86_64-linux-gnu` build, this drops:
```bash
x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov
x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov-dump
x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov-tool
x86_64-linux-gnu/lib/gcc/x86_64-linux-gnu/12.4.0: libgcov.a
```

For mingw-w64-gcc, `--disable-gcov` is currently passed for this
target in Guix, due to issues with mingw-w64, see
https://github.com/fanquake/guix/blob/8bed031e58c9cf895d243790e7c80808b0075de7/gnu/packages/gcc.scm#L99-L102.
However we'll add it in any case, in case it's re-enabled in future,
when the underlying issues are fixed.
Same as llvm/llvm-project#113951.

Avoids compile failures under clang-20 &
`D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES`:
```bash
In file included from /bitcoin/src/test/fuzz/addition_overflow.cpp:5:
/bitcoin/src/test/fuzz/FuzzedDataProvider.h:209:5: error: use of undeclared identifier 'abort'
  209 |     abort();
      |     ^
/bitcoin/src/test/fuzz/FuzzedDataProvider.h:250:5: error: use of undeclared identifier 'abort'
  250 |     abort();
```
cdd207c test: add coverage for migrating standalone imported keys (furszy)
297a876 test: add coverage for migrating watch-only script (furszy)
932cd1e wallet: fix crash during watch-only wallet migration (furszy)

Pull request description:

  The crash occurs because we assume the cached scripts structure will not be empty,
  but it can be empty for watch-only wallets that start blank.

  This also adds test coverage for standalone imported keys, which were also crashing
  because pubkey imports are treated the same way as hex script imports through
  `importaddress()`.

  Testing Notes:
  This can be verified by cherry-picking and running any of the test commits on master.
  It will crash there but pass on this branch.

ACKs for top commit:
  theStack:
    re-ACK cdd207c
  brunoerg:
    reACK cdd207c
  achow101:
    ACK cdd207c

Tree-SHA512: e05c77cf3e9f35f10f122a73680b3f131f683c56685c1e26b5ffc857f95195b64c8c9d4535960ed3d6f931935aa79b0b1242537462006126bdb68251f0452954
Non-HD keys in legacy wallets without a HD seed ID were being migrated
to separate pk(), pkh(), sh(wpkh()), and wpkh() descriptors for each key.
These could be more compactly represented as combo() descriptors, so
migration should make combo() for them.

It is possible that existing non-HD wallets that were migrated, or
wallets that started blank and had private keys imported into them have
run into this issue. However, as the 4 descriptors produce the same output
scripts as the single combo(), so any previously migrated wallets are
not missing any output scripts. The only observable difference should be
performance related, and the wallet size on disk.
bb7e686 fuzz: add cstdlib to FuzzedDataProvider (fanquake)

Pull request description:

  Same as llvm/llvm-project#113951.

  Avoids compile failures under clang-20 & `D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES`:
  ```bash
  In file included from /bitcoin/src/test/fuzz/addition_overflow.cpp:5:
  /bitcoin/src/test/fuzz/FuzzedDataProvider.h:209:5: error: use of undeclared identifier 'abort'
    209 |     abort();
        |     ^
  /bitcoin/src/test/fuzz/FuzzedDataProvider.h:250:5: error: use of undeclared identifier 'abort'
    250 |     abort();
  ```

ACKs for top commit:
  dergoegge:
    utACK bb7e686
  brunoerg:
    ACK bb7e686

Tree-SHA512: 22efd5505273ec7254e8dccbb275e648fe02107397c45eff6752e4a6ea787d9d2e45eb0f2ee309df431e9b92ffd14cbcba4b0f4b11a127664466e20be43c383e
To match the other HOST_debug_flags.
…ibility.py

ec77791 test: Fix intermittent issue in wallet_backwards_compatibility.py (Randall Naar)

Pull request description:

  When creating and replacing a transaction using `bumpfee`, an async update is sent in the form of the `TransactionAddedToMempool` and `TransactionRemovedFromMempool` signals. When `wallet_backwards_compatibility.py` creates `tx3_id` this way and replaces it with `tx4_id`, the `abandontransaction` rpc is called right after. In some cases the `TransactionAddedToMempool` and `TransactionRemovedFromMempool` is handled after the transaction is abandoned in the wallet, and overwrites the transaction's `abandoned` flag. This PR forces the signals to get handled before `abandontransaction` is called by invoking `self.sync_mempools` which calls `syncwithvalidationinterfacequeue` on every node's rpc connection.

  This will mitigate the immediate inconsistency observed with the abandontransaction call, but the potential race conditions between the signals and wallet operations may also be useful to note in a separate issue (if it's okay to not address it in this one).

  Fixes #29806

ACKs for top commit:
  achow101:
    ACK ec77791
  tdb3:
    ACK ec77791

Tree-SHA512: e75bc2c1f7fefc4f4910bb353654848fed5661c1436416798a5f4e0c5a76bde15617a5af04c2384464005953326317b8f273039e47508d5124677908cf36d31e
988721d test: avoid internet traffic in rpc_net.py (Sebastian Falbesoner)

Pull request description:

  In order to avoid connecting to the internet in the functional test `rpc_net.py`, specify a non-working proxy (parameter `-proxy=127.0.0.1:1`, same approach as in #31142) for the nodes.  There is at least one known instance where this is currently happening on master where a connection attempt to a public IP is made (see also the discussion in #31339):

  https://github.com/bitcoin/bitcoin/blob/17834bd1976df7a2ff6c2f5f05a59ae3fd3f6875/test/functional/rpc_net.py#L253

  Can be tested by running
  ```
  $ sudo tcpdump -i eth0 host 11.22.33.44
  ```
  both on master and the PR branch and verifying that no packets appear in the tcpdump in the latter anymore.

ACKs for top commit:
  achow101:
    ACK 988721d
  tdb3:
    ACK 988721d
  vasild:
    ACK 988721d

Tree-SHA512: 0f51fedccbfac0f80a7e6f9c5ba9193d0c20b5a788553c7cd7e583225df7b1151b86cd848d6ccf61f7b2de848f0ac98d73d7b5db100aa54fe8cbeeb4c0549106
…arity

c93bf0e test: Add missing %c character test (Hodlinator)
76cca4a test: Document non-parity between tinyformat and ConstevalFormatstring (Hodlinator)
533013c test: Prove+document ConstevalFormatString/tinyformat parity (Hodlinator)
b81a465 refactor test: Profit from using namespace + using detail function (Hodlinator)

Pull request description:

  Clarifies and puts the extent of parity under test.

  Broken out from #30546 based on #30546 (comment) and #30546 (comment).

ACKs for top commit:
  maflcko:
    re-ACK c93bf0e 🗜
  l0rinc:
    ACK c93bf0e
  ryanofsky:
    Code review ACK c93bf0e. Just a few cleanups tweaking function declarations and commit comments and consolidating some test cases since last review.

Tree-SHA512: 5ecc893b26cf2761c0009861be392ec4c4fceb0ef95052a2f6f9df76b2e459cfb3f9e257f61be07c3bb2ecc6e525e72c5ca853be1f63b70b52785323d3db6b42
f6496a8 guix: disable gcov in base-linux-gcc (fanquake)

Pull request description:

  In a `x86_64-linux-gnu` build, this drops:
  ```bash
  x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov
  x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov-dump
  x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov-tool
  x86_64-linux-gnu/lib/gcc/x86_64-linux-gnu/12.4.0: libgcov.a
  ```

  For mingw-w64-gcc, `--disable-gcov` is currently passed for this target in Guix, due to issues with mingw-w64, see
  https://github.com/fanquake/guix/blob/8bed031e58c9cf895d243790e7c80808b0075de7/gnu/packages/gcc.scm#L99-L102. However we'll add it in any case, in case it's re-enabled in future, when the underlying issues are fixed.

ACKs for top commit:
  TheCharlatan:
    ACK f6496a8

Tree-SHA512: ad6de53f63e7bb658cac05fb023fb1f8e76103073c7dffb4267412d3046148e1389df8848010128c1bd3d428f05e1587b656ef2cad8c7d9078ebec83a68bad49
Necessary for users that have signature output enabled by default, since the script would stumble on them and error out.
015aad8 docs: remove repetitive words (RiceChuan)

Pull request description:

  remove repetitive words

ACKs for top commit:
  maflcko:
    lgtm ACK 015aad8
  hebasto:
    ACK 015aad8.

Tree-SHA512: c241d19786c1fb9836f7c5b3bf4f16df737d3bee75556cecea354b66a7ab653f524ec020676b04a001daeb4293c2cd3e7a0b9b7f54df44d88290bf4409f7b304
b7ec69c depends: add -g to *BSD_debug flags (fanquake)

Pull request description:

  To match the other HOST_debug_flags. Pulled out of #29796.

ACKs for top commit:
  theuni:
    utACK b7ec69c

Tree-SHA512: 654a6dc2c1e295021380f18565379ccde5c5bebcbb5e48ab0364aa79c6f15d301b4acf058d75629a4b217483c6788a0ecb60560e8701882e09490b92c4c346d0
e2d3372 lint: Disable signature output in git log (Hodlinator)

Pull request description:

  Necessary for users that have signature output enabled by default, since the script would stumble on them and error out.

  ---

  ### Testing setup

  Set local repo config to show signatures in log by default, simulating a user having that setting turned on globally.
  ```
  ₿ git config set log.showSignature true
  ```
  ### Command under test

  ```
  ₿ ( cd ./test/lint/test_runner/ && COMMIT_RANGE='HEAD^..HEAD' cargo run )
  ```
  #### Before
  ```
  ...
  fatal: invalid object name 'gpg'.
  Traceback (most recent call last):
    File "/home/hodlinator/bitcoin/test/lint/lint-git-commit-check.py", line 52, in <module>
      main()
    File "/home/hodlinator/bitcoin/test/lint/lint-git-commit-check.py", line 42, in main
      commit_info = check_output(["git", "log", "--format=%B", "-n", "1", hash], text=True, encoding="utf8").splitlines()
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/nix/store/wfbjq35kxs6x83c3ncpfxdyl5gbhdx4h-python3-3.12.6/lib/python3.12/subprocess.py", line 466, in check_output
      return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/nix/store/wfbjq35kxs6x83c3ncpfxdyl5gbhdx4h-python3-3.12.6/lib/python3.12/subprocess.py", line 571, in run
      raise CalledProcessError(retcode, process.args,
  subprocess.CalledProcessError: Command '['git', 'log', '--format=%B', '-n', '1', 'gpg: Signature made ons 11 dec 2024 10:46:34 CET']' returned non-zero exit status 128.
  ^---- ⚠️ Failure generated from lint-git-commit-check.py
  ...
  ```
  #### After

  (No failure generated by *lint-git-commit-check.py*).

ACKs for top commit:
  maflcko:
    lgtm ACK e2d3372
  willcl-ark:
    ACK e2d3372

Tree-SHA512: 584ccece1e6e0f4691683a2b1816eff33b88f48e9ead9272e2dc73ea9c637b182632108fbeddea1ffc8ed6ba5a5838d7eac7a9f33dfda5bdf325dd7a41e43365
This fixes compilation on FreeBSD.
See:
capnproto/capnproto@1c19c36.
…dy warning

df27ee9 refactor: Fix "modernize-use-starts-ends-with" clang-tidy warning (Hennadii Stepanov)

Pull request description:

  This PR is a follow-up to #31306 and fixes the "modernize-use-starts-ends-with" warning in the multiprocess code (see #30975 (comment)).

  Fixes chaincodelabs/libmultiprocess#124.

ACKs for top commit:
  l0rinc:
    ACK df27ee9
  theuni:
    utACK df27ee9
  ryanofsky:
    Code review ACK df27ee9

Tree-SHA512: b5685818e9a3f161949b79586138e4341c5fbcc77296d5f4b13ff0183b6efaac1baea8a6d9304566f25517018d4f67b6d407105a36971a03f4077697d53f17ff
62b2d23 wallet: Migrate non-HD keys to combo() descriptor (Ava Chow)

Pull request description:

  Non-HD keys do not have an HD seed ID associated with them, so if this value is the null value (all 0s), then we should not perform any seed ID comparison that would result in excluding the keys from combo() migration.

  This changes the migration of non-HD wallets (or blank wallets with imported private keys) to make a single combo() descriptors for the non-HD/imported keys, rather than pk(), pkh(), sh(wpkh()), and wpkh() descriptors for the keys.

  Implements #31374 (comment)

ACKs for top commit:
  laanwj:
    Concept and code review ACK 62b2d23
  brunoerg:
    code review ACK 62b2d23
  furszy:
    Nice catch. ACK 62b2d23
  theStack:
    ACK 62b2d23
  rkrux:
    tACK 62b2d23

Tree-SHA512: 86a80b7dcc1598ab18068a2572ff4b4920b233178b760f7b76c5b21a9e6608005ac872f90e082a8f99b51daab0b049e73e4bee5b8e0b537d56ed0d34122a1f49
5cd9e95 depends: update capnproto to 1.0.2 (fanquake)

Pull request description:

  This fixes compilation on FreeBSD:
  ```bash
  -- Build files have been written to: /tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4
  Building native_capnp...
  gmake[1]: Entering directory '/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4'
  [ 1%] Building CXX object src/kj/CMakeFiles/kj.dir/array.c++.o
  [ 2%] Building CXX object src/kj/CMakeFiles/kj.dir/cidr.c++.o
  /tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:112:71: error: member access into incomplete type 'const struct sockaddr_in6'
  otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
  ^
  /tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:112:51: note: forward declaration of 'sockaddr_in6'
  otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
  ^
  /tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:123:63: error: member access into incomplete type 'const struct sockaddr_in'
  &reinterpret_cast<const struct sockaddr_in*>(addr)->sin_addr.s_addr);
  ^
  /tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:123:44: note: forward declaration of 'sockaddr_in'
  &reinterpret_cast<const struct sockaddr_in*>(addr)->sin_addr.s_addr);
  ^
  /tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:133:69: error: member access into incomplete type 'const struct sockaddr_in6'
  otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
  ^
  /tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:133:49: note: forward declaration of 'sockaddr_in6'
  otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
  ^
  3 errors generated.
  ```

  See: capnproto/capnproto@1c19c36.

ACKs for top commit:
  kevkevinpal:
    Concept ACK [5cd9e95](5cd9e95)
  theuni:
    utACK 5cd9e95
  ryanofsky:
    Code review ACK 5cd9e95. Downloaded the file and checked the hash. Also followed theuni's lead and looked at the source changes which were very minor. It did look like thousands of lines changed in the autotools build, but this should not affect us as we are using the cmake build.

Tree-SHA512: 5d78887a9e950c8532c427b17969128de0c6d466ec5ffba85241457e8e19673c22ddb3493cdfce5086f57ba760eac5e91f703992b2f70f2a7c82ba885255279c
fa47baa ci: Bump centos gcc (MarcoFalke)

Pull request description:

  Currently the centos stream9 CI task is using gcc-11. This is fine, because this is also the minimum supported.

  However:

  * There is already a CI task that is checking the minimum supported version: https://github.com/bitcoin/bitcoin/blob/62bd61de110b057cbfd6e31e4d0b727d93119c72/ci/test/00_setup_env_native_previous_releases.sh#L11-L12
  * The CI log is a bit useless, because it is mostly just `#warning _FORTIFY_SOURCE > 2 is treated like 2 on this platform [-Werror=cpp]`. This makes it harder to spot real warnings, such as #31476

  Fix both issues by using gcc-12.

ACKs for top commit:
  hebasto:
    ACK fa47baa.

Tree-SHA512: 573618efc949437d33365a24f77a26a9b68457f7fb9bd603ee92bc5f17fec73ccba114cafb900eddee3531af47508ce5c246def93268787cdfa2b99e6f45a13d
37946c0 Set notifications m_tip_block in LoadChainTip() (Sjors Provoost)

Pull request description:

  Ensure KernelNotifications `m_tip_block` is set even if no new block arrives.

  Suggested in #31297 (comment)

ACKs for top commit:
  ryanofsky:
    Code review ACK 37946c0, fixing comment bug caught by @mzumsande in #31346 (comment) in another really helpful clarification
  mzumsande:
    Code Review ACK 37946c0
  TheCharlatan:
    ACK 37946c0

Tree-SHA512: 931bf820440a0cdda276f6dbd63f03fdbcdc90b18e7d5e160a74bdd9d0290acc706c35aab15bbdcd6e5e0b77565b3d07ff49b0dcf6551cb83961bae67be5d1bb
@joanna2028 joanna2028 merged commit 1e6c143 into joanna2028:master Dec 13, 2024
@joanna2028 joanna2028 changed the title BITCOIN CORE NEEDS NEW TECH LEADER BITCOIN CORE QUEEN Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.