💬 sipa commented on pull request "optimization: Reduce cache lookups in CCoinsViewCache::FetchCoin":
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2187033512)
If you can benchmark pre-assumevalid IBD I think you'll see `FetchCoins` have a bigger impact than in `AssembleBlock`. It may need an actual `-reindex` though, I don't know if we have a benchmark with realistic workload.

💬 rkrux commented on pull request "wallet: notify when preset + automatic inputs exceed max weight":
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1651366991)
Oh yeah, makes sense. TY.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1651381337)
Unfortunately, that gives a new warning, because the `int` isn't the exact type:
```
../../src/util/pcp.cpp:258:41: warning: narrowing conversion of ‘recvsz’ from ‘int’ to ‘std::size_t’ {aka ‘long unsigned int’} [-Wnarrowing]
258 | if (check_packet({response, recvsz})) {
```
Could add a `static_cast` but i don't think it's much of an improvement in that case?

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1651401511)
It's an OS API, it's very unlikely to fail, so i don't want to add a specific log message for it. i'm fine with it just returning nullptr.

💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1651401792)
will change to that when taking it out of draft.

💬 sipa commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1651420690)
In that case you could use `check_packet(Span(response, recvsz))` (the `<uint8_t>` can be automatically deducted).

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2187135835)
Force push: rebased to get `CreateSock()` from #30202.

🤔 Mtiisf125 reviewed a pull request: "doc: clarify Cirrus self-hosted workers setup"
(https://github.com/bitcoin/bitcoin/pull/30314#pullrequestreview-2136604935)
Mri>..ta**>_isf

📝 mzumsande opened a pull request: "fuzz: improve utxo_snapshot target"
(https://github.com/bitcoin/bitcoin/pull/30329)
Add the possibility of giving more guidance to the creation of the metadata and/or coins, so that the fuzzer gets the chance
to reach more error conditions in ActivateSnapshot and sometimes successfully creates a valid snapshot.

This also changes the asserts for the success case that were outdated (after #29370) and only didn't result in a crash because the fuzzer wasn't able to reach this code before.

💬 achow101 commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1651551124)
Although `GetOSRand` is declared in `random.h`, it isn't actually used by anything outside of `random.cpp`, so it could remain inside the anonymous `namespace`.

💬 achow101 commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1651558600)
In cf04e6c90e1902be28a85ef181b8f1ee4b89e11d "random: make GetRand() support entire range (incl. max)"

Since this function is only used in a few places, would it make sense to just inline it where it is used?

💬 achow101 commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2187344321)
75bcc1057bbf7d8e0803df09780f4237c10c4264 says "rand_exp_delay" in the commit message, but the actual function is "rand_exp_duration".

💬 romanz commented on pull request "rest: don't copy data when sending binary response":
(https://github.com/bitcoin/bitcoin/pull/30321#issuecomment-2187360008)
Added https://github.com/bitcoin/bitcoin/commit/9c9375cf3b581c31f047b87a2c05507dc7b31804 with a few more fixes, following [the suggestion above](https://github.com/bitcoin/bitcoin/pull/30321#discussion_r1649705033).

💬 fjahr commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#issuecomment-2187466319)
Concept ACK

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2187485180)
Rebased to fix merge conflict

FYI: This PR is scheduled as the topic for the PR Review Club on July 3rd: https://bitcoincore.reviews/29775 You can see a preview of my suggested questions here: https://github.com/bitcoin-core-review-club/website/pull/765 Consider yourself invited!

👍 tdb3 approved a pull request: "rest: don't copy data when sending binary response"
(https://github.com/bitcoin/bitcoin/pull/30321#pullrequestreview-2136908546)
re ACK 9c9375cf3b581c31f047b87a2c05507dc7b31804

Re-ran the tests in https://github.com/bitcoin/bitcoin/pull/30321#pullrequestreview-2133999354

Also performed similar PR branch / master (538363738e9e30813cf3e76ca4f71c1aaff349e7) comparison tests with `tx`, `headers`, `blockhashbyheight`, and `getutxos` endpoints. All matched.
```
curl -v http://127.0.0.1:38332/rest/tx/7382c61e570cbc882a3d32b272318d353386981a475f58341fb4268971a6f1ec.bin
curl -v http://127.0.0.1:38332/rest/headers/0000005
...

⚠️ techy2 opened an issue: "error cross compiling linux X64 => 32 bit i686"
(https://github.com/bitcoin/bitcoin/issues/30330)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

While building depends for
make HOST=i686-pc-linux-gnu -j8

issues this warning
configure: WARNING: using cross tools not prefixed with host triplet
.... then fails .....
configure: error: in `/master/gitrepo/bitcoin/depends/work/build/i686-pc-linux-gnu/libevent2.1.12-stable-f9904577bfb':
configure: error: C compiler cannot create executables


### Expected behaviour

clean bui
...

✅ techy2 closed an issue: "automake script error building 32 bit depends libevent-2.1.12"
(https://github.com/bitcoin/bitcoin/issues/30311)

💬 techy2 commented on issue "automake script error building 32 bit depends libevent-2.1.12":
(https://github.com/bitcoin/bitcoin/issues/30311#issuecomment-2187569050)
Ok, will open new issue cross compiling X64 => i686 32 bit

👍 ryanofsky approved a pull request: "init: Add option for rpccookie permissions (replace 26088)"
(https://github.com/bitcoin/bitcoin/pull/28167#pullrequestreview-2136945831)
Code review ACK e4798038c00e787023b9dedc907966a08592d79f. Seems very clean now with the test simplification and without the win32 stuff.