📝 0xBEEFCAF3 opened a pull request: "Reenable OP_CAT "
(https://github.com/bitcoin/bitcoin/pull/29247)
Renabling OP_CAT, as per the [draft BIP](https://github.com/bitcoin/bips/pull/1525)

This MR involves reinstating the legacy opcode by replacing OP_SUCCESS126. Currently, there are no proposed activation parameters.

### Relevant Links
[Btcd implementation](https://github.com/btcsuite/btcd/pull/2095)
[Signet MR](https://github.com/bitcoin-inquisition/bitcoin/pull/39)
[Mailing list post](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-October/022049.html)

👍 andrewtoth approved a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1820309688)
ACK 6c603490023317a84c7c96ef4b64f4f96ea03d1f modulo nits

For `peerman_tests.cpp`, we should prefer list initialization and constness for variables when possible.

For commit message of `net: move state dependent peer services flags` (b934bfd327f68d39aebb2d9ab52b39b7cd29525d),
the last sentence could be:
```
Additionally, this encapsulation enables us
to customize the connections decision-making process based on
new users' configurations in the future.
```

💬 andrewtoth commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1451813298)
nit: `int64_t best_block_time{};`

💬 andrewtoth commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1451813562)
nit: prefer list initialization, could also make this `constexpr` since we're modifying it.
`constexpr ServiceFlags requiredServiceBits{SeedsServiceFlags()};`

💬 andrewtoth commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1451815522)
nit: Could be one line
```
// Limited peers are desirable when we are close to the tip.
if ((services & NODE_NETWORK_LIMITED) && ApproximateBestBlockDepth() < NODE_NETWORK_LIMITED_MIN_BLOCKS) {
```

💬 andrewtoth commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1451815870)
nit: `2024`

💬 andrewtoth commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1451818095)
nit
```
// By now, we tested that 'CheckForStaleTipAndEvictPeers' can update the connections desirable services flags.
```

💬 andrewtoth commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1451818200)
nit
```
// Verify a block close to the tip enables limited peers connections
```

👍 yahiheb approved a pull request: "Fix typos"
(https://github.com/bitcoin/bitcoin/pull/29246#pullrequestreview-1820328168)
ACK [a1b1798](https://github.com/bitcoin/bitcoin/pull/29246/commits/a1b1798b1652e93caf66e5e1c8353bcd7951f1c4)

💬 Farnoosh85 commented on issue "ci: failure in `wallet_basic.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/29110#issuecomment-1891204537)
0xE7cF1C52C9f179B855D7a31eA79997882269057A send your bnb bep20 address my

💬 torkelrogstad commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#issuecomment-1891514722)
@ismaelsadeeq I fixed the tests per your comments. Note that I pass the arguments through both the `arg_` prefixed and non-prefixed versions. Figured we might as well validate that there's no behavior changes between passing as positional arguments or through `options`.

Also added a new commits with tests that validates that `estimate_mode` is passed together with a confirmation target.

💬 ajtowns commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#discussion_r1452031664)
Might be better if this information were also exposed to the gui? Perhaps something along the lines of:

```c++
auto& cm = active_chainstate.m_chainman;
if (!cm.IsInitialBlockDownload()) {
cm.GetNotifications().progress("Loading mempool...", percentage_done, false);
}
```

would make sense? (completely untested)

💬 ajtowns commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1891521307)
> "I wonder why Bitcoin Core takes so long to shutdown sometimes?" repeatedly from users.

I don't think that's due to dumping the mempool, but rather spinning on locks elsewhere in the code. I've seen very slow shutdowns myself, but been unable to nail it down precisely.

👍 BrandonOdiwuor approved a pull request: "wallet: Reset chain notifications handler if AttachChain fails"
(https://github.com/bitcoin/bitcoin/pull/29243#pullrequestreview-1821161372)
Code Review ACK ea2551e55d260854a5cca8aa95034970d4adca1c

💬 S3RK commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#issuecomment-1891638400)
Code review ACK c856a1093009c6098c7c9fed5cc3f03fd0c0cf5b

Nice improvements removing duplication between `CreateRecipient` and `AddOutputs`, also embedding SFFO values in `CRecipient` instead of passing as a separate parameter to `FundTransaction`.

I'd still prefer `InterpretSubtractFeeFromOutputInstructions` to work with one type of SFFO option - `vector<int>` and process others at the call site. SFFO option can be in three different forms and `vector<int>` is the only form that used multi
...

💬 fanquake commented on pull request "[POC] guix: produce a fully `-static-pie` bitcoind":
(https://github.com/bitcoin/bitcoin/pull/25573#issuecomment-1891768915)
> Looks like support in glibc will also be available soon: https://sourceware.org/pipermail/libc-alpha/2023-October/152132.html.

This has now landed: https://sourceware.org/git/?p=glibc.git;a=commit;h=e0590f41fe1e7a54169e8f8828efe62b5064139e.

💬 fanquake commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#discussion_r1452167939)
It's weird that miniupnpc would add a special build system option for setting a single, Windows-only define. It's not really clear what the benefits of taht are over just using `CPPFLAGS`, which should always work. The only rationale from the PR author for not doing that was [`I don't like it.`](https://github.com/miniupnp/miniupnp/pull/659#issuecomment-1585595562).

✅ fanquake closed an issue: "ci: failure in `wallet_assumeutxo.py --descriptors`"
(https://github.com/bitcoin/bitcoin/issues/29234)

🚀 fanquake merged a pull request: "wallet: Reset chain notifications handler if AttachChain fails"
(https://github.com/bitcoin/bitcoin/pull/29243)

💬 maflcko commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1891802590)
> > Seems confusing to have a test runner that calls another runner (`all-lint.py`), which calls a subset of the lint tests.
>
> However, it is quite convenient to run `./test/lint/all-lint.py` locally, no?

Why would it be convenient, when it is called "all-lint", but at the same time skips checks such as the fs check, the subtree check, and the doc check?