📝 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:

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:

utACK 232928b58a82e3f15307deba1ae921ae2960ccc8
(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)
(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
(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.
(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.
(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.
(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.
(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.
(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.
💬 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_r1627904057)
Good question. I'm not sure if it's really that common, but there was a wish to also test the utility function via a functional test (see comment https://github.com/bitcoin/bitcoin/pull/28312#issuecomment-1690367321) and it seemed to be a worthwhile idea to me.
(https://github.com/bitcoin/bitcoin/pull/28312#discussion_r1627904057)
Good question. I'm not sure if it's really that common, but there was a wish to also test the utility function via a functional test (see comment https://github.com/bitcoin/bitcoin/pull/28312#issuecomment-1690367321) and it seemed to be a worthwhile idea to me.
💬 theuni commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#issuecomment-2150205568)
@hebasto I think this is only ~half of what needs to be done here. Need to port #30137 as well, which should make this even simpler for CMake.
(https://github.com/bitcoin/bitcoin/pull/30095#issuecomment-2150205568)
@hebasto I think this is only ~half of what needs to be done here. Need to port #30137 as well, which should make this even simpler for CMake.
💬 theuni commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#issuecomment-2150209122)
Ah nm, I see, that's already done :)
(https://github.com/bitcoin/bitcoin/pull/30095#issuecomment-2150209122)
Ah nm, I see, that's already done :)
💬 alfonsoromanz commented on pull request "test: add validation for gettxout RPC response":
(https://github.com/bitcoin/bitcoin/pull/30226#issuecomment-2150252609)
> Why not just expanding the tests in `wallet_basic.py`?
I initially added the test to `rpc_blockchain.py` because it focuses on testing RPC commands from `src/rpc/blockchain.cpp`. However, I'm happy to expand the tests in `wallet_basic.py` if that's preferred.
(https://github.com/bitcoin/bitcoin/pull/30226#issuecomment-2150252609)
> Why not just expanding the tests in `wallet_basic.py`?
I initially added the test to `rpc_blockchain.py` because it focuses on testing RPC commands from `src/rpc/blockchain.cpp`. However, I'm happy to expand the tests in `wallet_basic.py` if that's preferred.
💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627952167)
Did you really push it? https://github.com/bitcoin/bitcoin/compare/a92e28e852f213c522df9858e240470299e4ecaa..471d27d5d740f82fd9f51a6e909a8ae3bdd75828 says no changes.
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627952167)
Did you really push it? https://github.com/bitcoin/bitcoin/compare/a92e28e852f213c522df9858e240470299e4ecaa..471d27d5d740f82fd9f51a6e909a8ae3bdd75828 says no changes.
💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627952472)
Ok, fair enough.
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627952472)
Ok, fair enough.
💬 brunoerg commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1627954290)
Why is it calling `Connect` without specifying `to`?
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1627954290)
Why is it calling `Connect` without specifying `to`?
💬 fanquake 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-2150331121)
https://github.com/bitcoin/bitcoin/pull/30232/checks?check_run_id=25845075037:
```bash
A new circular dependency in the form of "kernel/mempool_options -> policy/policy -> kernel/mempool_options" appears to have been introduced.
```
(https://github.com/bitcoin/bitcoin/pull/30232#issuecomment-2150331121)
https://github.com/bitcoin/bitcoin/pull/30232/checks?check_run_id=25845075037:
```bash
A new circular dependency in the form of "kernel/mempool_options -> policy/policy -> kernel/mempool_options" appears to have been introduced.
```
💬 ryanofsky commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2150332074)
The idea of saving heights is interesting to me because wallet code assumes it know block heights many of places, but it doesn't actually store heights anywhere. So for example, if there wallet stored a mapping of block hashes to heights (or other block information) it might be useful for allowing the wallet to work in an offline mode or letting the wallet CLI tool have more capabilities. This is all pretty abstract though, so I'm not suggesting something specific.
> Perhaps an easy solution
...
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2150332074)
The idea of saving heights is interesting to me because wallet code assumes it know block heights many of places, but it doesn't actually store heights anywhere. So for example, if there wallet stored a mapping of block hashes to heights (or other block information) it might be useful for allowing the wallet to work in an offline mode or letting the wallet CLI tool have more capabilities. This is all pretty abstract though, so I'm not suggesting something specific.
> Perhaps an easy solution
...
💬 fanquake commented on pull request "guix: bump time-machine to f0bb724211872cd6158fce6162e0b8c73efed126":
(https://github.com/bitcoin/bitcoin/pull/30231#issuecomment-2150426967)
Guix Build (aarch64):
```bash
766de337912412943b38f6f0dd100bafaa858bf2ad43bbc56de9de86b59f99c9 guix-build-2599655c1fb8/output/aarch64-linux-gnu/SHA256SUMS.part
a426d7189067919d418e164b5770c580bc6e94c3aa5320b2980c65762055ec92 guix-build-2599655c1fb8/output/aarch64-linux-gnu/bitcoin-2599655c1fb8-aarch64-linux-gnu-debug.tar.gz
c70f267474be9121d70e217c966c6c359ff12efb369a98e787dabc63221e2a47 guix-build-2599655c1fb8/output/aarch64-linux-gnu/bitcoin-2599655c1fb8-aarch64-linux-gnu.tar.gz
75038a
...
(https://github.com/bitcoin/bitcoin/pull/30231#issuecomment-2150426967)
Guix Build (aarch64):
```bash
766de337912412943b38f6f0dd100bafaa858bf2ad43bbc56de9de86b59f99c9 guix-build-2599655c1fb8/output/aarch64-linux-gnu/SHA256SUMS.part
a426d7189067919d418e164b5770c580bc6e94c3aa5320b2980c65762055ec92 guix-build-2599655c1fb8/output/aarch64-linux-gnu/bitcoin-2599655c1fb8-aarch64-linux-gnu-debug.tar.gz
c70f267474be9121d70e217c966c6c359ff12efb369a98e787dabc63221e2a47 guix-build-2599655c1fb8/output/aarch64-linux-gnu/bitcoin-2599655c1fb8-aarch64-linux-gnu.tar.gz
75038a
...
💬 instagibbs commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2150437453)
ACK e4ecb8217ada3dae1c1645a8d0a12e14b0f935da
verified fuzz target coverage and sensible invariant checks are being enforced
(https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2150437453)
ACK e4ecb8217ada3dae1c1645a8d0a12e14b0f935da
verified fuzz target coverage and sensible invariant checks are being enforced