💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627796687)
I've added an Assume that `bits` is in range.

🤔 maflcko reviewed a pull request: "Several randomness improvements"
(https://github.com/bitcoin/bitcoin/pull/29625#pullrequestreview-2098835816)
e7676b6c52672f557f0e2b036d5e877393f1ce46 🎩

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: e7676b6c52672f557f0e2b036d5e877393
...

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627567196)
nit in 0b92cd2ae4a317be4fe0dfd208017a7d05d1eb2a: Not sure I understand this concept. Is there anything that is not already enforced normally in C++ that would be enforced by this concept?

For example, `static_cast` in the `Impl()` method, which enforces this concept should already check for inheritance:

```
<source>:17:40: error: static_cast from 'Mixin<FRC> *' to 'FRC *', which are not related by inheritance, is not allowed
```

Conversely, if the class wasn't derived from `RandomMixi
...

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627736240)
unrelated q in 6370a51aac06a430ec53f29fe03f5bbd08371b34: Can you explain where one finds this "copy()" method?

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627554476)
nit in 0b92cd2ae4a317be4fe0dfd208017a7d05d1eb2a:

Looks like this commit is doing several things at once. It may be easier to review if move-only stuff (like moving rand256 and randbytes function bodies to the header file, and removing the explicit template instantiation) was done in a separate prepare commit?

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1627806235)
perfect thanks

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627807244)
nit in c099cd2e3c20fc416e3f7fc1da4798bb8042af1c: Could increase the severity to `LogError()` while touching this?

📝 marcofleon opened a pull request: "fuzz: add I2P harness"
(https://github.com/bitcoin/bitcoin/pull/30230)
Addresses https://github.com/bitcoin/bitcoin/issues/28803.

The SAM protocol for interacting with I2P requires some specifc inputs so it's best to use a dictionary when running this harness.

<details>
<summary>I2P dict</summary>

```
"HELLO VERSION"
"HELLO REPLY RESULT=OK VERSION="
"HELLO REPLY RESULT=NOVERSION"
"HELLO REPLY RESULT=I2P_ERROR"
"SESSION CREATE"
"SESSION STATUS RESULT=OK DESTINATION="
"SESSION STATUS RESULT=DUPLICATED_ID"
"SESSION STATUS RESULT=DUPLICATED_DEST"
"SE
...

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627821690)
Well in my view, concepts are really a kind of compiler-enforced documentation. Their primary purpose is giving a readable compiler error when template instantiation ought to be (and often, would be) failing anyway. In this case, someone implementing a new RNG but say missing `rand64`, would instead of getting an error in the instantiation of `RandomMixin` get an error message that references `RandomNumberGenerator`, which then explicitly lists what one is required to do to make it work.

🤔 furszy reviewed a pull request: "wallet: Automatically repair corrupted metadata with doubled derivation path"
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2099269310)
ACK 9db2d8b

💬 furszy commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r1627825496)
> I've made `m_batch` public so that we can just pass the `WalletBatch` but still use the underlying batch object. However I'm not sure if that's something we want to do in the long term, but I think it should be okay.

Yeah.. I'm not happy reading all the the `batch.m_batch` usages neither but np.

📝 fanquake opened a pull request: "guix: bump time-machine to f0bb724211872cd6158fce6162e0b8c73efed126"
(https://github.com/bitcoin/bitcoin/pull/30231)
Includes:
LLVM 18.1.x (#30201)
GCC 13.x (#29881)

git-minimal 2.41.0 -> 2.45.1
Kernel Headers 6.1.80 -> 6.1.92
moreutils 0.68 -> 0.69

Commits like https://git.savannah.gnu.org/cgit/guix.git/commit/?id=7b0f145802f0c2c785014293d748721678fef824, which should improve the bootstrap situation (#30042). This can somewhat be visualised by comparing the (simplified) dependencies of guix itself, between the two time-machines.

Master:
![master_2](https://github.com/bitcoin/bitcoin/assets/86373
...

👍 theuni approved a pull request: "build: no-longer allow GCC-10 in C++20 check"
(https://github.com/bitcoin/bitcoin/pull/30228#pullrequestreview-2099370106)
utACK 232928b58a82e3f15307deba1ae921ae2960ccc8

📝 luke-jr opened a pull request: "refactor: policy: Pass kernel::MemPoolOptions to IsStandard[Tx] rather than long list of individual options"
(https://github.com/bitcoin/bitcoin/pull/30232)

💬 luke-jr commented on pull request "refactor: policy: Pass kernel::MemPoolOptions to IsStandard[Tx] rather than long list of individual options":
(https://github.com/bitcoin/bitcoin/pull/30232#issuecomment-2150152980)
@kristapsk @jonatack @achow101

💬 theStack commented on pull request "test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16":
(https://github.com/bitcoin/bitcoin/pull/28312#issuecomment-2150153924)
Rebased on master.

💬 hebasto commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#issuecomment-2150158568)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/214.

💬 hebasto commented on pull request "build: Remove `--enable-threadlocal`":
(https://github.com/bitcoin/bitcoin/pull/30137#issuecomment-2150163721)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/214 and https://github.com/hebasto/bitcoin/pull/220.

💬 theStack commented on pull request "test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16":
(https://github.com/bitcoin/bitcoin/pull/28312#discussion_r1627897054)
Such a test case was already introduced in the recently merged PR #28307 (commit 9be6065cc03f2408f290a332b203eef9c9cebf24). Note that this PR is primarily focused on the `keys_to_multisig_script` helper in the test framework and only uses the `createmultisig` RPC to verify it.

💬 hebasto commented on issue "Rethink thread_local (take 2)":
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2150170440)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/214 and https://github.com/hebasto/bitcoin/pull/220.