💬 pinheadmz commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3028164813)
@achow101 I started working on calling `WalletContext.scheduler` directly from `walletpassphrase()` but there is one nice extra feature we get from using `RPCRunLater` and `RPCTimerBase` which is that timers are mapped by name and can be removed by name if, for example, the user calls the RPC again with a different timeout value. This might result in a change of behavior where the shorter timer will still lock the wallet, and the longer timer will sit in the queue and eventually do nothing. Curr
...
(https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3028164813)
@achow101 I started working on calling `WalletContext.scheduler` directly from `walletpassphrase()` but there is one nice extra feature we get from using `RPCRunLater` and `RPCTimerBase` which is that timers are mapped by name and can be removed by name if, for example, the user calls the RPC again with a different timeout value. This might result in a change of behavior where the shorter timer will still lock the wallet, and the longer timer will sit in the queue and eventually do nothing. Curr
...
💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#discussion_r2180262495)
actually ended up being pretty easy to remove for a time-based timelock
```
diff --git a/test/functional/mempool_reorg.py b/test/functional/mempool_reorg.py
index 3906117e8b..0f6124f9ab 100755
--- a/test/functional/mempool_reorg.py
+++ b/test/functional/mempool_reorg.py
@@ -28,5 +28,6 @@ from test_framework.blocktools import (
# Number of blocks to create in temporary blockchain branch for reorg testing
-FORK_LENGTH = 10
+# Needs to be long enough to allow MTP to move arbitrarily
...
(https://github.com/bitcoin/bitcoin/pull/32587#discussion_r2180262495)
actually ended up being pretty easy to remove for a time-based timelock
```
diff --git a/test/functional/mempool_reorg.py b/test/functional/mempool_reorg.py
index 3906117e8b..0f6124f9ab 100755
--- a/test/functional/mempool_reorg.py
+++ b/test/functional/mempool_reorg.py
@@ -28,5 +28,6 @@ from test_framework.blocktools import (
# Number of blocks to create in temporary blockchain branch for reorg testing
-FORK_LENGTH = 10
+# Needs to be long enough to allow MTP to move arbitrarily
...
🤔 ryanofsky reviewed a pull request: "cmake: Move internal binaries from bin/ to libexec/"
(https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-2979400065)
Thanks for the reviews!
Updated d253fa9ebe4305118417a02aa72cc06810662ac6 -> 705791cd436f237fe9bbac2cf52d63ab4b2a41c7 ([`pr/libexec.6`](https://github.com/ryanofsky/bitcoin/commits/pr/libexec.6) -> [`pr/libexec.7`](https://github.com/ryanofsky/bitcoin/commits/pr/libexec.7), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/libexec.6..pr/libexec.7)) just fixing typos pointed out in reviews.
---
re: TheCharlatan https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-2807539249
...
(https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-2979400065)
Thanks for the reviews!
Updated d253fa9ebe4305118417a02aa72cc06810662ac6 -> 705791cd436f237fe9bbac2cf52d63ab4b2a41c7 ([`pr/libexec.6`](https://github.com/ryanofsky/bitcoin/commits/pr/libexec.6) -> [`pr/libexec.7`](https://github.com/ryanofsky/bitcoin/commits/pr/libexec.7), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/libexec.6..pr/libexec.7)) just fixing typos pointed out in reviews.
---
re: TheCharlatan https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-2807539249
...
💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2180213722)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2124090430
Thanks, fixed
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2180213722)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2124090430
Thanks, fixed
💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2180213943)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2120285755
Thanks, fixed
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2180213943)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2120285755
Thanks, fixed
💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2180213170)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2127330034
Rewrote the PR description to hopefully make this clearer, but this change is just adding a missing install rule. If you don't go out of your way to enable the BUILD_UTIL_CHAINSTATE option, the binary won't be installed, but if you do enable it, it will be installed. This is consistent with how we treat other build options like BUILD_GUI_TESTS with test_bitcoin-qt and BUILD_BENCH with bench_bitcoin.
but it doesn't
...
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2180213170)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2127330034
Rewrote the PR description to hopefully make this clearer, but this change is just adding a missing install rule. If you don't go out of your way to enable the BUILD_UTIL_CHAINSTATE option, the binary won't be installed, but if you do enable it, it will be installed. This is consistent with how we treat other build options like BUILD_GUI_TESTS with test_bitcoin-qt and BUILD_BENCH with bench_bitcoin.
but it doesn't
...
💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2180297022)
I'm aware. "Where" refers to the scripts. This is in opposition to the current sigops counting which, as you know, counts sigops in unexecuted scripts.
I think the wording is clear as such, but if you have a better one please suggest it.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2180297022)
I'm aware. "Where" refers to the scripts. This is in opposition to the current sigops counting which, as you know, counts sigops in unexecuted scripts.
I think the wording is clear as such, but if you have a better one please suggest it.
💬 luke-jr commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2180298687)
```suggestion
// Unlike the existing block wide sigop limit which counts sigops present in the block itself (including the scriptPubKey which is not executed until spending later), BIP54 counts sigops in the block where they are potentially executed (only).
```
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2180298687)
```suggestion
// Unlike the existing block wide sigop limit which counts sigops present in the block itself (including the scriptPubKey which is not executed until spending later), BIP54 counts sigops in the block where they are potentially executed (only).
```
📝 instagibbs opened a pull request: "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot"
(https://github.com/bitcoin/bitcoin/pull/32859)
Resolves https://github.com/bitcoin/bitcoin/pull/32841#discussion_r2180240391
(https://github.com/bitcoin/bitcoin/pull/32859)
Resolves https://github.com/bitcoin/bitcoin/pull/32841#discussion_r2180240391
💬 instagibbs commented on pull request "feature_taproot: sample tx version border values more":
(https://github.com/bitcoin/bitcoin/pull/32841#discussion_r2180310249)
opened potential resolution at https://github.com/bitcoin/bitcoin/pull/32859
was able to reproduce the issue and not run into it anymore
(https://github.com/bitcoin/bitcoin/pull/32841#discussion_r2180310249)
opened potential resolution at https://github.com/bitcoin/bitcoin/pull/32859
was able to reproduce the issue and not run into it anymore
💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2180310701)
Taken, thanks.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2180310701)
Taken, thanks.
👍 maflcko approved a pull request: "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot"
(https://github.com/bitcoin/bitcoin/pull/32859#pullrequestreview-2979572797)
lgtm
(https://github.com/bitcoin/bitcoin/pull/32859#pullrequestreview-2979572797)
lgtm
💬 maflcko commented on pull request "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32859#discussion_r2180318357)
```suggestion
and not (tx.version == 3 and tx.get_vsize() > TRUC_MAX_VSIZE) # Topological standardness rules must be followed
)
```
nit: Could move the `)` on the next line, to avoid having to touch the line again next time a rule is added below?
(https://github.com/bitcoin/bitcoin/pull/32859#discussion_r2180318357)
```suggestion
and not (tx.version == 3 and tx.get_vsize() > TRUC_MAX_VSIZE) # Topological standardness rules must be followed
)
```
nit: Could move the `)` on the next line, to avoid having to touch the line again next time a rule is added below?
💬 instagibbs commented on pull request "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32859#discussion_r2180355177)
taken, thanks
(https://github.com/bitcoin/bitcoin/pull/32859#discussion_r2180355177)
taken, thanks
📝 PixelPil0t1 opened a pull request: "Add BIP reference links to release notes"
(https://github.com/bitcoin/bitcoin/pull/32860)
Enhanced release notes documentation by adding direct links to BIP specifications for better readability and reference:
Added links to BIP 66 specification
Added links to BIP 22 (long polling) documentation
Added links to BIP 23 block proposals specification
Added links to BIP 34 miner-voting mechanism
All links point to the official Bitcoin BIPs repository for consistency and reliability. Maintained existing document structure and formatting while improving accessibility to technical spe
...
(https://github.com/bitcoin/bitcoin/pull/32860)
Enhanced release notes documentation by adding direct links to BIP specifications for better readability and reference:
Added links to BIP 66 specification
Added links to BIP 22 (long polling) documentation
Added links to BIP 23 block proposals specification
Added links to BIP 34 miner-voting mechanism
All links point to the official Bitcoin BIPs repository for consistency and reliability. Maintained existing document structure and formatting while improving accessibility to technical spe
...
✅ fanquake closed a pull request: "Add BIP reference links to release notes"
(https://github.com/bitcoin/bitcoin/pull/32860)
(https://github.com/bitcoin/bitcoin/pull/32860)
💬 fanquake commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3028428558)
Why is one of the native Windows jobs failing to compile: https://github.com/bitcoin/bitcoin/actions/runs/16027562790/job/45219347987?pr=32857#step:10:553, but not the other: https://github.com/bitcoin/bitcoin/actions/runs/16027562790/job/45219348012?pr=32857 ?
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3028428558)
Why is one of the native Windows jobs failing to compile: https://github.com/bitcoin/bitcoin/actions/runs/16027562790/job/45219347987?pr=32857#step:10:553, but not the other: https://github.com/bitcoin/bitcoin/actions/runs/16027562790/job/45219348012?pr=32857 ?
💬 hebasto commented on pull request "cmake: Improve Python robustness and test usability":
(https://github.com/bitcoin/bitcoin/pull/31233#issuecomment-3028458944)
My Guix build:
```
aarch64
cc67459424a62c706895f382cff677c1e0c0ecd7ee7f43429d64d1f5d872f151 guix-build-67dc7523f3e1/output/aarch64-linux-gnu/SHA256SUMS.part
3c1eb6f629cc3b00d7e3803fc33383aea1f7cf6646d4de3e5c96d30923a1a795 guix-build-67dc7523f3e1/output/aarch64-linux-gnu/bitcoin-67dc7523f3e1-aarch64-linux-gnu-debug.tar.gz
494c55c6843e0d1f8aa133ee4c7685f06f04ca4c3dcb8ccee02aa11686a64350 guix-build-67dc7523f3e1/output/aarch64-linux-gnu/bitcoin-67dc7523f3e1-aarch64-linux-gnu.tar.gz
e12c0167
...
(https://github.com/bitcoin/bitcoin/pull/31233#issuecomment-3028458944)
My Guix build:
```
aarch64
cc67459424a62c706895f382cff677c1e0c0ecd7ee7f43429d64d1f5d872f151 guix-build-67dc7523f3e1/output/aarch64-linux-gnu/SHA256SUMS.part
3c1eb6f629cc3b00d7e3803fc33383aea1f7cf6646d4de3e5c96d30923a1a795 guix-build-67dc7523f3e1/output/aarch64-linux-gnu/bitcoin-67dc7523f3e1-aarch64-linux-gnu-debug.tar.gz
494c55c6843e0d1f8aa133ee4c7685f06f04ca4c3dcb8ccee02aa11686a64350 guix-build-67dc7523f3e1/output/aarch64-linux-gnu/bitcoin-67dc7523f3e1-aarch64-linux-gnu.tar.gz
e12c0167
...
🤔 danielabrozzoni reviewed a pull request: "headerssync: Preempt unrealistic unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-2966948837)
Concept ACK, I've left a few questions/nits
I've only done a partial review so far, I still need to go through the last two commits and part of ced2ddb37c957e53d4a8aa40d3512ea3786511b4
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-2966948837)
Concept ACK, I've left a few questions/nits
I've only done a partial review so far, I still need to go through the last two commits and part of ced2ddb37c957e53d4a8aa40d3512ea3786511b4
💬 danielabrozzoni commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2180235458)
nit in 79bdc6a785bca96dababc698336c04b8625b7d1c: in some CHECK_RESULT you are putting `/*full_headers_message=*/`, in some others you're not, like this one, the one below, and the ones in TooLittleWork
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2180235458)
nit in 79bdc6a785bca96dababc698336c04b8625b7d1c: in some CHECK_RESULT you are putting `/*full_headers_message=*/`, in some others you're not, like this one, the one below, and the ones in TooLittleWork