💬 TheCharlatan commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2426025067)
I'm a bit confused by this change too. Previously we wrote the reindexing flag before starting to connect blocks (through the call to ABC), but now it seems like we are connecting blocks first, and only then writing the reindexing flag again?
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2426025067)
I'm a bit confused by this change too. Previously we wrote the reindexing flag before starting to connect blocks (through the call to ABC), but now it seems like we are connecting blocks first, and only then writing the reindexing flag again?
💬 hebasto commented on pull request "Update `minisketch` subtree and switch to its build script":
(https://github.com/bitcoin/bitcoin/pull/32856#discussion_r2426034017)
As stated in the comment above, this is done intentionally to avoid cluttering the CMake cache with options originating from a subtree, which should never be modified by the user.
(https://github.com/bitcoin/bitcoin/pull/32856#discussion_r2426034017)
As stated in the comment above, this is done intentionally to avoid cluttering the CMake cache with options originating from a subtree, which should never be modified by the user.
💬 TheCharlatan commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3397101094)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3397101094)
Concept ACK
💬 hebasto commented on pull request "Update `minisketch` subtree and switch to its build script":
(https://github.com/bitcoin/bitcoin/pull/32856#discussion_r2426056415)
I agree that using `block()` would be more appropriate. For now, this code follows the approach used here: https://github.com/bitcoin/bitcoin/blob/563747971be492a8da772fb2f3e45dd5ee05da86/src/CMakeLists.txt#L23-L24 for consistency.
(https://github.com/bitcoin/bitcoin/pull/32856#discussion_r2426056415)
I agree that using `block()` would be more appropriate. For now, this code follows the approach used here: https://github.com/bitcoin/bitcoin/blob/563747971be492a8da772fb2f3e45dd5ee05da86/src/CMakeLists.txt#L23-L24 for consistency.
🚀 fanquake merged a pull request: "[28.x] 28.3rc2"
(https://github.com/bitcoin/bitcoin/pull/33557)
(https://github.com/bitcoin/bitcoin/pull/33557)
💬 hebasto commented on pull request "Update `minisketch` subtree and switch to its build script":
(https://github.com/bitcoin/bitcoin/pull/32856#issuecomment-3397150568)
Refreshed and rebased. The PR description has been updated.
(https://github.com/bitcoin/bitcoin/pull/32856#issuecomment-3397150568)
Refreshed and rebased. The PR description has been updated.
💬 caesrcd commented on pull request "contrib: Add bash completion for new bitcoin command":
(https://github.com/bitcoin/bitcoin/pull/33385#issuecomment-3397213980)
@prusnak It will complete the arguments of `bitcoin` itself and the subarguments of commands that already have their own completion.
To cover `bitcoin test`, a completion script for the `test_bitcoin` command would need to be created.
(https://github.com/bitcoin/bitcoin/pull/33385#issuecomment-3397213980)
@prusnak It will complete the arguments of `bitcoin` itself and the subarguments of commands that already have their own completion.
To cover `bitcoin test`, a completion script for the `test_bitcoin` command would need to be created.
🤔 janb84 reviewed a pull request: "refactor: Construct g_verify_flag_names on first use"
(https://github.com/bitcoin/bitcoin/pull/33600#pullrequestreview-3331335695)
lgtm ACK faa9d10c84bc6b465cbca266468990cc716b4300
PR refactors `g_verify_flag_names` init to use the "Construct on first use" Idiom to prevent a "static initialization order fiasco" error. This currently gives some false positives, imho a valid change to remove the false positives and to prevent a "static initialization order fiasco" type of error in the future.
- code review ✅
- build en tests run ✅
(https://github.com/bitcoin/bitcoin/pull/33600#pullrequestreview-3331335695)
lgtm ACK faa9d10c84bc6b465cbca266468990cc716b4300
PR refactors `g_verify_flag_names` init to use the "Construct on first use" Idiom to prevent a "static initialization order fiasco" error. This currently gives some false positives, imho a valid change to remove the false positives and to prevent a "static initialization order fiasco" type of error in the future.
- code review ✅
- build en tests run ✅
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2426186301)
Done thanks
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2426186301)
Done thanks
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2426187161)
This test is redundant removed.
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2426187161)
This test is redundant removed.
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2426187649)
This test is also redundant removed.
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2426187649)
This test is also redundant removed.
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2426187998)
fixed
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2426187998)
fixed
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3397331200)
reACK 023cd5a5469ad61205bf7bb1135895f2b4a20ea9
`git range-diff master 7421e58dee402ee8b5b58a18684953a89ad350d7 023cd5a5469ad61205bf7bb1135895f2b4a20ea9`
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3397331200)
reACK 023cd5a5469ad61205bf7bb1135895f2b4a20ea9
`git range-diff master 7421e58dee402ee8b5b58a18684953a89ad350d7 023cd5a5469ad61205bf7bb1135895f2b4a20ea9`
👍 willcl-ark approved a pull request: "doc: archive release notes for v30.0"
(https://github.com/bitcoin/bitcoin/pull/33601#pullrequestreview-3331448273)
ACK 8d6e49158e3a1b3215484b5d139c5d8a3fffc4c9
This archive is OK.
I do notice that we seem to have zero `Build System` notes though, which is slightly sad. New binaries (which could have been in this section) have their own section, but perhaps we miss detailing capnproto dependency.
(https://github.com/bitcoin/bitcoin/pull/33601#pullrequestreview-3331448273)
ACK 8d6e49158e3a1b3215484b5d139c5d8a3fffc4c9
This archive is OK.
I do notice that we seem to have zero `Build System` notes though, which is slightly sad. New binaries (which could have been in this section) have their own section, but perhaps we miss detailing capnproto dependency.
💬 instagibbs commented on issue "test: intermittent issue in p2p_1p1c_network.py":
(https://github.com/bitcoin/bitcoin/issues/33318#issuecomment-3397358091)
I don't think the pre-received child peer(s) make sense anymore, as we intentionally zero these out of the orphanage when all the announcing peers disconnect.
If we want to keep this part just to exercise that state space, we could make sure that we wait until all the node's orphan slots are empty via rpc before continuing with the ultimate submitpackage?
(https://github.com/bitcoin/bitcoin/issues/33318#issuecomment-3397358091)
I don't think the pre-received child peer(s) make sense anymore, as we intentionally zero these out of the orphanage when all the announcing peers disconnect.
If we want to keep this part just to exercise that state space, we could make sure that we wait until all the node's orphan slots are empty via rpc before continuing with the ultimate submitpackage?
🚀 fanquake merged a pull request: "doc: archive release notes for v30.0"
(https://github.com/bitcoin/bitcoin/pull/33601)
(https://github.com/bitcoin/bitcoin/pull/33601)
💬 fanquake commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3397400978)
Rel notes archived in #33601 & GH release: https://github.com/bitcoin/bitcoin/releases/tag/v30.0.
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3397400978)
Rel notes archived in #33601 & GH release: https://github.com/bitcoin/bitcoin/releases/tag/v30.0.
✅ fanquake closed an issue: "Release Schedule for 30.0"
(https://github.com/bitcoin/bitcoin/issues/32275)
(https://github.com/bitcoin/bitcoin/issues/32275)
🚀 fanquake merged a pull request: "ci: Use native platform for win-cross task"
(https://github.com/bitcoin/bitcoin/pull/33558)
(https://github.com/bitcoin/bitcoin/pull/33558)
💬 fanquake commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3397429669)
Kicked all the unrelated failures.
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3397429669)
Kicked all the unrelated failures.