π¬ 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
π¬ danielabrozzoni commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2172253857)
Just to confirm that I understand this correctly, are you removing the `<256>` because it's implicit?
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2172253857)
Just to confirm that I understand this correctly, are you removing the `<256>` because it's implicit?
π¬ danielabrozzoni commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2180366086)
nit in ced2ddb37c957e53d4a8aa40d3512ea3786511b4: the previous comment (in headerssync.cpp) said:
"Only **feed headers to validation** once this many headers.."
I think that phrasing was a bit clearer than βoutputtingβ
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2180366086)
nit in ced2ddb37c957e53d4a8aa40d3512ea3786511b4: the previous comment (in headerssync.cpp) said:
"Only **feed headers to validation** once this many headers.."
I think that phrasing was a bit clearer than βoutputtingβ
π¬ danielabrozzoni commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2180450887)
micro nit in 79bdc6a785bca96dababc698336c04b8625b7d1c: you could check `exp_pow_validated_prev` first and `exp_locator_hash` second, to maintain the same order as the macro parameters
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2180450887)
micro nit in 79bdc6a785bca96dababc698336c04b8625b7d1c: you could check `exp_pow_validated_prev` first and `exp_locator_hash` second, to maintain the same order as the macro parameters
π¬ l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2180509361)
Yes, they're redundant, can be deduced by the compiler
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2180509361)
Yes, they're redundant, can be deduced by the compiler
π¬ Sjors commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3028521384)
@fanquake because it's in the fuzzer code, I forgot to change one of the function signatures there.
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3028521384)
@fanquake because it's in the fuzzer code, I forgot to change one of the function signatures there.
π darosior approved a pull request: "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot"
(https://github.com/bitcoin/bitcoin/pull/32859#pullrequestreview-2979878770)
utACK f0524cda3995cf65adab3d0ca8da0dee4e31c79b
(https://github.com/bitcoin/bitcoin/pull/32859#pullrequestreview-2979878770)
utACK f0524cda3995cf65adab3d0ca8da0dee4e31c79b
π¬ Sjors commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3028577968)
I probably need to implement this at the `SigningProvider` level rather than `HidingSigningProvider`. The early return needs to be inside `SignTaproot()` right before `// Try script path spending` rather than in `GetTaprootSpendData()`. The goal isn't to hide leaf scripts from co-signers, only to make sure we don't sign them ourselves.
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3028577968)
I probably need to implement this at the `SigningProvider` level rather than `HidingSigningProvider`. The early return needs to be inside `SignTaproot()` right before `// Try script path spending` rather than in `GetTaprootSpendData()`. The goal isn't to hide leaf scripts from co-signers, only to make sure we don't sign them ourselves.
π€ w0xlt reviewed a pull request: "functional test: correctly detect nonstd TRUC tx vsize in feature_taproot"
(https://github.com/bitcoin/bitcoin/pull/32859#pullrequestreview-2979983814)
ACK https://github.com/bitcoin/bitcoin/pull/32859/commits/f0524cda3995cf65adab3d0ca8da0dee4e31c79b
(https://github.com/bitcoin/bitcoin/pull/32859#pullrequestreview-2979983814)
ACK https://github.com/bitcoin/bitcoin/pull/32859/commits/f0524cda3995cf65adab3d0ca8da0dee4e31c79b
π¬ tnndbtc commented on issue "bitcoind shouldn't fail to progress with synchronization: endless [leveldb] Generated table ... logs":
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-3028740646)
Just for document purpose, I profiled `bitcoind` for IBD process on Mac through `gperftool`, and have chatgpt to help analyze out.txt (attached) and following is the summary:
```
LevelDB I/O & compaction ~1500s+ ~30% Block/UTXO writes, frequent flushes
leveldb::DecodeFixed32 β 241.66s (4.93%)
leveldb::DBImpl::DoCompactionWork β 1430.83s (29.2%)
leveldb::TableBuilder::Add β 1276.91s (26.06%)
leveldb::TableBuilder::Flush β 1261.79s (25.75%)
This shows:
Heavy disk write and
...
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-3028740646)
Just for document purpose, I profiled `bitcoind` for IBD process on Mac through `gperftool`, and have chatgpt to help analyze out.txt (attached) and following is the summary:
```
LevelDB I/O & compaction ~1500s+ ~30% Block/UTXO writes, frequent flushes
leveldb::DecodeFixed32 β 241.66s (4.93%)
leveldb::DBImpl::DoCompactionWork β 1430.83s (29.2%)
leveldb::TableBuilder::Add β 1276.91s (26.06%)
leveldb::TableBuilder::Flush β 1261.79s (25.75%)
This shows:
Heavy disk write and
...
π pinheadmz approved a pull request: "test: allow all functional tests to be run or skipped with --usecli"
(https://github.com/bitcoin/bitcoin/pull/32290#pullrequestreview-2980079242)
re-ACK 666016e56b28b77f798dc85c767b95c1ca0abfae
Built and ran all functional tests with --usecli on macos/arm64. Reviewed all code changes again since it's been a while since last review. Coverage is really great.
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK 666016e56b28b77f798dc85c767b95c1ca0abfae
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmhlbmsACgkQ5+KYS2KJ
yTp9HxAAmFHGtNyp2QfIz2EERL9n8BI/
...
(https://github.com/bitcoin/bitcoin/pull/32290#pullrequestreview-2980079242)
re-ACK 666016e56b28b77f798dc85c767b95c1ca0abfae
Built and ran all functional tests with --usecli on macos/arm64. Reviewed all code changes again since it's been a while since last review. Coverage is really great.
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK 666016e56b28b77f798dc85c767b95c1ca0abfae
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmhlbmsACgkQ5+KYS2KJ
yTp9HxAAmFHGtNyp2QfIz2EERL9n8BI/
...