✅ fanquake closed a pull request: "refactor: refactored platform assignment into get_platform function"
(https://github.com/bitcoin/bitcoin/pull/29971)

💬 fanquake commented on pull request "refactor: refactored platform assignment into get_platform function":
(https://github.com/bitcoin/bitcoin/pull/29971#issuecomment-2142510334)
Thanks for the contribution, however we are going to leave this code as-is for now.

💬 fanquake commented on pull request "RFC: depends: add release type to CMake builds":
(https://github.com/bitcoin/bitcoin/pull/29962#issuecomment-2142512483)
This "seems" like the right thing to do, but I'm also not sure. Could you rebase here, to incorporate other recent CMake changes, and so it's easier to test?

💬 fanquake commented on pull request "Lint: support running individual rust linters and improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#issuecomment-2142513388)
> Looks like the conflicting https://github.com/bitcoin/bitcoin/pull/30034 is close(r) to merge, if you want to review it

This is now merged. Can you rebase here?

💬 fanquake commented on pull request "depends: Use `CC_FOR_BUILD` for `config.guess `":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2142519151)
Is this still an issue given recent CMake changes?

👍 hebasto approved a pull request: "build: remove `--enable-lcov-branch-coverage`"
(https://github.com/bitcoin/bitcoin/pull/30192#pullrequestreview-2091159773)
ACK cbd4640ede92a1a5d7b7c1367eb7c00a9f476c62, tested on Ubuntu 22.04.

💬 hebasto commented on pull request "build: remove `--enable-lcov-branch-coverage`":
(https://github.com/bitcoin/bitcoin/pull/30192#discussion_r1622610069)
Not only "when configuring". Providing `LCOV_OPTS` variable when building the `cov` or `cov_fuzz` targets also works and improves flexibility:
```
./configure --enable-lcov CXXFLAGS="-fprofile-update=prefer-atomic"
make
make cov LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch"
```

💬 fanquake commented on pull request "build: remove `--enable-lcov-branch-coverage`":
(https://github.com/bitcoin/bitcoin/pull/30192#discussion_r1622616533)
> Not only "when configuring".

Sure. This is just one example.

💬 sipa commented on issue "Make Transport independent of CNetMessage and CSerializedNetMsg":
(https://github.com/bitcoin/bitcoin/issues/30209#issuecomment-2142537783)
Two ideas to address this.

### Make the command type implicitly part of the message being sent.

So instead of taking a `CSerializedNetMsg` in `SetMessageToSend`, the interface would simply take an `std::vector<std::byte>` for example, which in some way stores the message type. For example, by convention in V1Transaction and V2Transport, the command type could be the first 12 bytes. So from the perspective of net_processing, sending e.g. a `ping` message would look like sending 20-byte mess
...

📝 fanquake opened a pull request: "doc: JSON-RPC request Content-Type is application/json"
(https://github.com/bitcoin/bitcoin/pull/30215)
Specify json content type in RPC examples.

Picks up #29946. Which needed rebasing and the commit message fixing,

💬 sipa commented on issue "Make Transport independent of CNetMessage and CSerializedNetMsg":
(https://github.com/bitcoin/bitcoin/issues/30209#issuecomment-2142542534)
I'm happy to try taking a stab at both. The second idea would take more time, I expect.

✅ fanquake closed a pull request: "JSON-RPC request Content-Type is application/json"
(https://github.com/bitcoin/bitcoin/pull/29946)

💬 fanquake commented on pull request "JSON-RPC request Content-Type is application/json":
(https://github.com/bitcoin/bitcoin/pull/29946#issuecomment-2142542647)
Picked this up in #30215, given this is a very straightforward change, that just needed rebase and the commit message fixing.

💬 fanquake commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1622620624)
@pinheadmz did you end up following up to these comments?

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1622621496)
Getting there...

💬 fanquake commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2142599331)
@theuni do you have any thoughts here?

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#issuecomment-2142608188)
I've slightly extended the approach adding 3 commits to deal with short id collisions, which were not taken into account. Some of this may be squashable, I've added them separately for now so they are easy to diff/review.

This would be missing an additional commit/amend to deal with https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1401014445, which I overlooked when addressing the outstanding comments

💬 Sjors commented on issue "Make Transport independent of CNetMessage and CSerializedNetMsg":
(https://github.com/bitcoin/bitcoin/issues/30209#issuecomment-2142631320)
Perhaps a third approach is to shoehorn `Sv2MsgType` into `CNetMessage`. I haven't tried that yet. In terms of elegance it's probably no worse than option (1) above.

💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2142645727)
Update: Since leaving these servers alone for a week and not rebooting them or restarting bitcoind, they have stayed perfectly in sync without issues. So it looks like the problem is triggered by starting and stopping bitcoind (which I can live with, if I do have to restart a server and I get data corruption, I'll image the data from another server).

💬 sipa commented on issue "Make Transport independent of CNetMessage and CSerializedNetMsg":
(https://github.com/bitcoin/bitcoin/issues/30209#issuecomment-2142648687)
@Sjors I guess that works! It could just ignore the command name, and always decode that to ""?