🤔 stickies-v reviewed a pull request: "[WIP] net: return result from addnode RPC"
(https://github.com/bitcoin/bitcoin/pull/30381#pullrequestreview-2158639334)
There's a lot of stuff happening in one commit:
1. behaviour change: allow removing a v2 connection when `!node_v2transport`
2. behaviour change: explicitly throwing a a `JSONRPCError` when opening the connection for a `onetry` fails, instead of returning NULL (which makes it indistinguishable from a success operation)
3. behaviour change: always returning a results object with input values and `result==success`
4. clang-tidy fixes

I would suggest breaking things up a bit and describing t
...

💬 stickies-v commented on pull request "[WIP] net: return result from addnode RPC":
(https://github.com/bitcoin/bitcoin/pull/30381#discussion_r1665533399)
nit: is this necessary?

💬 glozow commented on pull request "#28984 package rbf followups":
(https://github.com/bitcoin/bitcoin/pull/30295#discussion_r1665523241)
Renaming would help distinguish the variables which are currently reassigned later. Also, since fee doesn't matter.

```suggestion
_, placeholder_txns3 = self.create_simple_package(coin)
```

💬 glozow commented on pull request "#28984 package rbf followups":
(https://github.com/bitcoin/bitcoin/pull/30295#discussion_r1665526245)
nit: variable naming could be more clear, e.g. `fail_package_hex3` and `success_package_hex3` instead of just numbers

💬 glozow commented on pull request "#28984 package rbf followups":
(https://github.com/bitcoin/bitcoin/pull/30295#discussion_r1665529930)
nit: out of scope for this PR, but I think it's high time we create a constant that represents 1 satoshi as a `Decimal`. Would save a lot of time counting zeroes...

💬 glozow commented on pull request "#28984 package rbf followups":
(https://github.com/bitcoin/bitcoin/pull/30295#discussion_r1665536084)
I think adcfd69e443475f941c0b0f9d1a0246801ab5dad can be dropped.

See https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1665534798. The error strings are correct to be in past tense instead of conditional.

💬 maflcko commented on issue "RPC `getblock` resulted in 500 and ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0)":
(https://github.com/bitcoin/bitcoin/issues/20978#issuecomment-2208691414)
> However there is also a log message in OP from `ReadBlockFromDisk()` which was passed a flat file position it was unable to read from (which is a server error, and should be 500 IMO)

The issue explained in OP can normally happen in normal operation. See https://github.com/bitcoin/bitcoin/issues/20978#issuecomment-1463557219 on how to reproduce.

This is still the case. Header exists, but block has no data:

```
$ curl 'http://__cookie__:a@127.0.0.1:18442' --data '{"method":"getblock",
...

💬 glozow commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#issuecomment-2208699739)
Needs rebase

📝 ismaelsadeeq opened a pull request: "BlockAssembler: return selected packages vsize and feerate"
(https://github.com/bitcoin/bitcoin/pull/30391)
This PR enables `BlockAssembler` to add all selected packages' feerate and vsize to a vector, and then return the vector as `vFeeratePerSize` in the `CBlockTemplate` struct.

This PR is the first step in the #30157 project.

The packages' vsize and fee rates are used in #30157 to determine the percentile fee rate of the top block in the mempool.

The first commit changes the data type of `CFeeRate` size from `uint32_t` to `uint64_t` because the `BlockAssembler::addPackageTxs` package size
...

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2208702356)
Addressed @dergoegge and @vasild comments. Ready for review again, reACKs would be appreciated!

💬 glozow 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-2208707191)
concept ACK

💬 fanquake commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2208745034)
@instagibbs @sr-gi maybe @mzumsande. This will likely affect you. Any feedback?

⚠️ ismaelsadeeq opened an issue: "Improving Fee estimation tracking issue"
(https://github.com/bitcoin/bitcoin/issues/30392)
This is a tracking issue for an effort to improve fee estimation on Bitcoin Core.

For the conceptual motivation behind this work, see issue #27995 and the previous discussion on Delving Bitcoin: [Mempool-Based Fee Estimation on Bitcoin Core](https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703/13),

What to Review

- #30391
- #30275

Roadmap

- [ ] #30391
- [ ] Add Fee estimator Module / Mempool forecaster and unit tests
- [ ] Create `estimatefee` rpc an
...

💬 Sjors commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2208753220)
So the `Sv2Connman` is called, but `start()` isn't, so the mock `BindListenPort` doesn't happen.

💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1665606867)
I looked into catching specifically `EAI_ADDRFAMILY` errors but this only applies on GNU Libc and is not POSIX. I checked windows and they have different errors. Rather than try to catalogue every possible system and libc implementation, the code now retries on any error (assuming you first tried with `AI_ADDRCONFIG` in the first place).

💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1665607508)
Good suggestion thanks, implemented.

📝 maflcko opened a pull request: " net_processing: use existing RNG object in ProcessGetBlockData "
(https://github.com/bitcoin/bitcoin/pull/30393)
Two small rand fixups. The debug mode one is a follow-up to commit 7af95afa8bc3f36a37d082ef3475cb3e0bd3a0e4 and https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658344049. The RNG object one to commit 8e31cf9c9b5e9fdd01e8b220c08a3ccde5cf584c

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1665615924)
First half done in https://github.com/bitcoin/bitcoin/pull/30393

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1665616205)
Done in https://github.com/bitcoin/bitcoin/pull/30393

💬 fjahr commented on pull request "validation: Check if mempool exists before size check in ActivateSnapshot":
(https://github.com/bitcoin/bitcoin/pull/30388#discussion_r1665621472)
nit: The type doesn't seem so complex that it has to be auto

```suggestions
CTxMemPool* mempool{m_active_chainstate->GetMempool()};
```