🤔 jonatack reviewed a pull request: "fuzz: set `nMaxOutboundLimit` in connman target"
(https://github.com/bitcoin/bitcoin/pull/29172#pullrequestreview-1810118033)
ACK e5b9ee0221ec8aa238fb5720fcd6faa01b09fe46

💬 yancyribbens commented on pull request "test: Add algo assert to bnb_search_test":
(https://github.com/bitcoin/bitcoin/pull/29206#issuecomment-1881922860)
> In the specific test with result13, the 9 is a preselected input and therefore BnB is returning 9+1 instead of 10.

Ok. The test reads that it expects [9, 1] as a result AFAICT and since the fee_rate is set to be high (fee_rate > long_term_feerate) it should be 10. I verified that the BnB algo is actually doing the correct thing, although it's good to know there are some fixes coming for the tests.

🤔 furszy reviewed a pull request: "wallet: simplify and batch zap wallet txes process"
(https://github.com/bitcoin/bitcoin/pull/28987#pullrequestreview-1810138430)
CI failure is unrelated. Relates to #29112.

👍 jamesob approved a pull request: "logging: Simplify API for level based logging"
(https://github.com/bitcoin/bitcoin/pull/28318#pullrequestreview-1810116924)
ACK e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c ([`jamesob/ackr/28318.1.ajtowns.logging_simplify_api_for`](https://github.com/jamesob/bitcoin/tree/ackr/28318.1.ajtowns.logging_simplify_api_for))

This is a definite improvement in the new-style logging interface.

I built and reviewed each commit locally, and ran `./test/functional/feature_segwit.py --nocleanup` and reviewed the resulting debug.log to sanity check.

Eventually it would be nice to unify the interface for each logging function (
...

💬 jamesob commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1445429093)
https://github.com/bitcoin/bitcoin/pull/28318/commits/f7ce5ac08c669ac763e275bb7c82dcfb2b1b6c33

It's somewhat wonky that e.g. `LogInfo()` doesn't take a category, but e.g. `LogDebug` does, and in later commits we actually remove category information from info-equivalent log statements, which seems backwards to me. Ideally, `LogInfo` would _optionally_ take a category. But maybe this can be done later?

💬 jamesob commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1445407709)
Minor thing, but there's still a lingering reference in `contrib/devtools/bitcoin-tidy/example_logprintf.cpp`. Not sure if that needs to be updated or not.

💬 stickies-v commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1445447535)
Gha forgout about that. Macro it is, thanks.

💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1445448364)
Agree, I'll change it soon

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1881954408)
Mea culpa. Fixed the linter issue.

👍 jaonoctus approved a pull request: "RFC: Deprecate libconsensus"
(https://github.com/bitcoin/bitcoin/pull/29189#pullrequestreview-1810195373)
Concept ACK

💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#issuecomment-1881989245)
Rebased + fix correct usage of `utf8string()` instead of `u8string()`.

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1882055933)
Added missing test title in overview of CoinGrinder tests

💬 kevkevinpal commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1445523848)
fixed in latest force push [fba275b](https://github.com/bitcoin/bitcoin/pull/28885/commits/fba275b39c83525e98540c110e09d5177fcdb4a0)

💬 kevkevinpal commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1445557967)
Added a comment ontop showing where I got these numbers from

💬 kevkevinpal commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1445558060)
updated in 74fae193384b722462a9ede2d55b3375bad74332

💬 kevkevinpal commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1445559789)
yes you are right updated in [3fbcbc2](https://github.com/bitcoin/bitcoin/pull/28885/commits/3fbcbc26dd2ea0f3257ca52b3f5a037aad7b9b04)

💬 BombayN1 commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1445562578)
Une fois terminer tout ceux qui m'auront aidé seront récompensé

💬 kevkevinpal commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-1882243654)
ACK [7f0be89](https://github.com/bitcoin/bitcoin/pull/29156/commits/7f0be8969e721de69fc352f76db5787836803b76)

💬 niteshbalusu11 commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1882399978)
Concept ACK.

💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1445616402)
I don't think this `std::ceil` is doing anything. The `/` operator is an integer division here, which rounds down. Applying `std::ceil` to that just converts it to a floating point number. I don't think this is incorrect, as rounding down is just more conservative, but it's probably not what you intend.

To compute $\lceil \frac{a}{b} \rceil$, you can use `(a+b-1)/b`.

Also: I don't think `best_selection.empty()` is possible at this point; a UTXO was just added.