📝 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.

💬 ryanofsky commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1651743320)
In commit "init: add option for rpccookie permissions" (7ad5349a2ec04399c99c8cbf71c8d3b4627132f8)

Could move this variable inside the if statement since it's not used afterwards.

🚀 ryanofsky merged a pull request: "Introduce Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30200)

👍 ryanofsky approved a pull request: "Support self-hosted Cirrus workers on forks"
(https://github.com/bitcoin/bitcoin/pull/29274#pullrequestreview-2136964998)
Code review ACK b80a573bb42e10f1ec550c79133317c1c0b021c6, but I would think about dropping the second commit ("ci: skip Github CI on branch pushes for forks" (b9fdd0dc75b5b4944dffc700b0391b38465f754a)) and moving it to another PR, since it is a little confusing and maybe could use more discussion.

Otherwise looks very good, I just left some suggestions to try to improve some comments.

💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1651755883)
In commit "ci: forks can opt-out of CI branch push (Cirrus only)" (9e30be769dfc06bf122db1ea2042a68c597509a0)

Would suggest expanding comment: "Allow forks to specify NO_BRANCH=true and skip CI runs when a branch is pushed, but still run CI when a PR is created."

Otherwise it is not clear what the NO_BRANCH variable has to do with forks or why the CIRRUS_PR condition is there.

💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1651756679)
In commit "ci: forks can opt-out of CI branch push (Cirrus only)" (9e30be769dfc06bf122db1ea2042a68c597509a0)

Would it be possible as a followup to delete this skip condition and just specify NO_BRANCH=true in the bitcoin GUI cirrus environment?

💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1651761313)
In commit "ci: forks can opt-out of CI branch push (Cirrus only)" (9e30be769dfc06bf122db1ea2042a68c597509a0)

How actually do you set a custom in variable in cirrus for a fork? Maybe this could suggest a way to do it or link to the cirrus docs.

Also should this say NO_BRANCH=true instead of NO_BRANCH=1 since code below appears to compare against "true" specifically?

💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1651782082)
In commit "ci: skip arm if no worker is configured" (dc98f4c1ab181c79352f9a14351efe87a58f203d)

Questions / suggestions:

- It looks like code below is specifically checking $NO_ARM == "true", so should this say NO_ARM=true instead of NO_ARM=1?

- It might be good to join this paragraph with the previous paragraph so the arm information is in one place.

- It might be good to say that forks either need to provide an arm worker or set NO_ARM=true, otherwise ARM tasks will appear pending f
...

💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1651793332)
In commit "ci: test-each-commit merge base optional" (b80a573bb42e10f1ec550c79133317c1c0b021c6)

Might be useful to expand commit message to explain why this change is needed. Maybe say:

- The ci "test-each-commit" job fetches the PR branch being tested with a depth of (# of commits in PR + 2), and then tries to run tests on commits after the most recent merge commit.

When a PR is opened against a bitcoin core branch, a merge commit is always guaranteed to exist within the fetch depth
...