💬 Earnestly commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-2155041327)
> since they were disabled by satoshi.

Hmm.

💬 m3dwards commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#discussion_r1631366827)
The comment? I'm not sure

💬 m3dwards commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2155048864)
Force pushed but only changed the commit description as it was a bad example with an extra space

💬 maflcko commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2155053946)
reACK 3d976000afccac4e89931496f23cfe8593daeb75

💬 maflcko commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#discussion_r1631373966)
You can test by removing it and pushing, then observing the colour of the `lint` task after 3 minutes.

💬 glozow commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-2155066959)
concept ACK + lgtm 659663af32

Seems like this is a remaining mention of `CTransaction::nVersion`?
https://github.com/bitcoin/bitcoin/blob/659663af32f02c570e334e8f375fd5f5737258d7/src/compressor.h#L57-L60

Also noting that #29496 unfortunately conflicts and I would advocate for merging that first... but I'm happy to re-review

📝 maflcko opened a pull request: "refactor: Add explicit cast to expected_last_page to silence fuzz ISan"
(https://github.com/bitcoin/bitcoin/pull/30248)
Fixes #30247

I don't think this implicit cast can lead to any bugs, so make it explicit to silence the fuzz integer sanitizer.

Can be tested with:

```
FUZZ=wallet_bdb_parser UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./src/test/fuzz/fuzz /tmp/1376869be72eebcc87fe737020add634b1a29533
```

After downloading the raw fuzz input from https://github.com/bitcoin-core/qa-assets/blob/24c507b3ea6263e6b121fb8dced
...

👍 dergoegge approved a pull request: "Safegcd-based modular inverses in MuHash3072"
(https://github.com/bitcoin/bitcoin/pull/21590#pullrequestreview-2104888365)
tACK 030c9edf5b12033207da2bc0735f97840dc88056

I differentially fuzzed the muhash implementation we have on master against the version in this PR. Given the coverage reached and assuming that the implementation on master is correct, I am confident that the muhash implementation in this PR is also correct. I tested across multiple compilers and optimization levels i.e. the following tuples: `(master clang -O2, pr clang -O2)`, `(master clang -O2, pr gcc -O2)`, `(master clang-O2, pr gcc -O0)` (al
...

💬 iw4p commented on pull request "test: switch from curl to urllib for HTTP requests":
(https://github.com/bitcoin/bitcoin/pull/29970#issuecomment-2155110688)
> One drawback of this change is that a user doesn't see download progress per file any more (relevant if e.g. servers are slow or the connection is bad for any other reason; for me this currently takes several minutes per file :/) and hence is left in the dark whether there is still activity, so Concept ~0.

I appreciate your feedback. While it's true that switching from `curl` to `urllib` removes the per-file download progress visibility, this change offers significant improvements in code c
...

🤔 instagibbs requested changes to a pull request: "util: add BitSet"
(https://github.com/bitcoin/bitcoin/pull/30160#pullrequestreview-2104783238)
c99063e902c810dde1742696b3140610120d391c seems to need `swap` fuzz coverage

rest of comments are nits

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631369553)
```Suggestion
/* First / Last */
```

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631335816)
```Suggestion
/** Assign from a list of positional values. */
```

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631393064)
lacks coverage?

my local coverage generation is kinda screwed up, but fuzz coverage should be computed :pray:

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631406724)
maybe just use conditional `Set`?

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631403529)
Set(i)

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631406944)
maybe just use conditional `Set`?

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631408015)
stick with `pos` for arg names?

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631403413)
Set(i)?

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631411035)
just check `!None()`?

💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1631431050)
Thanks for pointing this out. The background validation does not seem to finish in this case.

I am now testing with a new node `n3` (in my local copy) to avoid the manual rollback to `START_HEIGHT` ([L207](https://github.com/bitcoin/bitcoin/pull/29996/files#diff-8eaa6b77b4e4f242deb67950425cb6b6b7de5370c56a1e8fa7e3a12e95df0a9dR207)). Additionally, `sync_blocks()` was throwing a timeout when reusing n2 for this test (not sure why).

Here's the new approach I'm trying:

- The new node start
...