📝 hebasto opened a pull request: "depends: Ensure definitions are passed when building SQLite with `DEBUG=1`"
(https://github.com/bitcoin/bitcoin/pull/29282)
The SQLite build system overrides the `CFLAGS` when is configured with the `--enable-debug` option.

Amends https://github.com/bitcoin/bitcoin/pull/25987.

Otherwise, for example in OSS-Fuzz, the testing binaries diverge from release ones.

💬 ryanofsky commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1458922669)
re: https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1458838935

> The signals could be optional in the chainman too.

Yes, absolutely. The question is in cases where libbitcoinkernel applications don't care about receiving signals (if maybe just they are just validating a block), should they be forced to declare `ValidationSignals validation_signals{std::make_unique<util::ImmediateTaskRunner>()};` and pass a `validation_signals` parameter to the ChainstateManager object that won't
...

💬 sipa commented on issue "Post startup stalling":
(https://github.com/bitcoin/bitcoin/issues/29281#issuecomment-1900333798)
Discussed this briefly with @mzumsande.

The stalling detection logic is ineffective here, because that only triggers when the 1024-block wide download window cannot move, and as you're less than 1024 blocks behind, the window is already as far as it can go.

We should probably have a separate criterion for stalling detection in this case.

💬 TheCharlatan commented on pull request "depends: add NM output to gen_id":
(https://github.com/bitcoin/bitcoin/pull/29249#issuecomment-1900352612)
Yeah, also built on the wrong commit, so:
Re-ACK 6ec2813cd88d5f0b955d746e4711a8ad256ea47f

Guix build (x86):
```
67d1d5dd27bc7b2df6b5d77265a9be0c5e3d7cf0bd003b8ec177010c18526814 guix-build-6ec2813cd88d/output/aarch64-linux-gnu/SHA256SUMS.part
59d2f605151251ee05e9c8b043ead799254c0b61da513df8ba4b9adb8c3dd619 guix-build-6ec2813cd88d/output/aarch64-linux-gnu/bitcoin-6ec2813cd88d-aarch64-linux-gnu-debug.tar.gz
d299e058786d3db9e145ea692b4e90b9ce8a60ec16d101e475e8b2927b53f5ac guix-build-6ec28
...

💬 1440000bytes commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1900364519)
> This isn't Twitter, if you want to participate here make an effort to read everything through and through.

Exactly, this isn't twitter so a detailed explanation with the ACK helps everyone and adds more weight to your comment.

👍 TheCharlatan approved a pull request: "refactor: remove CTxMemPool::queryHashes()"
(https://github.com/bitcoin/bitcoin/pull/29260#pullrequestreview-1832517919)
ACK a30af58d26ec98350ae273ad10793a12ad71cff6

💬 moonsettler commented on pull request "Implement OP_CHECKTEMPLATEVERIFY":
(https://github.com/bitcoin/bitcoin/pull/29280#issuecomment-1900407815)
Concept ACK!

💬 moonsettler commented on pull request "Implement OP_CHECKSIGFROMSTACK(VERIFY)":
(https://github.com/bitcoin/bitcoin/pull/29270#issuecomment-1900408113)
Concept ACK!

💬 moonsettler commented on pull request "Add OP_INTERNALKEY for Tapscript":
(https://github.com/bitcoin/bitcoin/pull/29269#issuecomment-1900408304)
Concept ACK!

🚀 fanquake merged a pull request: "depends: add NM output to gen_id"
(https://github.com/bitcoin/bitcoin/pull/29249)

🤔 furszy reviewed a pull request: "test: p2p: check disconnect due to lack of desirable service flags"
(https://github.com/bitcoin/bitcoin/pull/29279#pullrequestreview-1832560555)
See #28170. Obvious concept ACK.
We do have a post-IBD bug on this process.

💬 vasild commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1900458347)
> HasAllDesirableServiceFlags(), relies on IsInitialBlockDownload() / DEFAULT_MAX_TIP_AGE (24h) on master
...
> I think this means that the safety buffer of 144 blocks proposed in [BIP159](https://github.com/bitcoin/bips/blob/master/bip-0159.mediawiki) would have been removed.

I do not think `-maxtipage` / `DEFAULT_MAX_TIP_AGE` implements "the safety buffer" from BIP159. The former was introduced about 2 years before BIP159, in 2015: https://github.com/bitcoin/bitcoin/pull/5987.

BIP159 r
...

💬 theuni commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1900470464)
Here's a godbolt test that shows how various compilers perform: https://gcc.godbolt.org/z/nTadqEP83

To test with/without builtins, comment/un-comment the `DISABLE_BUILTIN_BSWAPS` define at line 14.

From my tests:
- clang: all c++20 supporting versions (>= 10.0) optimize without the need for builtins
- gcc: no version optimizes without the builtins
- msvc: versions >= 19.37 optimize without builtins

💬 jamesob commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1900505846)
Oddly enough, even with `gcc 12.2.0`, the regression on the first machine holds. Artifacts here: https://gist.github.com/jamesob/1f4456f1f9bafcabb392210bbfe9f240

Will run the secp benches in isolation.

📝 stickies-v opened a pull request: "test: ensure output is large enough to pay for its fees"
(https://github.com/bitcoin/bitcoin/pull/29283)
Fixes a (rare) intermittency issue in wallet_import_rescan.py

Since we [use](https://github.com/bitcoin/bitcoin/blob/03752444cd54df05a731557968d5a9f33a55c55c/test/functional/wallet_import_rescan.py#L296) `subtract_fee_from_outputs=[0]` in the `send` command, the output amount must at least be as large as the fee we're paying.

Example in CI: https://api.cirrus-ci.com/v1/task/6107972259020800/logs/ci.log

```
2024-01-18T22:16:12.383000Z TestFramework (INFO): Test that the mempool is resca
...

💬 stickies-v commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#issuecomment-1900516149)
Reviewers might be interested in https://github.com/bitcoin/bitcoin/pull/29283, fixing a rare intermittency issue

💬 josibake commented on pull request "Silent Payments: sending":
(https://github.com/bitcoin/bitcoin/pull/28201#issuecomment-1900521814)
Rebased https://github.com/bitcoin/bitcoin/commit/3f4b0f77c7f2ff52b9c84bf1c5a70281a066cfe2 -> https://github.com/bitcoin/bitcoin/commit/3b0dd79cacaf4e97489746ace5c1c8d6fe8db38d ([implement-bip352-sending-01](https://github.com/josibake/bitcoin/tree/implement-bip352-sending-01) -> [implement-bip352-01-rebase](https://github.com/josibake/bitcoin/tree/implement-bip352-sending-01-rebase), [compare](https://github.com/josibake/bitcoin/compare/implement-bip352-sending-01..implement-bip352-sending-01-r
...

🤔 glozow reviewed a pull request: "test: ensure output is large enough to pay for its fees"
(https://github.com/bitcoin/bitcoin/pull/29283#pullrequestreview-1832846810)
utACK 3bfc5bd36e38ca07306a41a3f56ea102ffe02b67, didn't experience this issue but in theory a minimum of `AMOUNT_DUST` could be too low to pay the fees

💬 glozow commented on pull request "test: ensure output is large enough to pay for its fees":
(https://github.com/bitcoin/bitcoin/pull/29283#discussion_r1459106218)
Adding `AMOUNT_DUST` isn't really necessary, it's just a floor

💬 jamesob commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1900564313)
> If that doesn't help, please benchmark libsecp256k1 with ([bitcoin-core/secp256k1@10e6d29](https://github.com/bitcoin-core/secp256k1/commit/10e6d29b60c3931e327bc18e6c50cea78296b1ba)) and without [bitcoin-core/secp256k1#1446](https://github.com/bitcoin-core/secp256k1/pull/1446) ([bitcoin-core/secp256k1@07687e8](https://github.com/bitcoin-core/secp256k1/commit/07687e811d1c9700e6fe9d658aef080e3568c0f1)), see the PR for instructions and also consider setting `SECP256K1_BENCH_ITERS=200000` or anoth
...