Results 251 comments of l0rinc
trafficstars

Hey @fanquake, thanks for your input. How do you suggest we unify this in the codebase, since it's not used consistently currently? This change could level the field before branching...

Thanks @Empact, I think we can ignore the internal consistency argument, and the change would still make the build more maintainable - do you agree?

@theuni, I've split it out the crypto package to https://github.com/bitcoin/bitcoin/pull/29834, thanks for checking it!

Rebased after the https://github.com/bitcoin/bitcoin/pull/29834 merge, please let me know what to change or verify here.

Rebased and adjusted to the CMake build following @hebasto's `Needs CMake port` label.

> @l0rinc can you rebase this? Rebased, checked for leftover `MAC_OSX` (found none)

> TIL: one can compare `optional` against a value without `.value()` (C++17 addition). Yeah, but I don't think that's used here

> It is used in your change from BOOST_CHECK_EQUAL(uint256::FromUserHex("").value(), uint256::ZERO); Details I'm not sure it's related, commenting out: ```diff -inline std::ostream& operator

> 2\. but `BOOST_CHECK` is deprecated I don't [see the deprecation](https://live.boost.org/doc/libs/1_86_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref/assertion_boost_level.html), I think it's still useful for truthy checks (e.g. empty optionals). > one can compare optional against a value...

Thanks for the thorough review @hodlinator and @stickies-v, I pushed another batch (first an empty rebase), followed by https://github.com/bitcoin/bitcoin/compare/c6f9add458bb8e142d8ccfe5c661ec56143e3f31..6fa4c92e4c9bf5ea90e25d41b525c88f9c2d834d Edit: > we're not increasing FromUserHex fuzz coverage imo The line...