:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/31637)
(https://github.com/bitcoin/bitcoin/issues/31637)
✅ maflcko closed a pull request: "fix: grammatical errors"
(https://github.com/bitcoin/bitcoin/pull/31636)
(https://github.com/bitcoin/bitcoin/pull/31636)
💬 maflcko commented on pull request "fix: grammatical errors":
(https://github.com/bitcoin/bitcoin/pull/31636#issuecomment-2583000205)
historic release notes are archived and should not be modified
(https://github.com/bitcoin/bitcoin/pull/31636#issuecomment-2583000205)
historic release notes are archived and should not be modified
💬 fanquake commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#issuecomment-2583006911)
https://github.com/bitcoin/bitcoin/actions/runs/12700499877/job/35403281996?pr=31622#step:12:8987:
```bash
test 2025-01-10T00:28:24.738000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "D:\a\bitcoin\bitcoin\build\test\functional\test_framework\test_framework.py", line 135, in main
self.run_test()
File "D:\
...
(https://github.com/bitcoin/bitcoin/pull/31622#issuecomment-2583006911)
https://github.com/bitcoin/bitcoin/actions/runs/12700499877/job/35403281996?pr=31622#step:12:8987:
```bash
test 2025-01-10T00:28:24.738000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "D:\a\bitcoin\bitcoin\build\test\functional\test_framework\test_framework.py", line 135, in main
self.run_test()
File "D:\
...
💬 fanquake commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2583015317)
https://cirrus-ci.com/task/6223844022681600?logs=ci#L2898:
```bash
[08:40:37.725] + LD_LIBRARY_PATH=/ci_container_base/depends/i686-pc-linux-gnu/lib
[08:40:37.725] + test/functional/test_runner.py --ci -j10 --tmpdirprefix /ci_container_base/ci/scratch/test_runner/ --ansi --combinedlogslen=99999999 --timeout-factor=40 --v2transport --quiet --failfast
[08:40:38.317] 2025-01-10T13:40:38.202000Z TestFramework (INFO): PRNG seed is: 5569585375096173377
[08:40:38.317] 2025-01-10T13:40:38.206000Z T
...
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2583015317)
https://cirrus-ci.com/task/6223844022681600?logs=ci#L2898:
```bash
[08:40:37.725] + LD_LIBRARY_PATH=/ci_container_base/depends/i686-pc-linux-gnu/lib
[08:40:37.725] + test/functional/test_runner.py --ci -j10 --tmpdirprefix /ci_container_base/ci/scratch/test_runner/ --ansi --combinedlogslen=99999999 --timeout-factor=40 --v2transport --quiet --failfast
[08:40:38.317] 2025-01-10T13:40:38.202000Z TestFramework (INFO): PRNG seed is: 5569585375096173377
[08:40:38.317] 2025-01-10T13:40:38.206000Z T
...
💬 Sjors commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2583073982)
Concept ACK
Lightly tested 8c44a947b501c8417454527637bd3adb3ce2ff4e by building on macOS 15.2 (M4). I like how the documentation is changed with a scripted-diff. The build system changes in the first commit look reasonable to me at first glance, but did not review thoroughly.
@ryanofsky wrote:
> Possibly, it might make sense to build bitcoin-chainstate.exe and bitcoinkernel.dll to libexec/ instead of bin/.
I like that as well. If we add them to the release later, they won't clutter `
...
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2583073982)
Concept ACK
Lightly tested 8c44a947b501c8417454527637bd3adb3ce2ff4e by building on macOS 15.2 (M4). I like how the documentation is changed with a scripted-diff. The build system changes in the first commit look reasonable to me at first glance, but did not review thoroughly.
@ryanofsky wrote:
> Possibly, it might make sense to build bitcoin-chainstate.exe and bitcoinkernel.dll to libexec/ instead of bin/.
I like that as well. If we add them to the release later, they won't clutter `
...
💬 mzumsande commented on issue "Potential crash (assert) rescanning wallet":
(https://github.com/bitcoin/bitcoin/issues/31474#issuecomment-2583089022)
Up to you of course, but yes, if this fixed the problem for you it'd help to say it there - thanks!
(https://github.com/bitcoin/bitcoin/issues/31474#issuecomment-2583089022)
Up to you of course, but yes, if this fixed the problem for you it'd help to say it there - thanks!
💬 fanquake commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#issuecomment-2583098454)
Guix Build:
```bash
3922712c31b158aefa34a0700c02dc0c2c02c318002ead81cc7c9836c34c7b6a guix-build-01df180bfb82/output/aarch64-linux-gnu/SHA256SUMS.part
600a22f791e838113d1b0401f0b038a5cf7e28638e6eb80cd80f39885dd2c63b guix-build-01df180bfb82/output/aarch64-linux-gnu/bitcoin-01df180bfb82-aarch64-linux-gnu-debug.tar.gz
e9a43319a6f6358940ff265be574d067604ce4f31d7c1e5be0c143c4407892ef guix-build-01df180bfb82/output/aarch64-linux-gnu/bitcoin-01df180bfb82-aarch64-linux-gnu.tar.gz
a049be1301297687
...
(https://github.com/bitcoin/bitcoin/pull/31125#issuecomment-2583098454)
Guix Build:
```bash
3922712c31b158aefa34a0700c02dc0c2c02c318002ead81cc7c9836c34c7b6a guix-build-01df180bfb82/output/aarch64-linux-gnu/SHA256SUMS.part
600a22f791e838113d1b0401f0b038a5cf7e28638e6eb80cd80f39885dd2c63b guix-build-01df180bfb82/output/aarch64-linux-gnu/bitcoin-01df180bfb82-aarch64-linux-gnu-debug.tar.gz
e9a43319a6f6358940ff265be574d067604ce4f31d7c1e5be0c143c4407892ef guix-build-01df180bfb82/output/aarch64-linux-gnu/bitcoin-01df180bfb82-aarch64-linux-gnu.tar.gz
a049be1301297687
...
💬 theuni commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2583175364)
> * It would be good if multiprocess binaries `bitcoin-node` and `bitcoin-gui` could be built in libexec/ instead of than bin/
This makes sense to me if we're going to go down the route of the wrapper executable, but I don't think we should do that until users aren't actually supposed to be launching them directly.
IMO it's not a problem to move them after a release cycle or two if the expected user behavior changes, if nothing else that's actually a good indication that their behavior _sh
...
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2583175364)
> * It would be good if multiprocess binaries `bitcoin-node` and `bitcoin-gui` could be built in libexec/ instead of than bin/
This makes sense to me if we're going to go down the route of the wrapper executable, but I don't think we should do that until users aren't actually supposed to be launching them directly.
IMO it's not a problem to move them after a release cycle or two if the expected user behavior changes, if nothing else that's actually a good indication that their behavior _sh
...
💬 fanquake commented on pull request "build: enable libc++ and libstdc++ hardening":
(https://github.com/bitcoin/bitcoin/pull/31424#discussion_r1910658510)
Not sure. This just seems to be removing an option (that otherwise doesn't conflict with the other changes)?
(https://github.com/bitcoin/bitcoin/pull/31424#discussion_r1910658510)
Not sure. This just seems to be removing an option (that otherwise doesn't conflict with the other changes)?
💬 fanquake commented on pull request "build: enable libc++ and libstdc++ hardening":
(https://github.com/bitcoin/bitcoin/pull/31424#discussion_r1910658664)
> so we use _GLIBCXX_ASSERTIONS in both Debug and non-Debug builds.
How did you test the performance impacts of this? For example master:
```bash
./master/src/bench/bench_bitcoin -filter=LinearizeOptimallyExample11
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|-
...
(https://github.com/bitcoin/bitcoin/pull/31424#discussion_r1910658664)
> so we use _GLIBCXX_ASSERTIONS in both Debug and non-Debug builds.
How did you test the performance impacts of this? For example master:
```bash
./master/src/bench/bench_bitcoin -filter=LinearizeOptimallyExample11
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|-
...
💬 darosior commented on pull request "Miner: never create a template which exploits the timewarp bug":
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2583326352)
Yes. I intend to rebase on top of it once it's merged.
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2583326352)
Yes. I intend to rebase on top of it once it's merged.
💬 darosior commented on pull request "Miner: never create a template which exploits the timewarp bug":
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2583327852)
Actually, this and #31600 can go in either order.
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2583327852)
Actually, this and #31600 can go in either order.
💬 hebasto commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#issuecomment-2583331131)
My Guix build:
```
aarch64
3922712c31b158aefa34a0700c02dc0c2c02c318002ead81cc7c9836c34c7b6a guix-build-01df180bfb82/output/aarch64-linux-gnu/SHA256SUMS.part
600a22f791e838113d1b0401f0b038a5cf7e28638e6eb80cd80f39885dd2c63b guix-build-01df180bfb82/output/aarch64-linux-gnu/bitcoin-01df180bfb82-aarch64-linux-gnu-debug.tar.gz
e9a43319a6f6358940ff265be574d067604ce4f31d7c1e5be0c143c4407892ef guix-build-01df180bfb82/output/aarch64-linux-gnu/bitcoin-01df180bfb82-aarch64-linux-gnu.tar.gz
a049be13
...
(https://github.com/bitcoin/bitcoin/pull/31125#issuecomment-2583331131)
My Guix build:
```
aarch64
3922712c31b158aefa34a0700c02dc0c2c02c318002ead81cc7c9836c34c7b6a guix-build-01df180bfb82/output/aarch64-linux-gnu/SHA256SUMS.part
600a22f791e838113d1b0401f0b038a5cf7e28638e6eb80cd80f39885dd2c63b guix-build-01df180bfb82/output/aarch64-linux-gnu/bitcoin-01df180bfb82-aarch64-linux-gnu-debug.tar.gz
e9a43319a6f6358940ff265be574d067604ce4f31d7c1e5be0c143c4407892ef guix-build-01df180bfb82/output/aarch64-linux-gnu/bitcoin-01df180bfb82-aarch64-linux-gnu.tar.gz
a049be13
...
💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2583384638)
Pushed a commit to test https://github.com/chaincodelabs/libmultiprocess/pull/121
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2583384638)
Pushed a commit to test https://github.com/chaincodelabs/libmultiprocess/pull/121
💬 psgreco commented on pull request "wallet: fix rescanning inconsistency":
(https://github.com/bitcoin/bitcoin/pull/31629#issuecomment-2583454367)
Not that it matters much, but UTACK 4818da809f0e300016390dd41e02c6067ffa008f
This is what fixed the issue in Elements
(https://github.com/bitcoin/bitcoin/pull/31629#issuecomment-2583454367)
Not that it matters much, but UTACK 4818da809f0e300016390dd41e02c6067ffa008f
This is what fixed the issue in Elements
💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1910819090)
Good catch!
I must have copied that URI (without the `?`) from the previous check (`// Invalid query string syntax is the same as not having parameters`).
But for this test in this PR it doesn't really matter I think (we can have even 2, one with `?` and one without it - or just clarify it in the comment to avoid confusion) as the URI would be invalid anyways (it includes invalid char `%`).
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1910819090)
Good catch!
I must have copied that URI (without the `?`) from the previous check (`// Invalid query string syntax is the same as not having parameters`).
But for this test in this PR it doesn't really matter I think (we can have even 2, one with `?` and one without it - or just clarify it in the comment to avoid confusion) as the URI would be invalid anyways (it includes invalid char `%`).
🤔 ryanofsky reviewed a pull request: "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior"
(https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2543016983)
Note to previous reviewers: I broke up and rearranged all the commits here and rewrote most of the descriptions to make the purpose of the PR clearer, but there are no significant code changes since the last push (just a small refactoring in the `-onlynet` code and a suggested test simplification).
---
Updated 80608ba5d282ae8713ea0136ea9a0208b254c082 -> d4b02f04c8960f9d1fa9ff69977b21eccb104d41 ([`pr/listset.6`](https://github.com/ryanofsky/bitcoin/commits/pr/listset.6) -> [`pr/listset.7`](
...
(https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2543016983)
Note to previous reviewers: I broke up and rearranged all the commits here and rewrote most of the descriptions to make the purpose of the PR clearer, but there are no significant code changes since the last push (just a small refactoring in the `-onlynet` code and a suggested test simplification).
---
Updated 80608ba5d282ae8713ea0136ea9a0208b254c082 -> d4b02f04c8960f9d1fa9ff69977b21eccb104d41 ([`pr/listset.6`](https://github.com/ryanofsky/bitcoin/commits/pr/listset.6) -> [`pr/listset.7`](
...
💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1910688838)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893738529
Thanks, I adopted your suggestion now. I assumed the other tests were also checking for `addcon_thread_started` to fail quickly instead of timing out if expected messages did not show up. But actually the other tests are using `addcon_thread_started`, because they are checking for absence not presence of other log messages, so they don't have any other alternative except checking for the later log message.
I still th
...
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1910688838)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893738529
Thanks, I adopted your suggestion now. I assumed the other tests were also checking for `addcon_thread_started` to fail quickly instead of timing out if expected messages did not show up. But actually the other tests are using `addcon_thread_started`, because they are checking for absence not presence of other log messages, so they don't have any other alternative except checking for the later log message.
I still th
...
💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1910687690)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893895081
Thanks, good catches. All of this should be fixed now.
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1910687690)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893895081
Thanks, good catches. All of this should be fixed now.