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

🚀 fanquake merged a pull request: "doc: Add missing backtick in developer notes logging section"
(https://github.com/bitcoin/bitcoin/pull/29241)

💬 fanquake commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#issuecomment-1891821253)
Concept ACK for 27.0.

💬 maflcko commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#discussion_r1452200703)
Missing test coverage seems unrelated to this pull? (This line and condition is not changed)

💬 maflcko commented on pull request "logging: Update to new logging API":
(https://github.com/bitcoin/bitcoin/pull/29231#issuecomment-1891833356)
> happy to open a separate pull for it too?

Yeah, I think it is better to change a line of code only once, instead of twice, unless there is a reason to change it twice.

💬 torkelrogstad commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#issuecomment-1891915104)
I believe CI failures are unrelated. Is it possible to force CI to re-run?

💬 torkelrogstad commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1891924076)
> However, it is quite convenient to run `./test/lint/all-lint.py` locally, no?

Chiming in as a very inexperienced contributor that has been bashing against CI linter failures: I would say it's very nice to have an easy way to run all tests and checks locally before pushing. The correct thing to do here would be to _expand_ the linter script to include more of the checks, IMO.

On a slightly related note: it would also be nice to include either instructions or CLI flags that fix linter com
...

⚠️ fanquake opened an issue: "build: multiprocess compile failure on macOS"
(https://github.com/bitcoin/bitcoin/issues/29248)
master @ 28ccc7003a4c3f198a255d6f55f5748d4fd8fd1a

```bash
Apple clang version 15.0.0 (clang-1500.1.0.2.5)

make -C depends -j12 MULTIPROCESS=1 NO_QT=1 NO_WALLET=1 NO_NATPMP=1 NO_UPNP=1 NO_ZMQ=1
./autogen.sh
CONFIG_SITE=/bitcoin/depends/aarch64-apple-darwin23.2.0/share/config.site ./configure
make
```

```bash
In file included from ipc/capnp/echo.capnp.proxy-client.c++:3:
In file included from ./ipc/capnp/echo.capnp.proxy-types.h:6:
In file included from ./ipc/capnp/echo.capnp.prox
...