💬 l0rinc commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1778709371)
we could add a few more corner cases, e.g. `\n\n` and `\n ` -> otherwise the tests would pass for a `contains("\n")` or `count("\n") == 1` as well

💬 l0rinc commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1778720770)
> `Assume` should be used to document assumptions when program execution can safely continue even if the assumption is violated.

Shouldn't this be an `assert` instead?

💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1778745490)
Ah, I wasn't sure why that was. Fuzzer compiles and linter seems happy so updated, thanks!

🤔 mzumsande reviewed a pull request: "p2p: rpc: add `tx_reconciliation` to `getpeerinfo`"
(https://github.com/bitcoin/bitcoin/pull/30984#pullrequestreview-2333964060)
This looks similar to #26602 / #27797 - I don't think any Erlay PR has been merged since then, so the reasoning from back then should still apply?

💬 davidgumberg commented on pull request "Don't zero-after-free `DataStream`: ~25% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2379480304)
## Appendix

<details>

<summary>

### Benchmarks

</summary>

Command being timed:
```bash
./src/bitcoind -daemon=0 -connect=amd-ryzen-7900x-node:8333 -stopatheight=815000 -port=8444 -rpcport=8445 -dbcache=2048 -prune=550 -debug=bench -debug=blockstorage -debug=coindb -debug=mempool -debug=prune"
```

I applied my branch on
[6d546336e800](https://github.com/bitcoin/bitcoin/commit/6d546336e800), which is
"master" in the data below.

Average master time (hh:mm:ss): 48:17:15 (17
...

💬 maflcko commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1778761138)
It does not append a `\n`. It removes one and adds one, the effect of which is a nullopt.

I understand that this is a bit confusing and it would be better if there were no `\n` anywhere in log strings, but this can be done in the future. For example when doing other breaking changes, like switching to `std::format` in C++23 in a few years down the line. Closing for now.

💬 maflcko commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1778763759)
A missing `\n` is a harmless, albeit confusing, stylistic issue in the debug logs. I'd very much say that the program can safely continue and that this is not a fatal error.

💬 sr-gi commented on pull request "p2p: rpc: add `tx_reconciliation` to `getpeerinfo`":
(https://github.com/bitcoin/bitcoin/pull/30984#issuecomment-2379488317)
> This looks similar to #26602 / #27797 - I don't think any Erlay PR has been merged since then, so the reasoning from back then should still apply?

I was unaware of this. I'm happy to cherry-pick these two commits into one of the upcoming Erlay PRs, especially once the network messaging bits have been included.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1778767304)
Removed, thanks!

💬 maflcko commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1778769226)
There aren't any Windows newlines anywhere in log lines, and this pull requests isn't changing anything about that. So I'll leave this as-is for now.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2379495804)
`03f6cc2b4a...70c2f13f83`: fix CI failure, and address suggestions

> Can you put sockman.h in libbitcoin_common

Done.

💬 maflcko commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1778770906)
Seems fine to document twice. I don't really see the risk or downside. Leaving as-is for now.

💬 fanquake commented on issue "win64-cross CI timeout after 2h ":
(https://github.com/bitcoin/bitcoin/issues/30969#issuecomment-2379500101)
Maybe the documentation is not correct, or `ctest` is buggy.

Cross compiling master for Windows, and running `ctest --test-dir build --extra-verbose`, I see:
```bash
ctest --test-dir build --extra-verbose
Internal ctest changing into directory: /root/ci_scratch/build
UpdateCTestConfiguration from :/root/ci_scratch/build/DartConfiguration.tcl
UpdateCTestConfiguration from :/root/ci_scratch/build/DartConfiguration.tcl
Test project /root/ci_scratch/build
Constructing a list of tests
Do
...

💬 mzumsande commented on pull request "p2p: rpc: add `tx_reconciliation` to `getpeerinfo`":
(https://github.com/bitcoin/bitcoin/pull/30984#issuecomment-2379505903)
> I was unaware of this. I'm happy to cherry-pick these two commits into one of the upcoming Erlay PRs, especially once the network messaging bits have been included.

Sounds good to me. I still think that this (and anything else that is user-facing) should make it into master/releases as soon as Erlay is functional (not necessarily activated by default) but not before.

💬 brunoerg commented on pull request "p2p: rpc: add `tx_reconciliation` to `getpeerinfo`":
(https://github.com/bitcoin/bitcoin/pull/30984#issuecomment-2379530391)
> This looks similar to #26602 / #27797 - I don't think any Erlay PR has been merged since then, so the reasoning from back then should still apply?

I completely forgot my own past PR haha. But one of the main motivation is to improve testing which currently relies on logs.

💬 brunoerg commented on pull request "p2p: rpc: add `tx_reconciliation` to `getpeerinfo`":
(https://github.com/bitcoin/bitcoin/pull/30984#issuecomment-2379532585)
> Sounds good to me. I still think that this (and anything else that is user-facing) should make it into master/releases as soon as Erlay is functional (not necessarily activated by default) but not before.

Sounds good to me as well. Closing this for now.

✅ brunoerg closed a pull request: "p2p: rpc: add `tx_reconciliation` to `getpeerinfo`"
(https://github.com/bitcoin/bitcoin/pull/30984)

🤔 stickies-v reviewed a pull request: "[28.x] backports and finalize (or rc3)"
(https://github.com/bitcoin/bitcoin/pull/30959#pullrequestreview-2334037773)
code LGTM 98745e03ffc9f69f901b827e19e4d8d645a27112

Verified all backport commits are clean and make sense, and that I'm getting the same manpages. I think `doc/release-notes.md` still needs to be updated though?

⚠️ maflcko opened an issue: "ci: Intermittent issue in feature_fee_estimation.py in sanity_check_rbf_estimates (Block sync timed out after 2400s)"
(https://github.com/bitcoin/bitcoin/issues/30990)
A quick skim of the log looks like there are two blocks at the same height, but I haven't looked too closely yet.

https://cirrus-ci.com/task/6299307458953216?logs=ci#L4251

💬 mzumsande commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2379597880)
Concept ACK

> In #29618 and here, I don't see the motivation to forbid v1 connections.

Well, to gain the benefits of BIP324. For example if you are concerned about someone inspecting the traffic to your node it really doesn't matter how many BIP324 peers you have if you have just one unencrypted connection. You could also run tor-only in this case, but that has its own disadvantages and `-onlynet=onion` shares the exact same downside that if everyone did it, the network would split. If som
...

📝 ismaelsadeeq opened a pull request: "test: enable running independent functional test methods"
(https://github.com/bitcoin/bitcoin/pull/30991)
- Some test methods in the functional test framework are independent and do not require any prior context or setup in `run_test`.
- This commit adds a new option for running these specific methods within a test file, allowing them to be executed individually without running the entire test suite.
- Using this option reduces the time you need to wait before the test you are interested in starts executing.
- The functionality added by this PR can be achieved manually by commenting out code, but
...