👍 brunoerg approved a pull request: "doc, test: test and explain service flag handling"
(https://github.com/bitcoin/bitcoin/pull/29213#pullrequestreview-1823365366)
utACK 74ebd4d1359edce82a134dfcd3da9840f8d206e2

📝 kristapsk opened a pull request: "Don't use scientific notation in "Dumped mempool" log message"
(https://github.com/bitcoin/bitcoin/pull/29254)
Don't see any benefits here, only harder to read for most of the users.

Before:
```
2024-01-16T13:11:36Z Dumped mempool: 8.165e-06s to copy, 0.00224268s to dump
```

After:
```
2024-01-16T13:11:36Z Dumped mempool: 0.000008165s to copy, 0.00224268s to dump
```

💬 brunoerg commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1893758411)
I use `all-lint.py` frequently, it's convenient even not running all lints de facto. However, since there is other way to achieve the same and it's documentated, Concept ACK on removing it.

💬 theuni commented on pull request "build: depends move macOS C(XX) FLAGS out of C & CXX":
(https://github.com/bitcoin/bitcoin/pull/29233#issuecomment-1893773441)
> > Or are we adding it to the linker elsewhere?
>
> We are passing the min version as part of `-platform_version` in `darwin_LDFLAGS`. Note that we also check that the release bins have this min os set in `symbol-check.py` (along with sdk & linker version).

Heh, right. The next line. Not sure how I missed that :)

utACK cbc9bf11fe84deb96daf9b97a8e7499979360db2

💬 maflcko commented on pull request "log: Don't use scientific notation in "Dumped mempool" message":
(https://github.com/bitcoin/bitcoin/pull/29254#issuecomment-1893776655)
I don't think nanosecond precision helps here. Might as well limit to `%.2fs`?

Same here?

```
src/policy/fees.cpp: LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n", num_entries, Ticks<SecondsDouble>(endclear - startclear));

👍 vasild approved a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1823533958)
ACK 79e10351b1ce1f8db98ece67aacc24f323008b37

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1453506519)
I've removed this commit. Going to open followup PR for it shortly

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1453507069)
done

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1453507149)
Taken. Also realized I forgot to divide by 4, so fixed the test

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1453507229)
Right, removed

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1453507442)
Right, done

💬 maflcko commented on pull request "refactor: Allow std::span construction from CKey":
(https://github.com/bitcoin/bitcoin/pull/29133#issuecomment-1893861762)
I dropped the last commit, to not introduce new `std::span` usage for now. To verify the changes in this pull, the commit's diff can be used. (Fails to compile before, compiles after):

```diff
diff --git a/src/key.cpp b/src/key.cpp
index 2bd6396298..300c95e35a 100644
--- a/src/key.cpp
+++ b/src/key.cpp
@@ -385,7 +385,7 @@ bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const {
return key.Derive(out.key, out.chaincode, _nChild, chaincode);
}

-void CExtKey::SetSeed(Spa
...

💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1893868905)
https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1452493222

> are you still working on this?

I don't think I will work on this PR in the nearest future.

✅ hebasto closed a pull request: "ci, iwyu: Update mappings"
(https://github.com/bitcoin/bitcoin/pull/27710)

🤔 maflcko reviewed a pull request: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960#pullrequestreview-1823257728)
Left some questions, feel free to ignore.

💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1453524696)
nit: Any reason to call it "main", when main.cpp has long been split up and renamed to validation.cpp? Could call it `validation_signals`, or leave it as is.

💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1453516899)
Wouldn't it be better to use existing `chainman` references to get the signals reference, instead of using unchecked pointer dereferences? (Same in all other places)

💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1453413538)
nit in the first commit: Could drop `signals` and use `active_chainstate.GetMainSignals()` instead?

💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1453405057)
first commit: Why check below, but not here, for nullptr?

💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1453382756)
in the first commit: Any reason to re-order this and remove the doc comment?