🤔 glozow reviewed a pull request: "policy: restrict all TRUC (v3) transactions to 10KvB"
(https://github.com/bitcoin/bitcoin/pull/29873#pullrequestreview-2065844531)
fixed commit message, thanks :+1:

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2120121913)
Rebased for #29817

💬 stickies-v commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2120147856)
Concept ACK

💬 fanquake commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/30120#issuecomment-2120154716)
Guix build (aarch64):
```bash
e7f19ceb7286ff81d1464575418d494dc2143d43cdc8b0e9a1bff7e47cffc773 guix-build-a057869aa3c4/output/aarch64-linux-gnu/SHA256SUMS.part
f7e9fa7ada1cc470342e7851862b25a026add7dbd2a441f22327f516268eabb3 guix-build-a057869aa3c4/output/aarch64-linux-gnu/bitcoin-a057869aa3c4-aarch64-linux-gnu-debug.tar.gz
30e3001bf9e9ea03defd21f094887e7c9a5f5b4c95162fbf5ea7d9cbe7458458 guix-build-a057869aa3c4/output/aarch64-linux-gnu/bitcoin-a057869aa3c4-aarch64-linux-gnu.tar.gz
3c520e
...

👍 cbergqvist approved a pull request: "cli: Detect port errors in rpcconnect and rpcport"
(https://github.com/bitcoin/bitcoin/pull/29521#pullrequestreview-2065901985)
re ACK 24bc46c83b39149f4845a575a82337eb46d91bdb

Inspected `git range-diff 867a673~2..867a673 24bc46c~2..24bc46c`.

Ran same tests as last time (https://github.com/bitcoin/bitcoin/pull/29521/#pullrequestreview-2046768298) with same results.

Tried to repro a boiled down version of the mysterious compiler warning but was unable to reproduce it. Here is what I ended up with: https://godbolt.org/z/fh5KcbbeW
(Not sure about the legalese around uploading Bitcoin Core source to Godbolt but just
...

💬 glozow commented on pull request "refactor: TxDownloadManager":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2120203238)
Rebased for #29817 and added a "ensure we can always download a tx as long as we have 1 good outbound peer" fuzz test

💬 maflcko commented on pull request "lint/contrib/doc updates":
(https://github.com/bitcoin/bitcoin/pull/30084#discussion_r1606611069)
I think the key point of both sentences is that all dependency versions must match exactly. Installing via distro packages or via pip are just two ways. Both can fail in the same way (outdated or too-recent dependency versions), so communicating them in the same section makes sense, but again, just a nit.

📝 hebasto opened a pull request: "doc: Update NetBSD Build Guide"
(https://github.com/bitcoin/bitcoin/pull/30143)
This PR updates the NetBSD Build Guide to reflect:
- the recent NetBSD Release
- GCC minimum supported version update (https://github.com/bitcoin/bitcoin/pull/28348 and https://github.com/bitcoin/bitcoin/pull/29091)
- Python minimum supported version update (https://github.com/bitcoin/bitcoin/pull/28211)

Also a smaller package set has been suggested:
- `boost-headers` instead of the full `boost`
- `qt5-qtbase qt5-qttools` instead of the full `qt5` (similar to https://github.com/bitcoin/b
...

💬 maflcko commented on pull request "doc: Update NetBSD Build Guide":
(https://github.com/bitcoin/bitcoin/pull/30143#issuecomment-2120337184)
utACK 85e480a41ad0901a8b46759e921c187ebbbcccdf

💬 dergoegge commented on pull request "refactor: TxDownloadManager":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1606683947)
Structs that are part of the public interface should probably go into `txdownloadman.h`. Otherwise one needs to include the impl header, e.g. like `txdownloadman.h` does right now, which defeats the purpose of pimpling to some extend.

💬 dergoegge commented on pull request "refactor: TxDownloadManager":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1606717505)
Iiuc correctly, clearing the filters here has been removed as we don't want `TxDownloadManager` to depend on `ChainstateManager`.

An alternative to the new synchronous validation interface callback could be to preserve the previous behavior (clearing the filters in `AlreadyHaveTx` on a tip change) by introducing an interface for TxDownloadMan to fetch the latest tip hash. For example, `TxDownloadOptions` could be extended to hold a lambda that returns the latest tip hash, which in production
...

👍 dergoegge approved a pull request: "Several randomness improvements"
(https://github.com/bitcoin/bitcoin/pull/29625#pullrequestreview-2066132059)
Code review ACK 636e4864516782b7e6e33be2b2ec7743afaae8ba

💬 maflcko commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1606723428)
nit: Now that this is merged, it could say "in 27.0 and prior releases." Otherwise, on 29.x it will read as-if 28.0 had it missing.

💬 maflcko commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1606724711)
nit instead of duplicating the section from `doc/JSON-RPC-interface.md` here, it seems better to link/refer to it. Otherwise, if the section is updated, this may go stale or must be updated at the same time.

💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#issuecomment-2120390457)
@theuni https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1597108664 has been resolved by https://github.com/bitcoin/bitcoin/pull/30047/commits/d196442c015c4cbf490751a650ecb3aa29321442, if you have time for a second look/reACK

🤔 furszy reviewed a pull request: "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/28670#pullrequestreview-2066175552)
utACK 4a2df05e

💬 furszy commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1606753156)
nit: should use `fs::PathToString(path)`

💬 maflcko commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1606762456)
> PathToString

That is only for internal use. However, the result here will be returned on RPC, so the dev-notes apply:

```
- Use `fs::path::u8string()`/`fs::path::utf8string()` and `fs::u8path()` functions when converting path
to JSON strings, not `fs::PathToString` and `fs::PathFromString`

- *Rationale*: JSON strings are Unicode strings, not byte strings, and
RFC8259 requires JSON to be encoded as UTF-8.

🤔 furszy reviewed a pull request: "validation: improve performance of CheckBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/28339#pullrequestreview-2066200941)
ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8

💬 ajtowns commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#discussion_r1606763359)
`assert_equal` is helpful because when `a == 3` fails, it can be helpful to know what the value of `a` actually is without having to add debug code and rerun. If `a != 3` fails, though, you already know that `a == 3`, so this doesn't seem very helpful.

There are cases where you're comparing `a != b` and may not be sure which ones getting the wrong value where this could perhaps be helpful, but those seem pretty rare.

Concept -0 for me.