🚀 fanquake merged a pull request: "[25.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/28768)

💬 vasild commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1893591435)
Concept ACK on removing the library.

This PR documents the library as deprecated, removes its tests but leaves the library. So, for some time there will be a library that will be untested. What is the merit of doing that? Isn't it better to document it as deprecated now and later remove the library and its tests at the same time?

👍 stickies-v approved a pull request: "logging: Update to new logging API"
(https://github.com/bitcoin/bitcoin/pull/29231#pullrequestreview-1823164145)
ACK 1441ab74d6cd40e75f1dba80b68bc4b1791bf667

👍 stickies-v approved a pull request: "[26.x] more backports"
(https://github.com/bitcoin/bitcoin/pull/29209#pullrequestreview-1823233187)
ACK 234f32860dd0e450e214c93af09e9dd9df20e9ea

💬 Kenshiro-28 commented on issue "Implement PayJoin / Pay-to-EndPoint":
(https://github.com/bitcoin/bitcoin/issues/19148#issuecomment-1893693072)
@DanGould while PayJoin offers improved privacy over traditional Bitcoin transactions, there's a concern about the potential exposure of UTXO balances. This is particularly risky for users with substantial BTC holdings, as the balance of the UTXOs used in a transaction is visible to the payee.

I propose an enhancement to PayJoin that involves using UTXOs below a certain threshold (e.g., 0.1 BTC). This would entail:

- Threshold Implementation: Automatically selecting UTXOs for PayJoin trans
...

💬 kristapsk commented on issue "Implement PayJoin / Pay-to-EndPoint":
(https://github.com/bitcoin/bitcoin/issues/19148#issuecomment-1893710520)
For payjoin receiver, IMHO it makes sense to always contribute smallest amount UTXO available. That way you likely don't expose some big value UTXOs (assuming they aren't the only ones in your wallet) and also it works as UTXO consolidation.

💬 stickies-v commented on pull request "log: Nuke error(...)":
(https://github.com/bitcoin/bitcoin/pull/29236#discussion_r1453410392)
nit: it seems the return value from `SerializeFileDB` is never used to actually shut down the node. In the case of failing to open file, I think `LogError` is appropriate because that probably should lead to a shutdown. In the case of "Failed to flush file", I think a `LogWarning` could be more appropriate.

I think it's best to leave the PR as is to make progress, bikeshedding over warning vs error categories doesn't have a huge amount of benefit here. Just leaving this comment for visibility
...

🤔 stickies-v reviewed a pull request: "log: Nuke error(...)"
(https://github.com/bitcoin/bitcoin/pull/29236#pullrequestreview-1823323902)
Approach ACK fa7658b8d190580eab385691a5e571035e399129

I too have been confused by `return error` many a times.

💬 brunoerg commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1893731972)
Concept ACK

👍 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