💬 sipsorcery commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2041068780)
> > This is from an attempt to compile (no linking involved) the single blockfilter_tests.cpp file.
>
> Does it compile on the master branch for you?

No.

I only checked that after posting the compiler error message. The msvc compiler error isn't related to this PR, it's on master as well.

💬 tdb3 commented on pull request "p2p: gives seednode priority over dnsseed if both are provided":
(https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-2041082464)
re-ACK for 2842e51a246b162a586941184b7694f187d7aee7

The code adjustments made to bound scope and increase declaration/usage proximity look great to me (cleaner, more readable).

Built and ran all functionals (all passed).

Cleared datadir (with the exception of bitcoin.conf, chainstate, and blocks). Performed >1000 block download on signet to a reachable public node specified as a seednode. dnsseeds seemed to be avoided as expected.

```
2024-04-06T13:15:52Z dnsseed thread start
202
...

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2041086383)
> The msvc compiler error isn't related to this PR, it's on master as well.

Could you please open a separate issue then?

FWIW, I have no compile errors for the master branch @ b5d21182e5a66110ce2796c2c99da39c8ebf0d72 using VS Community 2022 17.9.5. For example, the `test_bitcoin.exe` compiles for both `Debug` and `Release` configurations:
```
> msbuild --version
MSBuild version 17.9.8+b34f75857 for .NET Framework
17.9.8.16306
> py -3 build_msvc\msvc-autogen.py
> msbuild build_msvc\bi
...

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1554621632)
`... + 32` is consistent with other `CKey::Set()` calls in our codebase:

https://github.com/bitcoin/bitcoin/blob/b5d21182e5a66110ce2796c2c99da39c8ebf0d72/src/bench/bip324_ecdh.cpp#L30

https://github.com/bitcoin/bitcoin/blob/b5d21182e5a66110ce2796c2c99da39c8ebf0d72/src/bench/ellswift.cpp#L20

https://github.com/bitcoin/bitcoin/blob/b5d21182e5a66110ce2796c2c99da39c8ebf0d72/src/key.cpp#L330

https://github.com/bitcoin/bitcoin/blob/b5d21182e5a66110ce2796c2c99da39c8ebf0d72/src/key.cpp#L393
...

📝 hebasto opened a pull request: "refactor, bench, fuzz: Drop unneeded `UCharCast` calls"
(https://github.com/bitcoin/bitcoin/pull/29820)
The `CKey::Set()` template function handles `std::byte` just fine: https://github.com/bitcoin/bitcoin/blob/b5d21182e5a66110ce2796c2c99da39c8ebf0d72/src/key.h#L105

Noticed in https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1546288181.

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1554623656)
> If there is no need for it, it may be better to remove it.

Please see https://github.com/bitcoin/bitcoin/pull/29820.

💬 fanquake commented on pull request "ci: remove --with-asm=no (secp256k1) from MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/29742#issuecomment-2041113829)
Have run this successfully on all 4 MSAN jobs locally.

📝 hebasto opened a pull request: "fuzz: Some `test/fuzz/test_runner.py` improvements"
(https://github.com/bitcoin/bitcoin/pull/29821)
These changes are split from https://github.com/bitcoin/bitcoin/pull/29774 and can be beneficial on their own.

The new `BITCOINFUZZ` environment variable complements the already existing set of variables used by tests: https://github.com/bitcoin/bitcoin/blob/b5d21182e5a66110ce2796c2c99da39c8ebf0d72/test/functional/test_framework/test_framework.py#L238-L243

💬 hebasto commented on pull request "fuzz: Some `test/fuzz/test_runner.py` improvements":
(https://github.com/bitcoin/bitcoin/pull/29821#issuecomment-2041115220)
cc @maflcko @dergoegge

👍 BrandonOdiwuor approved a pull request: "assumeutxo: Fix -reindex before snapshot was validated"
(https://github.com/bitcoin/bitcoin/pull/29726#pullrequestreview-1984575158)
re-ACK b7ba60f81a33db876f88b5f9af1e5025d679b5be

🤔 BrandonOdiwuor reviewed a pull request: "test: Refactor fee calculation to remove satoshi_round function"
(https://github.com/bitcoin/bitcoin/pull/29566#pullrequestreview-1984578430)
Concept ACK moving from `float` to `Decimal`

📝 theStack opened a pull request: "test: remove immediate tx relay workaround in wallet_groups.py"
(https://github.com/bitcoin/bitcoin/pull/29822)
Reverts commit ab4efad51b9ba276ffeb6871931e13772493f7cc (PR #26970). This workaround is not needed anymore, as since #27114 the test sets the noban permission for both in- and outbound connections via the `noban_tx_relay` setting, and we don't have to rely on this topology hack anymore. See commit c985eb854cc86deb747caea5283c17cf51b6a983 (kudos to brunoerg!).

Can be tested by executing `$ time ./test/functional/wallet_groups.py` both on master and PR and verifying that the execution time is r
...

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1554638887)
Thanks! Removed.

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2041136905)
The branch has been reworked to address @maflcko's comments.

> ... but bool return value + exceptions doesn't make sense, does it?

Whether [`std::istream::read()`](https://en.cppreference.com/w/cpp/io/basic_istream/read) throw or not depends on how [`exceptions()`](https://en.cppreference.com/w/cpp/io/basic_ios/exceptions) is set. The suggested code does not use exceptions.

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1554639703)
Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2041136905).

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1554639779)
[Reworked](https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2041136905).

💬 hebasto commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1554639810)
[Reworked](https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2041136905).

💬 fanquake commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1554640585)
I don't really think we want a reversion here (or in addition), and if we were reverting shouldn't the `__GNUC__` case go away?

In any case, I'd rather we just not define this for MSVC.

💬 fjahr commented on pull request "[DO NOT MERGE] testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2041139129)
Pushed some improvements and addressed some feedback. I am experimenting with some of the proposals from the mailing list and so I added Andres Poelstra's suggested difficulty adjustment with 6h/1M from here: https://groups.google.com/g/bitcoindev/c/9bL00vRj7OU/m/kFPaQCzmBwAJ

> Probably we should support tracking both testnet3 and the new testnet4 for some time. Making the new code conditional on a different chain param that's only set for testnet4 would probably be the easiest way of accompl
...

💬 itornaza commented on pull request "test: Bump timeouts in feature_index_prune and wallet_importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2041145104)
@cbergqvist I re-run today the `test/functional/test_runner.py --jobs=16 --extended` two times on the **PR branch** after cleaning up everything and building the branch again. I include the log files for each of the errors. I will happily assist you witht this since my current goal is to build up confidence in running the test with confidence!

## First run

```
wallet_importdescriptors.py --descriptors | ✖ Failed | 139 s
wallet_transactiontime_rescan.py --legacy-wallet
...