💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2213251672)
Ugh good catch.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2213251672)
Ugh good catch.
💬 dergoegge commented on pull request "fuzz: Make process_message(s) more deterministic":
(https://github.com/bitcoin/bitcoin/pull/32822#issuecomment-3083944969)
This needs a rebase, but should be good to go otherwise.
(https://github.com/bitcoin/bitcoin/pull/32822#issuecomment-3083944969)
This needs a rebase, but should be good to go otherwise.
🤔 marcofleon reviewed a pull request: "validation: docs and cleanups for MemPoolAccept coins views"
(https://github.com/bitcoin/bitcoin/pull/32973#pullrequestreview-3029414103)
ACK b6d4688f77df9e31fd64e2be300f55bb8e944bd0
Verified that the two functional tests fail with the suggested buggy patches. Seems fine to remove this test if it doesn't provide any new assurances.
> I think there is a low likelihood of accidentally reintroducing this problem and not immediately hitting e.g. a fuzzer crash on this line
I also tried to see if we'd get a fuzz crash after applying the first patch and it doesn't seem to be happening. I'm running `tx_package_eval` with a small
...
(https://github.com/bitcoin/bitcoin/pull/32973#pullrequestreview-3029414103)
ACK b6d4688f77df9e31fd64e2be300f55bb8e944bd0
Verified that the two functional tests fail with the suggested buggy patches. Seems fine to remove this test if it doesn't provide any new assurances.
> I think there is a low likelihood of accidentally reintroducing this problem and not immediately hitting e.g. a fuzzer crash on this line
I also tried to see if we'd get a fuzz crash after applying the first patch and it doesn't seem to be happening. I'm running `tx_package_eval` with a small
...
💬 josibake commented on issue "ci: improve "test each commit" job to handle more complex scenarios":
(https://github.com/bitcoin/bitcoin/issues/32991#issuecomment-3083977467)
> you'll want to do `git rebase --interactive c42c7351a6316805fdfb20163762c1ab8293dd0d` and then delete the old subtree commits
Apologies if I'm being dense, but what you're suggesting doesn't make sense to me. What I'm doing currently:
```shell
# starting from master
git checkout -b refresh-secp256k1
# git subtree pull --prefix src/secp256k1/ <myfork> <mybranch> --squash
git subtree pull --prefix src/secp256k1/ josie-secp256k1 bip352-silentpayments-module-2025 --squash
# rebase my bitcoin c
...
(https://github.com/bitcoin/bitcoin/issues/32991#issuecomment-3083977467)
> you'll want to do `git rebase --interactive c42c7351a6316805fdfb20163762c1ab8293dd0d` and then delete the old subtree commits
Apologies if I'm being dense, but what you're suggesting doesn't make sense to me. What I'm doing currently:
```shell
# starting from master
git checkout -b refresh-secp256k1
# git subtree pull --prefix src/secp256k1/ <myfork> <mybranch> --squash
git subtree pull --prefix src/secp256k1/ josie-secp256k1 bip352-silentpayments-module-2025 --squash
# rebase my bitcoin c
...
💬 glozow commented on pull request "validation: docs and cleanups for MemPoolAccept coins views":
(https://github.com/bitcoin/bitcoin/pull/32973#issuecomment-3083993965)
> Fwiw if I add LimitMempoolSize() right before ClearSubPackageState() in AcceptSubPackage there is an almost immediate crash.
That's the kind of scenario I was thinking - somehow reintroducing a trim somewhere mid-subpackage.
(https://github.com/bitcoin/bitcoin/pull/32973#issuecomment-3083993965)
> Fwiw if I add LimitMempoolSize() right before ClearSubPackageState() in AcceptSubPackage there is an almost immediate crash.
That's the kind of scenario I was thinking - somehow reintroducing a trim somewhere mid-subpackage.
💬 glozow commented on pull request "validation: docs and cleanups for MemPoolAccept coins views":
(https://github.com/bitcoin/bitcoin/pull/32973#discussion_r2213299485)
Will change if I need to retouch
(https://github.com/bitcoin/bitcoin/pull/32973#discussion_r2213299485)
Will change if I need to retouch
💬 hebasto commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3084054356)
> CI failure seems to be due to [a bug in qt6 6.4](https://bugreports.qt.io/browse/QTBUG-31496?focusedId=888930&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-888930):
Although both failed CI job use Clang 20.1.7, the error can also be reproduced with GCC 13.3.
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3084054356)
> CI failure seems to be due to [a bug in qt6 6.4](https://bugreports.qt.io/browse/QTBUG-31496?focusedId=888930&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-888930):
Although both failed CI job use Clang 20.1.7, the error can also be reproduced with GCC 13.3.
🤔 yuvicc reviewed a pull request: "validation: remove BLOCK_FAILED_CHILD"
(https://github.com/bitcoin/bitcoin/pull/32950#pullrequestreview-3029521987)
Concept ACK
The CI failure looks like due to #32855
(https://github.com/bitcoin/bitcoin/pull/32950#pullrequestreview-3029521987)
Concept ACK
The CI failure looks like due to #32855
💬 maflcko commented on issue "ci: improve "test each commit" job to handle more complex scenarios":
(https://github.com/bitcoin/bitcoin/issues/32991#issuecomment-3084061052)
> git rebase --onto refresh-secp256k1 702eb96c46904288f72f02f968339c27c6524195^ implement-bip352
Yeah, that looks correct. It should also be identical to my version. The only difference is that yours works also non-iteractively. That is, the two commands give the same result:
```
git rebase --interactive --onto=14224fd2f3b31ed869397989d88306df2a3cfe6f da3494063f^ 8c073407aa # no interactive changes needed
```
```
git checkout 8c073407aa && git rebase --interactive 14224fd2f3b31ed869397989d88
...
(https://github.com/bitcoin/bitcoin/issues/32991#issuecomment-3084061052)
> git rebase --onto refresh-secp256k1 702eb96c46904288f72f02f968339c27c6524195^ implement-bip352
Yeah, that looks correct. It should also be identical to my version. The only difference is that yours works also non-iteractively. That is, the two commands give the same result:
```
git rebase --interactive --onto=14224fd2f3b31ed869397989d88306df2a3cfe6f da3494063f^ 8c073407aa # no interactive changes needed
```
```
git checkout 8c073407aa && git rebase --interactive 14224fd2f3b31ed869397989d88
...
💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2213349114)
Fixed. Added the check in three instances where P2SH inputs are added.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2213349114)
Fixed. Added the check in three instances where P2SH inputs are added.
💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3084072448)
> > Edit: Oh nvm, the other GetSigOpCount() recurses a P2SH script.
>
> I wouldn't mind a comment spelling out what it's doing, it is a peculiar function
Added a comment to this effect on the call site.
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3084072448)
> > Edit: Oh nvm, the other GetSigOpCount() recurses a P2SH script.
>
> I wouldn't mind a comment spelling out what it's doing, it is a peculiar function
Added a comment to this effect on the call site.
💬 instagibbs commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3084085825)
agreed, build issue also on my end with gcc 13.3.0
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3084085825)
agreed, build issue also on my end with gcc 13.3.0
💬 instagibbs commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3084094225)
reACK https://github.com/bitcoin/bitcoin/pull/32521/commits/96da68a38fa295d2414685739c41b8626e198d27
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3084094225)
reACK https://github.com/bitcoin/bitcoin/pull/32521/commits/96da68a38fa295d2414685739c41b8626e198d27
💬 stickies-v commented on pull request "test: Do not pass tests on unhandled exceptions":
(https://github.com/bitcoin/bitcoin/pull/33001#discussion_r2213372838)
All these exception handlers (except SkipTest) have the same behaviour, except for making the logs less useful by hiding the actual error. Can't we just remove all (except SkipTest) and let the exception speak for itself?
<details>
<summary>git diff on fad0cd510c</summary>
```diff
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 21007d9156..17e5a6b397 100755
--- a/test/functional/test_framework/test_framework.py
+++
...
(https://github.com/bitcoin/bitcoin/pull/33001#discussion_r2213372838)
All these exception handlers (except SkipTest) have the same behaviour, except for making the logs less useful by hiding the actual error. Can't we just remove all (except SkipTest) and let the exception speak for itself?
<details>
<summary>git diff on fad0cd510c</summary>
```diff
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 21007d9156..17e5a6b397 100755
--- a/test/functional/test_framework/test_framework.py
+++
...
💬 fanquake commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3084135372)
Minified this is:
```bash
# podman run -it ubuntu:noble
apt update && apt upgrade -y
apt install g++ qt6-base-dev qt6-tools-dev git
git clone https://github.com/bitcoin/bitcoin
cd bitcoin
git fetch origin pull/32998/head:32998
git checkout 32998
/usr/lib/qt6/libexec/moc -I/usr/include/c++/13 --output-dep-file -o test.moc /bitcoin/src/qt/intro.cpp
usr/include/c++/13/bits/cpp_type_traits.:69:1: error: Parse error at "std"
```
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3084135372)
Minified this is:
```bash
# podman run -it ubuntu:noble
apt update && apt upgrade -y
apt install g++ qt6-base-dev qt6-tools-dev git
git clone https://github.com/bitcoin/bitcoin
cd bitcoin
git fetch origin pull/32998/head:32998
git checkout 32998
/usr/lib/qt6/libexec/moc -I/usr/include/c++/13 --output-dep-file -o test.moc /bitcoin/src/qt/intro.cpp
usr/include/c++/13/bits/cpp_type_traits.:69:1: error: Parse error at "std"
```
💬 dongcarl commented on issue "Guix build fails on M4 macOS host with Ubuntu in UTM":
(https://github.com/bitcoin/bitcoin/issues/32759#issuecomment-3084139301)
Don't have full context but it's curious what causes `guix` to want to `set armhf-linux personality` in the first place.
Clearly M4 macOS hosts are `aarch64-linux`, and as @Sjors mentions:
- `guix build --system=armhf-linux hello` "produces the same error"
- `guix build --system=aarch64-linux hello --no-substitutes` does work
Another clue from the Guix manual:
> **Note**: Building for an armhf-linux system is unconditionally enabled on aarch64-linux machines, although certain aarch64 chipset
...
(https://github.com/bitcoin/bitcoin/issues/32759#issuecomment-3084139301)
Don't have full context but it's curious what causes `guix` to want to `set armhf-linux personality` in the first place.
Clearly M4 macOS hosts are `aarch64-linux`, and as @Sjors mentions:
- `guix build --system=armhf-linux hello` "produces the same error"
- `guix build --system=aarch64-linux hello --no-substitutes` does work
Another clue from the Guix manual:
> **Note**: Building for an armhf-linux system is unconditionally enabled on aarch64-linux machines, although certain aarch64 chipset
...
🤔 furszy reviewed a pull request: "index: initial sync speedup, parallelize process"
(https://github.com/bitcoin/bitcoin/pull/26966#pullrequestreview-3029621925)
> I tried to use this to boost the silent payment indexer, but I don't know what I'm doing :-) [Sjors#96](https://github.com/Sjors/bitcoin/pull/96)
@Sjors, see https://github.com/furszy/bitcoin-core/commits/2025_bip352_blind_fix.
Note: I only spent a few minutes with it and the test seem to pass (I'm partially afk these days). Let me know how it goes and could check it in detail next week.
The first commit there (413ce51bf10326a6c56bd4250f9a9f19fde44ed4) is merely a code improvement + clean
...
(https://github.com/bitcoin/bitcoin/pull/26966#pullrequestreview-3029621925)
> I tried to use this to boost the silent payment indexer, but I don't know what I'm doing :-) [Sjors#96](https://github.com/Sjors/bitcoin/pull/96)
@Sjors, see https://github.com/furszy/bitcoin-core/commits/2025_bip352_blind_fix.
Note: I only spent a few minutes with it and the test seem to pass (I'm partially afk these days). Let me know how it goes and could check it in detail next week.
The first commit there (413ce51bf10326a6c56bd4250f9a9f19fde44ed4) is merely a code improvement + clean
...
💬 hebasto commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3084150541)
> > CI failure seems to be due to [a bug in qt6 6.4](https://bugreports.qt.io/browse/QTBUG-31496?focusedId=888930&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-888930):
>
> Although both failed CI job use Clang 20.1.7, the error can also be reproduced with GCC 13.3.
[Refactoring](https://github.com/hebasto/bitcoin/commits/pr32998/0717.refactored/) helps.
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3084150541)
> > CI failure seems to be due to [a bug in qt6 6.4](https://bugreports.qt.io/browse/QTBUG-31496?focusedId=888930&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-888930):
>
> Although both failed CI job use Clang 20.1.7, the error can also be reproduced with GCC 13.3.
[Refactoring](https://github.com/hebasto/bitcoin/commits/pr32998/0717.refactored/) helps.
💬 instagibbs commented on pull request "validation: docs and cleanups for MemPoolAccept coins views":
(https://github.com/bitcoin/bitcoin/pull/32973#issuecomment-3084178413)
@marcofleon
This seed:
ALoAAAAAAAABAAAAAAAAAAAAAAAAAZNpogAyAAAAAAAAgAAAAAAAAAAAAAEAAAAAAAAAAAB5AAAA
AAAhAQAAAAEAAAAAAAAAAAAAAAABk2miADIAAAAAAACAAAAAAAAAAAAAAQAAAAAAAAAAAHkAAAAA
ACEBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABwAAAGTaaIAMgAAAAAAAAEAAAAAAAAAAAABAAAA
AAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAn////AZNpogAyAAAAAAAAAQAA
AAAAAAAAAAEAAABAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAACNAAEAAAAAAAAAAAAAAAAA
AAAAAQAAAAAAAAAAAAAAAAAAAAAAAABkYXRhZGlyYXRhAAAA
Should cause failure if you ap
...
(https://github.com/bitcoin/bitcoin/pull/32973#issuecomment-3084178413)
@marcofleon
This seed:
ALoAAAAAAAABAAAAAAAAAAAAAAAAAZNpogAyAAAAAAAAgAAAAAAAAAAAAAEAAAAAAAAAAAB5AAAA
AAAhAQAAAAEAAAAAAAAAAAAAAAABk2miADIAAAAAAACAAAAAAAAAAAAAAQAAAAAAAAAAAHkAAAAA
ACEBAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABwAAAGTaaIAMgAAAAAAAAEAAAAAAAAAAAABAAAA
AAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAn////AZNpogAyAAAAAAAAAQAA
AAAAAAAAAAEAAABAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAACNAAEAAAAAAAAAAAAAAAAA
AAAAAQAAAAAAAAAAAAAAAAAAAAAAAABkYXRhZGlyYXRhAAAA
Should cause failure if you ap
...
📝 brunoerg opened a pull request: "test: add option to skip large re-org test in feature_block"
(https://github.com/bitcoin/bitcoin/pull/33003)
Fixes #32877
This PR adds a config flag `--skipreorg` which is used to skip the large re-org test. According to corecheck, `feature_block` is our slowest functional test and primarily because of this large re-org test. However, this test might not be useful for the mutation analysis of some files and could be skipped to save a huge amount of time.
```
time ./build/test/functional/feature_block.py --skipreorg
./build/test/functional/feature_block.py --skipreorg 11.38s user 0.33s system
...
(https://github.com/bitcoin/bitcoin/pull/33003)
Fixes #32877
This PR adds a config flag `--skipreorg` which is used to skip the large re-org test. According to corecheck, `feature_block` is our slowest functional test and primarily because of this large re-org test. However, this test might not be useful for the mutation analysis of some files and could be skipped to save a huge amount of time.
```
time ./build/test/functional/feature_block.py --skipreorg
./build/test/functional/feature_block.py --skipreorg 11.38s user 0.33s system
...