💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474116685)
ready for review, I think
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474116685)
ready for review, I think
💬 achow101 commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840779076)
It returned RPC originally, and I don't think that adding a rename to the diff would be helpful.
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840779076)
It returned RPC originally, and I don't think that adding a rename to the diff would be helpful.
💬 achow101 commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840781994)
No, the master node migrates a copy of the wallet. The old node will have the original copy which remains unchanged regardless.
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840781994)
No, the master node migrates a copy of the wallet. The old node will have the original copy which remains unchanged regardless.
📝 maflcko opened a pull request: "ci: Skip broken Wine64 tests by default"
(https://github.com/bitcoin/bitcoin/pull/31284)
I don't think the unit tests run in Wine after the Windows cross-compilation have ever shown a true positive since the MSVC task was added. However, they are a source of frequent false-positives.
Thus, disable them by default for now. Anyone can still enable them by setting `RUN_UNIT_TESTS=true`.
Fixes #31071
A follow-up could run them on real Windows, but this comes with its own downsides, see https://github.com/bitcoin/bitcoin/pull/31176.
Conceptually there are many other nightly t
...
(https://github.com/bitcoin/bitcoin/pull/31284)
I don't think the unit tests run in Wine after the Windows cross-compilation have ever shown a true positive since the MSVC task was added. However, they are a source of frequent false-positives.
Thus, disable them by default for now. Anyone can still enable them by setting `RUN_UNIT_TESTS=true`.
Fixes #31071
A follow-up could run them on real Windows, but this comes with its own downsides, see https://github.com/bitcoin/bitcoin/pull/31176.
Conceptually there are many other nightly t
...
💬 achow101 commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840786362)
`notloaded` is specifically tested because legacy wallets won't be able to be loaded.
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840786362)
`notloaded` is specifically tested because legacy wallets won't be able to be loaded.
💬 achow101 commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840787871)
Yes, no test relies on the time.
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840787871)
Yes, no test relies on the time.
💬 achow101 commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840789029)
Will do if I need to retouch.
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840789029)
Will do if I need to retouch.
💬 achow101 commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840789191)
Will do if I need to retouch.
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840789191)
Will do if I need to retouch.
💬 achow101 commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840789364)
Will do if I need to retouch.
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840789364)
Will do if I need to retouch.
💬 sdaftuar commented on pull request "validation: Remove RECENT_CONSENSUS_CHANGE validation result":
(https://github.com/bitcoin/bitcoin/pull/31269#issuecomment-2474285264)
As far as I can tell, I was also in agreement with not needing this `RECENT_CONSENSUS_CHANGE` enum in our code until it was time to use it! @TheBlueMatt was the one who thought we needed it, and I came around to thinking that there was a potential use case for it in the event of another soft fork.
But I'm fine with dropping it; if someone decides to rework our net_processing code to take advantage of subtler differences in validation failures, we can reintroduce it at that point.
(https://github.com/bitcoin/bitcoin/pull/31269#issuecomment-2474285264)
As far as I can tell, I was also in agreement with not needing this `RECENT_CONSENSUS_CHANGE` enum in our code until it was time to use it! @TheBlueMatt was the one who thought we needed it, and I came around to thinking that there was a potential use case for it in the event of another soft fork.
But I'm fine with dropping it; if someone decides to rework our net_processing code to take advantage of subtler differences in validation failures, we can reintroduce it at that point.
🤔 maflcko reviewed a pull request: "doc: Fix gen-manpages to check build options"
(https://github.com/bitcoin/bitcoin/pull/29457#pullrequestreview-2433974717)
This just fails on the release binaries. Please make sure to test the code yourself before asking for review. If you run into issues, you can ask questions.
```
BUILDDIR=bld ./contrib/devtools/gen-manpages.py
Aborting generating manpages...
Error: 'HAVE_SYSTEM' (System component) support is not enabled.
Please enable it and try again.
```
You'll probably have to use `bld/src/bitcoin-build-config.h` instead of the test ini. It seems odd anyway to tie the test config to this.
(https://github.com/bitcoin/bitcoin/pull/29457#pullrequestreview-2433974717)
This just fails on the release binaries. Please make sure to test the code yourself before asking for review. If you run into issues, you can ask questions.
```
BUILDDIR=bld ./contrib/devtools/gen-manpages.py
Aborting generating manpages...
Error: 'HAVE_SYSTEM' (System component) support is not enabled.
Please enable it and try again.
```
You'll probably have to use `bld/src/bitcoin-build-config.h` instead of the test ini. It seems odd anyway to tie the test config to this.
💬 maflcko commented on pull request "doc: Fix gen-manpages to check build options":
(https://github.com/bitcoin/bitcoin/pull/29457#discussion_r1840887318)
You can't just change the relative path to include an unrelated prefix. This breaks when using `BUILDDIR`. Please see the conflicting pull request on how to correctly do it.
(https://github.com/bitcoin/bitcoin/pull/29457#discussion_r1840887318)
You can't just change the relative path to include an unrelated prefix. This breaks when using `BUILDDIR`. Please see the conflicting pull request on how to correctly do it.
📝 fanquake opened a pull request: "guix: remove `util-linux`"
(https://github.com/bitcoin/bitcoin/pull/31285)
It's not immediately clear which of the utils was the last used (maybe getopt, will check), and when it was last used (maybe post CMake), but this package is no-longer needed to complete a Guix build (or needed in the CI). `util-linux` has been in the manifest since Guix was first introduced (3e80ec3ea9691c7c89173de922a113e643fe976b).
Guix build:
```bash
b681575760a0d19445a17d0d54f50a65d705923caee3a7cd7502e6611950dc11 guix-build-4d668549825c/output/aarch64-linux-gnu/SHA256SUMS.part
7af142
...
(https://github.com/bitcoin/bitcoin/pull/31285)
It's not immediately clear which of the utils was the last used (maybe getopt, will check), and when it was last used (maybe post CMake), but this package is no-longer needed to complete a Guix build (or needed in the CI). `util-linux` has been in the manifest since Guix was first introduced (3e80ec3ea9691c7c89173de922a113e643fe976b).
Guix build:
```bash
b681575760a0d19445a17d0d54f50a65d705923caee3a7cd7502e6611950dc11 guix-build-4d668549825c/output/aarch64-linux-gnu/SHA256SUMS.part
7af142
...
💬 marcofleon commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474371365)
From various fuzz runs:
<details>
<summary>stack trace 1</summary>
```
UndefinedBehaviorSanitizer:DEADLYSIGNAL
==1654160==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000010 (pc 0x7f4ed8ac0563 bp 0x7ffd7bdeff90 sp 0x7ffd7bdefba8 T1654160)
==1654160==The signal is caused by a READ memory access.
==1654160==Hint: address points to the zero page.
#0 0x7f4ed8ac0563 in std::_Rb_tree_rebalance_for_erase(std::_Rb_tree_node_base*, std::_Rb_tree_node_base&) (/lib/x86
...
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474371365)
From various fuzz runs:
<details>
<summary>stack trace 1</summary>
```
UndefinedBehaviorSanitizer:DEADLYSIGNAL
==1654160==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000010 (pc 0x7f4ed8ac0563 bp 0x7ffd7bdeff90 sp 0x7ffd7bdefba8 T1654160)
==1654160==The signal is caused by a READ memory access.
==1654160==Hint: address points to the zero page.
#0 0x7f4ed8ac0563 in std::_Rb_tree_rebalance_for_erase(std::_Rb_tree_node_base*, std::_Rb_tree_node_base&) (/lib/x86
...
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474376868)
@marcofleon to be clear this is happening on master or follow-up?
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474376868)
@marcofleon to be clear this is happening on master or follow-up?
💬 laanwj commented on pull request "guix: remove `util-linux`":
(https://github.com/bitcoin/bitcoin/pull/31285#issuecomment-2474381820)
Concept ACK
Does this make a difference to the build env?
i wonder if util-linux is somehow coming in through the closure of one of the other packages, it seems such a base thing
(https://github.com/bitcoin/bitcoin/pull/31285#issuecomment-2474381820)
Concept ACK
Does this make a difference to the build env?
i wonder if util-linux is somehow coming in through the closure of one of the other packages, it seems such a base thing
💬 maflcko commented on pull request "ci: Skip broken Wine64 tests by default":
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2474383795)
Checked that `RUN_UNIT_TESTS=true MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_win64.sh" ./ci/test_run_all.sh` still works.
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2474383795)
Checked that `RUN_UNIT_TESTS=true MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_win64.sh" ./ci/test_run_all.sh` still works.
💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474385001)
ok replicated on this PR, I'll track it down
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474385001)
ok replicated on this PR, I'll track it down
💬 marcofleon commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474387376)
Yeah not on master, just this PR.
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474387376)
Yeah not on master, just this PR.
💬 marcofleon commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474394030)
It seems to be commit 7f0d8633b174c8b96cc28ec05005b8e89ab4957d
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2474394030)
It seems to be commit 7f0d8633b174c8b96cc28ec05005b8e89ab4957d