👍 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
```

🤔 glozow reviewed a pull request: "RPC: Wallet: Add `maxfeerate` and `maxburnamount` startup option"
(https://github.com/bitcoin/bitcoin/pull/29278#pullrequestreview-1832875650)
I mentioned adding a `maxfeerate` option in #29220, but I was more imagining it as a wallet-only option. I don't think that wallet configurations should bleed into how the node RPCs function. Instead, the interaction there should be for wallet to pass it as a param to `BroadcastTransaction` when the it submits a tx to the node.

I'm also not sure about `maxburnamount` being a node-wide config (https://github.com/bitcoin/bitcoin/issues/29217#issuecomment-1900585661)

💬 glozow commented on pull request "RPC: Wallet: Add `maxfeerate` and `maxburnamount` startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1459145558)
Why not just make this a `CFeeRate`? Or describe this as satoshis per KvB. "fee rate (in satoshis)" doesn't really make sense to me.

💬 glozow commented on pull request "RPC: Wallet: Add `maxfeerate` and `maxburnamount` startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1459122123)
I think you accidentally put the `maxburnamount` helptext in the `incrementalrelayfee` helptext here?