💬 fanquake commented on pull request "build: Mark `x86_64-linux-gnu` release binaries as CET-enabled":
(https://github.com/bitcoin/bitcoin/pull/30685#issuecomment-2304400723)
Guix building 89f8e8e7f5828b1d06925e036c16eda050b05c81 on `aarch64` doesn't work:
```bash
time HOSTS="x86_64-linux-gnu" ./contrib/guix/guix-build
<snip>
CXXLD test/test_bitcoin
x86_64-linux-gnu-ld: /bitcoin/depends/x86_64-linux-gnu/lib/libevent.a(buffer.c.o): error: missing IBT and SHSTK properties
x86_64-linux-gnu-ld: /bitcoin/depends/x86_64-linux-gnu/lib/libevent.a(bufferevent.c.o): error: missing IBT and SHSTK properties
<snip>
x86_64-linux-gnu-ld: /bitcoin/depends/x86_64-linux-g
...

🤔 glozow reviewed a pull request: "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test"
(https://github.com/bitcoin/bitcoin/pull/30681#pullrequestreview-2254182247)
ACK 59ff17e5af4e382cbe16f183767beef1bdcd9131

💬 glozow commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#discussion_r1726826469)
typo nit e85f386c4b157b7d1ac16aface9bd2c614e62b46
```suggestion
* Enforce BIP94 timewarp attack mitigation. On testnet4 this also enforces
```

💬 hebasto commented on issue "Control-flow application capabilities for `x86_64-linux-gnu` release binaries":
(https://github.com/bitcoin/bitcoin/issues/30677#issuecomment-2304408493)
> Does the ELF flag control whether the feature will be enabled at runtime?

I don't think so. From https://docs.kernel.org/next/arch/x86/shstk.html:
> The kernel does not process these applications markers directly.

However, the linker will not place these markers if all object files are not properly instrumented. The `cet-report=error` [linker flag](https://sourceware.org/binutils/docs/ld.html) can make these checks visible.

💬 hebasto commented on pull request "build: Mark `x86_64-linux-gnu` release binaries as CET-enabled":
(https://github.com/bitcoin/bitcoin/pull/30685#discussion_r1726867429)
This is done to prevent disruption of internal flag checks.

💬 fanquake commented on pull request "build: Mark `x86_64-linux-gnu` release binaries as CET-enabled":
(https://github.com/bitcoin/bitcoin/pull/30685#discussion_r1726869045)
What disruption?

💬 l0rinc commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726874669)
Thanks for considering

💬 l0rinc commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726876602)
Haven't realized it was possible before as well, thanks for clearing that up

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1726878944)
The fuzz target is always setting `state.Invalid`, so I don't see where this coverage would come from?

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1726881353)
hmm, must have been referencing something else, nevermind

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726879771)
Resolved by removing `TrySanitizeHexNumber()` in latest force-push.

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726880784)
> (In the follow-up you can also adjust the error message to explain the new max-length check.

Done.

> Will use .value() either in next force-push

Resolved by using `FromUserHex()` instead.

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726879444)
> I think I would probably add base_blob::FromUserHex() function wrapping base_blob::FromHex().

Thanks for the suggestion, I've adopted this approach.

🤔 stickies-v reviewed a pull request: "node: reduce unsafe uint256S usage"
(https://github.com/bitcoin/bitcoin/pull/30569#pullrequestreview-2254228380)
Force-pushed (twice) to change the approach to address @achow101's, @ryanofsky's and @hodlinator's concerns about being too strict about user hex input for `-assumevalid` and `RANDOM_CTX_SEED`. Thanks again for sharing your concerns about breaking changes (and the detailed review), in hindsight I think this is indeed the better approach.

Specifically, these last two force-pushes:
- introduces a `uint256::FromUserHex()` to allow for lenient user input parsing and
does away with the `TrySanit
...

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726878931)
Thank you for summarizing your concerns @ryanofsky. I believe the latest force-push addresses all of them.

> It's good this change is making it an error to include trailing non-hex characters instead of silently ignoring them.
It's good this change is making it an error to specify hex strings that are too long.
It's probably ok this change makes it an error to include leading and trailing whitespace, instead of trimming it. Maybe that could be safer and prevent configuration bugs.

Releva
...

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726868348)
Resolved by removing `TrySanitizeHexNumber()` altogether (and empty strings are still allowed in `uint256::FromUserHex`, as per current behaviour on `master`)

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726858671)
Thanks, I've added these two test cases!

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726863840)
Oops, it wasn't, thanks for spotting. Reverted in latest force-push.

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726864665)
Taken both suggestions in latest force-push, thanks.

💬 marcofleon commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1726881676)
`i2p.cpp` is missing here