💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2766164288)
> These sizes look quite acceptable.
To clarify my calculation: The `win64-native-vcpkg-binary` cache takes the longest to compute, because the windows task is the slowest. So if there are three merges, where two of them modify the `win64-native-vcpkg-binary` cache key, there will be three trailing writes of the cache of a size of 2.6GB each. This will consume ~7.8GB, so after one more task is added after this one here, the cache could start breaking/evicting non-windows caches with two pus
...
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2766164288)
> These sizes look quite acceptable.
To clarify my calculation: The `win64-native-vcpkg-binary` cache takes the longest to compute, because the windows task is the slowest. So if there are three merges, where two of them modify the `win64-native-vcpkg-binary` cache key, there will be three trailing writes of the cache of a size of 2.6GB each. This will consume ~7.8GB, so after one more task is added after this one here, the cache could start breaking/evicting non-windows caches with two pus
...
💬 eval-exec commented on pull request "torcontrol: Define tor reply code as const to improve our maintainability":
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021024160)
Thank you.
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021024160)
Thank you.
💬 eval-exec commented on pull request "torcontrol: Define tor reply code as const to improve our maintainability":
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021027893)
I'm fine with either option; let the Bitcoin maintainers decide.
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021027893)
I'm fine with either option; let the Bitcoin maintainers decide.
💬 hodlinator commented on pull request "fuzz: Make partially_downloaded_block more deterministic":
(https://github.com/bitcoin/bitcoin/pull/32158#discussion_r2021028255)
> I don't think your suggestion works.
I've verified it works by executing something that fails determinism:
```
cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/build_fuzz ../qa-assets/fuzz_corpora/ connman 16
```
> it may return early and leave some threads dangling.
https://doc.rust-lang.org/stable/std/thread/struct.ScopedJoinHandle.html states:
"An owned permission to join on a scoped thread (block on its termination)."
> However,
...
(https://github.com/bitcoin/bitcoin/pull/32158#discussion_r2021028255)
> I don't think your suggestion works.
I've verified it works by executing something that fails determinism:
```
cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/build_fuzz ../qa-assets/fuzz_corpora/ connman 16
```
> it may return early and leave some threads dangling.
https://doc.rust-lang.org/stable/std/thread/struct.ScopedJoinHandle.html states:
"An owned permission to join on a scoped thread (block on its termination)."
> However,
...
💬 laanwj commented on pull request "torcontrol: Define tor reply code as const to improve our maintainability":
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021044241)
The requirements for stream isolation are documented here: https://spec.torproject.org/socks-extensions.html#stream-isolation . Our case is "They are both SOCKS5 with USERNAME/PASSWORD authentication, using legacy isolation parameters, and they have identical usernames and identical passwords". The `<torS0X>` extension is very recent so we don't use that (yet?).
So using different credentials is enough, there is no requirement, nor even benefit for them to be random.
Although thinking of i
...
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021044241)
The requirements for stream isolation are documented here: https://spec.torproject.org/socks-extensions.html#stream-isolation . Our case is "They are both SOCKS5 with USERNAME/PASSWORD authentication, using legacy isolation parameters, and they have identical usernames and identical passwords". The `<torS0X>` extension is very recent so we don't use that (yet?).
So using different credentials is enough, there is no requirement, nor even benefit for them to be random.
Although thinking of i
...
💬 laanwj commented on pull request "torcontrol: Define tor reply code as const to improve our maintainability":
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021046837)
Pretty sure it is was already validated, this is how we discovered that the parameter is `_randomize_credentials` not `randomize_credentials`.
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021046837)
Pretty sure it is was already validated, this is how we discovered that the parameter is `_randomize_credentials` not `randomize_credentials`.
💬 laanwj commented on pull request "torcontrol: Define tor reply code as const to improve our maintainability":
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021049987)
Oh, we used to have a `=` but it was lost? You're right.
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021049987)
Oh, we used to have a `=` but it was lost? You're right.
💬 maflcko commented on pull request "fuzz: Make partially_downloaded_block more deterministic":
(https://github.com/bitcoin/bitcoin/pull/32158#discussion_r2021053788)
You should be able to test my claim by panicking the threads and observing that the `Err` never makes it out.
See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b7091d91fc333d2e260327c46a90dec6
Of course, this is fine here. I just wanted to explain the change in behavior here and why I choose the more consistent behavior myself here.
(https://github.com/bitcoin/bitcoin/pull/32158#discussion_r2021053788)
You should be able to test my claim by panicking the threads and observing that the `Err` never makes it out.
See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b7091d91fc333d2e260327c46a90dec6
Of course, this is fine here. I just wanted to explain the change in behavior here and why I choose the more consistent behavior myself here.
💬 hodlinator commented on pull request "fuzz: Make partially_downloaded_block more deterministic":
(https://github.com/bitcoin/bitcoin/pull/32158#discussion_r2021054952)
> if you want less libfuzzer output, it would be easier to compile without libfuzzer
Given the defaults in contrib/devtools/README.md I still think this would be a good change.
Missed that `output()` still allowed checking the exit code. You are right in that it cleaned things up: a7f4e4b7cd0a04ae7c84ad7afa284006a600e369
(https://github.com/bitcoin/bitcoin/pull/32158#discussion_r2021054952)
> if you want less libfuzzer output, it would be easier to compile without libfuzzer
Given the defaults in contrib/devtools/README.md I still think this would be a good change.
Missed that `output()` still allowed checking the exit code. You are right in that it cleaned things up: a7f4e4b7cd0a04ae7c84ad7afa284006a600e369
👋 glozow's pull request is ready for review: "[29.x] backports and 29.0rc3"
(https://github.com/bitcoin/bitcoin/pull/32136)
(https://github.com/bitcoin/bitcoin/pull/32136)
🤔 fjahr reviewed a pull request: "wallet, migration: Fix empty wallet crash"
(https://github.com/bitcoin/bitcoin/pull/32149#pullrequestreview-2729324019)
tACK d4cb6d048b0222cf51c8a56fa56f97dd5d2c9add
Changes look good to me and I confirmed that the new test fails on master.
(https://github.com/bitcoin/bitcoin/pull/32149#pullrequestreview-2729324019)
tACK d4cb6d048b0222cf51c8a56fa56f97dd5d2c9add
Changes look good to me and I confirmed that the new test fails on master.
💬 fanquake commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2766324142)
Yea, it's kind of ridiculous the amount of packages we need to build and cache for the native Windows build (and the time it takes for a full compile). This is also one of the most common causes of sporadic CI failures, when dependencies of dependencies (which we don't even use, i.e a compression library compiled as a qt input) fail for some sporadic reason.
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2766324142)
Yea, it's kind of ridiculous the amount of packages we need to build and cache for the native Windows build (and the time it takes for a full compile). This is also one of the most common causes of sporadic CI failures, when dependencies of dependencies (which we don't even use, i.e a compression library compiled as a qt input) fail for some sporadic reason.
🚀 fanquake merged a pull request: "ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176)
(https://github.com/bitcoin/bitcoin/pull/31176)
💬 laanwj commented on pull request "torcontrol: Define tor reply code as const to improve our maintainability":
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021101693)
> So maybe it's worth doing something else here, for example generate a unique prefix per bitcoind session startup (this would be once, so no need for performant randomness). But it's out of scope for this PR.
i'll open a PR for this.
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021101693)
> So maybe it's worth doing something else here, for example generate a unique prefix per bitcoind session startup (this would be once, so no need for performant randomness). But it's out of scope for this PR.
i'll open a PR for this.
💬 hodlinator commented on pull request "fuzz: Make partially_downloaded_block more deterministic":
(https://github.com/bitcoin/bitcoin/pull/32158#discussion_r2021106990)
Thanks for the correction.
I believe this would work, even though rustfmt makes the line count less of a win: 67e6d9ece87606297865761f0fb222db9604c180
(https://github.com/bitcoin/bitcoin/pull/32158#discussion_r2021106990)
Thanks for the correction.
I believe this would work, even though rustfmt makes the line count less of a win: 67e6d9ece87606297865761f0fb222db9604c180
💬 hodlinator commented on pull request "fuzz: Make partially_downloaded_block more deterministic":
(https://github.com/bitcoin/bitcoin/pull/32158#discussion_r2021098980)
Now run through rustfmt: 142e65da352e77f061da3142bff8e2b19e56fc86
(https://github.com/bitcoin/bitcoin/pull/32158#discussion_r2021098980)
Now run through rustfmt: 142e65da352e77f061da3142bff8e2b19e56fc86
💬 furszy commented on pull request "wallet, migration: Fix empty wallet crash":
(https://github.com/bitcoin/bitcoin/pull/32149#discussion_r2021114298)
extra ; at the end.
(https://github.com/bitcoin/bitcoin/pull/32149#discussion_r2021114298)
extra ; at the end.
💬 hebasto commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2021118543)
https://github.com/bitcoin-core/libmultiprocess/pull/168 renders this line unnecessary.
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2021118543)
https://github.com/bitcoin-core/libmultiprocess/pull/168 renders this line unnecessary.
💬 pablomartin4btc commented on pull request "wallet, migration: Fix empty wallet crash":
(https://github.com/bitcoin/bitcoin/pull/32149#discussion_r2021126219)
argh! Thanks!
(https://github.com/bitcoin/bitcoin/pull/32149#discussion_r2021126219)
argh! Thanks!
💬 pablomartin4btc commented on pull request "wallet, migration: Fix empty wallet crash":
(https://github.com/bitcoin/bitcoin/pull/32149#issuecomment-2766373715)
Fixed a typo.
(https://github.com/bitcoin/bitcoin/pull/32149#issuecomment-2766373715)
Fixed a typo.