💬 luke-jr commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1622750027)
Seems like it'd be nicer to just pass `state` into the function rather than creating a new one and copying it.

💬 Sjors commented on issue "Make Transport independent of CNetMessage and CSerializedNetMsg":
(https://github.com/bitcoin/bitcoin/issues/30209#issuecomment-2142747413)
By "command" you mean "m_type"?

```cpp
class CNetMessage
{
public:
DataStream m_recv; //!< received message data
std::chrono::microseconds m_time{0}; //!< time of message receipt
uint32_t m_message_size{0}; //!< size of the payload
uint32_t m_raw_message_size{0}; //!< used wire size of the message (including header/checksum)
std::string m_type;

...
};
```

`Sv2Transport` could set `m_type` to the empty string. The Template
...

👍 ismaelsadeeq approved a pull request: "Fix waste calculation in SelectionResult"
(https://github.com/bitcoin/bitcoin/pull/28366#pullrequestreview-2091447335)
Code Review ACK bd34dd85e7b8b4cc26d2173d84bbeda2e9c27624

💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1622827428)
> Seems like it'd be nicer to just pass state into the function

I am taking this review comment as a non-blocking (nit) suggestion based on the tone.

I will update this when there are blocking comments that indicate a need to retouch the diffs.

Also, I would like to know whether this PR is conceptually acknowledged, @luke-jr.
Thanks for your review.

💬 Sjors commented on pull request "net: have GetReceivedMessage() return bytes":
(https://github.com/bitcoin/bitcoin/pull/30213#issuecomment-2142814268)
As discussed in https://github.com/bitcoin/bitcoin/issues/30209#issuecomment-2142747413 I'm going to try a different approach first.

✅ Sjors closed a pull request: "net: have GetReceivedMessage() return bytes"
(https://github.com/bitcoin/bitcoin/pull/30213)

💬 boring877 commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2142821401)
Testnet 3 will always be the only testnet. its the market who make the rules !

💬 Self-Hosting-Group commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2142841652)
Maybe too late or has been looked at. A PCP/NAT-PMP library https://github.com/libpcp/pcp

💬 maflcko commented on pull request "ci: move ASAN job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#issuecomment-2142855246)
Please remove the Cirrus task in the same commit, to make review easier.

👋 mzumsande's pull request is ready for review: "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling"
(https://github.com/bitcoin/bitcoin/pull/29664)

💬 mzumsande commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2142874157)
Cleaned up a bit, rebased and marked as ready for review!

🤔 murchandamus reviewed a pull request: "fuzz: add more coverage for `ScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/30134#pullrequestreview-2091652125)
Tested ACK e3249f21111f1dd4beb66f10af933c34a36c30ac

I also fuzzed a bit: I ran a recent commit of the `master` branch and this PR’s e3249f21111f1dd4beb66f10af933c34a36c30ac against `qa-assets:HEAD` and then again after fuzzing about 12 CPU hours on top of `qa-assets:HEAD`.

| seeds | `qa-assets` | `qa-assets` + fuzzing |
| --- | ------------- | ------------- |
| `master` | cov: 16525 ft: 96185 | cov: 16874 ft: 104431 |
| #30134: e3249f21111f1dd4beb66f10af933c34a36c30ac | cov: 17095 ft:
...

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2143004073)
> If we were in a situation where Testnet has no ASICs mining it and someone points a millionth of the mainnet hashpower at Testnet (today about 650 TH/s), they could easily mine 10 difficulty periods in minutes and put the first block of the next difficulty period well out of the range of non-ASICs.

The hashpower on Testnet3 seems to have been fluctuating around ~500 TH/s over the past 2 years: https://mempool.space/testnet/graphs/mining/hashrate-difficulty If that data is correct, I am not
...

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1622999723)
@maflcko Thanks, problem seems fixed indeed!

💬 theuni commented on pull request "refactor: use recommended type hiding on multi_index types":
(https://github.com/bitcoin/bitcoin/pull/30194#issuecomment-2143082508)
Accidentally pushed a temp branch here, ignore the noise. Still ready for review.

💬 theStack commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1623063336)
As already [mentioned in the secp256k1 PR](https://github.com/bitcoin-core/secp256k1/pull/1519#issuecomment-2143167510), this assert makes the indexer crash on signet on block 198023 for tx https://mempool.space/signet/tx/d73f4a19f3973e90af6df62e735bb7b31f3d5ab8e7e26e7950651b436d093313, which contains input pubkeys that add up to zero (point of infinity):

```
.....
2024-05-31T22:31:57Z Syncing bip352 index with block chain from height 113222
2024-05-31T22:32:27Z Syncing bip352 index with b
...

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2143396412)
> Maybe too late or has been looked at. A PCP/NAT-PMP library https://github.com/libpcp/pcp

Yes, some revievers have been looking at it as reference/comparison.

This has been said before but: implementing a self-contained version of the RFCs here is intentional, we don't want to introduce a dependency on a third-party library. It shoudn't be needed for a straightforward fixed packet-based protocol. The intent is to have code that integrates with bitcoin core's frameworks for logging, netwo
...

👍 tdb3 approved a pull request: "doc: JSON-RPC request Content-Type is application/json"
(https://github.com/bitcoin/bitcoin/pull/30215#pullrequestreview-2092102442)
ACK for 3c08e11c3ea4499e8d20609e2417cac859b3e98e

This is a reasonable change, which over time can help nudge implementations to usage of the more appropriate content-type `application/json`.

#### Spec tracing
I didn't see mention of content-type or linked RFC in the JSON-RPC v1 spec (https://www.jsonrpc.org/specification_v1), but the JSON-RPC 2.0 spec does reference RFC 4627 (https://www.ietf.org/rfc/rfc4627.txt), which specifies (in section 6) the type/subtype of `application/json`. RFC
...

💬 brunoerg commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#issuecomment-2143444520)
Concept ACK

🤔 Amygo777 reviewed a pull request: "JSON-RPC request Content-Type is application/json"
(https://github.com/bitcoin/bitcoin/pull/29946#pullrequestreview-2092111082)
Nice