💬 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
...

💬 glozow commented on issue "Enable `maxfeerate` and `maxburnamount` as startup config options.":
(https://github.com/bitcoin/bitcoin/issues/29217#issuecomment-1900585661)
fwiw I'm concept -0 on adding a global `maxburnamount` option. I think it's less footgunny for the default to always be 0 and for users specify explicitly each time they want to burn money. But if people really want this then ok.

🤔 furszy reviewed a pull request: "test: ensure output is large enough to pay for its fees"
(https://github.com/bitcoin/bitcoin/pull/29283#pullrequestreview-1832956095)
Can be replicated locally by changing line 273 to use the `get_rand_amount()` floor.
```diff
variant.initial_amount = AMOUNT_DUST * 2
```