💬 Sammie05 commented on pull request "ci: Run GUI unit tests in cross-Windows task":
(https://github.com/bitcoin/bitcoin/pull/33919#issuecomment-3560855219)
The workflow correctly attempts to execute the GUI test binary, ./bin/test_bitcoin-qt.exe. but the CI logs do not show any Qt test output and there are no lines such as ********* Start testing of <TestSuite> *********
PASS / FAIL entries
********* Finished testing of <TestSuite> *********
It would be helpful to confirm that test_bitcoin-qt.exe is executing as intended in the cross-Windows environment.
(https://github.com/bitcoin/bitcoin/pull/33919#issuecomment-3560855219)
The workflow correctly attempts to execute the GUI test binary, ./bin/test_bitcoin-qt.exe. but the CI logs do not show any Qt test output and there are no lines such as ********* Start testing of <TestSuite> *********
PASS / FAIL entries
********* Finished testing of <TestSuite> *********
It would be helpful to confirm that test_bitcoin-qt.exe is executing as intended in the cross-Windows environment.
💬 Sammie05 commented on pull request "test: Retry download in get_previous_releases.py":
(https://github.com/bitcoin/bitcoin/pull/33915#issuecomment-3561716043)
I verified the retry mechanism by testing both successful downloads (v25.0) and failure scenarios (invalid_version_123), the retry logic correctly activates on HTTP 404 errors and provides clear error message
Fetching: https://bitcoincore.org/bin/bitcoin-core-nvalid_version_123/bitcoin-nvalid_version_123-osx64.tar.gz
Download failed: HTTP Error 404: Not Found
Retrying download after failure ...
Download failed a second time: HTTP Error 404: Not Found
(https://github.com/bitcoin/bitcoin/pull/33915#issuecomment-3561716043)
I verified the retry mechanism by testing both successful downloads (v25.0) and failure scenarios (invalid_version_123), the retry logic correctly activates on HTTP 404 errors and provides clear error message
Fetching: https://bitcoincore.org/bin/bitcoin-core-nvalid_version_123/bitcoin-nvalid_version_123-osx64.tar.gz
Download failed: HTTP Error 404: Not Found
Retrying download after failure ...
Download failed a second time: HTTP Error 404: Not Found
💬 ajtowns commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2548796423)
I'm working on making it be:
* have node/miner generate two <4MW `CBlock` results, rather than a single <8MW one.
* send a `TEMPLATE` message with both blocks encoded as compact blocks
* request missing txns from the first block; request missing txns from the second block
* receive missing txns from the first block
* receive missing txns from the second block; add the txs from both blocks as that peer's template
Little bit fiddly to make it all work, but I'm not seeing any showst
...
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2548796423)
I'm working on making it be:
* have node/miner generate two <4MW `CBlock` results, rather than a single <8MW one.
* send a `TEMPLATE` message with both blocks encoded as compact blocks
* request missing txns from the first block; request missing txns from the second block
* receive missing txns from the first block
* receive missing txns from the second block; add the txs from both blocks as that peer's template
Little bit fiddly to make it all work, but I'm not seeing any showst
...
💬 maflcko commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2548811262)
It could make sense to ask Ubuntu to import g++-mingw-w64-ucrt64 from Debian, at least for the 26.04 release.
My hope was that one toolchain could be dropped while adding the new one relatively quickly, or even at the same time.
Unrelated, I wonder what all the major benefits of ucrt are. Is it just the x-modifier in fopen calls?
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2548811262)
It could make sense to ask Ubuntu to import g++-mingw-w64-ucrt64 from Debian, at least for the 26.04 release.
My hope was that one toolchain could be dropped while adding the new one relatively quickly, or even at the same time.
Unrelated, I wonder what all the major benefits of ucrt are. Is it just the x-modifier in fopen calls?
💬 janb84 commented on pull request "ci: Enable experimental kernel stuff in most CI tasks via `dev-mode`":
(https://github.com/bitcoin/bitcoin/pull/33824#discussion_r2548815278)
Weird, dev-mode should have those flags on.
Your build is reporting this [config](https://github.com/maflcko/bitcoin-core-nightly/actions/runs/19550555322/job/55988016371#step:6:1874).
```sh
bitcoin-chainstate (experimental) ... ON
libbitcoinkernel (experimental) ..... ON
kernel-test (experimental) .......... ON
```
(https://github.com/bitcoin/bitcoin/pull/33824#discussion_r2548815278)
Weird, dev-mode should have those flags on.
Your build is reporting this [config](https://github.com/maflcko/bitcoin-core-nightly/actions/runs/19550555322/job/55988016371#step:6:1874).
```sh
bitcoin-chainstate (experimental) ... ON
libbitcoinkernel (experimental) ..... ON
kernel-test (experimental) .......... ON
```
💬 hebasto commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2549088989)
> It could make sense to ask Ubuntu to import g++-mingw-w64-ucrt64 from Debian, at least for the 26.04 release.
It appears to be a WIP: https://packages.ubuntu.com/search?keywords=ucrt
> Unrelated, I wonder what all the major benefits of ucrt are. Is it just the x-modifier in fopen calls?
From https://learn.microsoft.com/en-us/cpp/porting/upgrade-your-code-to-the-universal-crt:
> Many functions were added or updated in the UCRT to improve ISO C99 conformance, and to address code qualit
...
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2549088989)
> It could make sense to ask Ubuntu to import g++-mingw-w64-ucrt64 from Debian, at least for the 26.04 release.
It appears to be a WIP: https://packages.ubuntu.com/search?keywords=ucrt
> Unrelated, I wonder what all the major benefits of ucrt are. Is it just the x-modifier in fopen calls?
From https://learn.microsoft.com/en-us/cpp/porting/upgrade-your-code-to-the-universal-crt:
> Many functions were added or updated in the UCRT to improve ISO C99 conformance, and to address code qualit
...
🤔 janb84 reviewed a pull request: "test: Retry download in get_previous_releases.py"
(https://github.com/bitcoin/bitcoin/pull/33915#pullrequestreview-3491910763)
ACK fad06f3bb436a97683e8248bfda1bd0d9998c899
PR adds one retry to download a prev. release.
The single retry is somewhat crude solution but a good first step to solve the intermitted download issues, good KISS sollution. If this isn't enough we can always fallback on more elegant solutions, but this also increases the complexity.
(https://github.com/bitcoin/bitcoin/pull/33915#pullrequestreview-3491910763)
ACK fad06f3bb436a97683e8248bfda1bd0d9998c899
PR adds one retry to download a prev. release.
The single retry is somewhat crude solution but a good first step to solve the intermitted download issues, good KISS sollution. If this isn't enough we can always fallback on more elegant solutions, but this also increases the complexity.
💬 maflcko commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-3562209741)
review ACK 55c6a69f777ac08a14e61b98efea00e5c8f98a5f 🚏
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 55c6a69f777a
...
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-3562209741)
review ACK 55c6a69f777ac08a14e61b98efea00e5c8f98a5f 🚏
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 55c6a69f777a
...
💬 hebasto commented on pull request "Add console commands for clearing output and history":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3562241063)
https://github.com/bitcoin-core/gui/pull/882#issue-3276693433:
> Provides explicit, privacy-orientd ways to clear console history and output without altering default behavior.
> Useful in shared or long-running sessions.
I’m not convinced that building a privacy-focused feature around the idea of a “shared session” is sound. I’m not even sure what exactly qualifies as a "shared session" in this context, and if privacy is the aim, preventing such scenarios altogether seems the more appropr
...
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3562241063)
https://github.com/bitcoin-core/gui/pull/882#issue-3276693433:
> Provides explicit, privacy-orientd ways to clear console history and output without altering default behavior.
> Useful in shared or long-running sessions.
I’m not convinced that building a privacy-focused feature around the idea of a “shared session” is sound. I’m not even sure what exactly qualifies as a "shared session" in this context, and if privacy is the aim, preventing such scenarios altogether seems the more appropr
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2549174545)
Wow! Taken :) plus a comment/note that now the declaration order is now relevant, to prevent somebody reordering those in the future or inserting new members at the beginning.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2549174545)
Wow! Taken :) plus a comment/note that now the declaration order is now relevant, to prevent somebody reordering those in the future or inserting new members at the beginning.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2549185247)
Done, even though `CService{addr}` is also slicing according to https://github.com/isocpp/CppCoreGuidelines/blob/e49158a/CppCoreGuidelines.md#es63-dont-slice.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2549185247)
Done, even though `CService{addr}` is also slicing according to https://github.com/isocpp/CppCoreGuidelines/blob/e49158a/CppCoreGuidelines.md#es63-dont-slice.
💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in most CI tasks via `dev-mode`":
(https://github.com/bitcoin/bitcoin/pull/33824#discussion_r2549200835)
Hmm, this was fixed by setting `DYLD_LIBRARY_PATH`.
Fixed in https://github.com/maflcko/bitcoin-core-nightly/commit/c35885a1904c16918af9602e47a456b15b15e941
(https://github.com/bitcoin/bitcoin/pull/33824#discussion_r2549200835)
Hmm, this was fixed by setting `DYLD_LIBRARY_PATH`.
Fixed in https://github.com/maflcko/bitcoin-core-nightly/commit/c35885a1904c16918af9602e47a456b15b15e941
💬 hebasto commented on pull request "Prevent re-execution of sensitive commands from console history":
(https://github.com/bitcoin-core/gui/pull/909#issuecomment-3562283264)
> Sensitive RPC commands such as `walletpassphrase` or `createwallet`
> have their arguments redacted when stored in the console history.
> Even though their parameters are hidden, these commands could still
> be recalled and executed again, which might lead to unintended or
> harmful actions.
If running the same RPC command twice "might lead to unintended or harmful actions", then the issue should be addressed in that command’s implementation, as it isn’t specific to the GUI console.
...
(https://github.com/bitcoin-core/gui/pull/909#issuecomment-3562283264)
> Sensitive RPC commands such as `walletpassphrase` or `createwallet`
> have their arguments redacted when stored in the console history.
> Even though their parameters are hidden, these commands could still
> be recalled and executed again, which might lead to unintended or
> harmful actions.
If running the same RPC command twice "might lead to unintended or harmful actions", then the issue should be addressed in that command’s implementation, as it isn’t specific to the GUI console.
...
💬 maflcko commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3562292543)
> > Not sure I understand the dependence on #33775?
> > CI is using it's own build, not the guix build right? Also MinGW brings it's own GCC doesn't it?
>
> The CI jobs are more useful when using toolchains with versions similar to those in the Guix script.
I think this is just a doc question. I think the `trixie/g++-mingw-w64-ucrt64` task can be left as-is, without adding a dependency here. In the future, https://github.com/bitcoin/bitcoin/pull/33593 could simply update the comment to cl
...
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3562292543)
> > Not sure I understand the dependence on #33775?
> > CI is using it's own build, not the guix build right? Also MinGW brings it's own GCC doesn't it?
>
> The CI jobs are more useful when using toolchains with versions similar to those in the Guix script.
I think this is just a doc question. I think the `trixie/g++-mingw-w64-ucrt64` task can be left as-is, without adding a dependency here. In the future, https://github.com/bitcoin/bitcoin/pull/33593 could simply update the comment to cl
...
💬 maflcko commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2549232668)
> > It could make sense to ask Ubuntu to import g++-mingw-w64-ucrt64 from Debian, at least for the 26.04 release.
>
> It appears to be a WIP: https://packages.ubuntu.com/search?keywords=ucrt
Ah ok, so maybe someone could open a question/request at https://launchpad.net/ubuntu/+source/gcc-mingw-w64/+bugs, so that the progress can be tracked.
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2549232668)
> > It could make sense to ask Ubuntu to import g++-mingw-w64-ucrt64 from Debian, at least for the 26.04 release.
>
> It appears to be a WIP: https://packages.ubuntu.com/search?keywords=ucrt
Ah ok, so maybe someone could open a question/request at https://launchpad.net/ubuntu/+source/gcc-mingw-w64/+bugs, so that the progress can be tracked.
✅ waketraindev closed a pull request: "Add console commands for clearing output and history"
(https://github.com/bitcoin-core/gui/pull/882)
(https://github.com/bitcoin-core/gui/pull/882)
✅ waketraindev closed a pull request: "Prevent re-execution of sensitive commands from console history"
(https://github.com/bitcoin-core/gui/pull/909)
(https://github.com/bitcoin-core/gui/pull/909)
✅ waketraindev closed an issue: "No way to clear command history in RPC console or reset the console without restarting the node"
(https://github.com/bitcoin-core/gui/issues/897)
(https://github.com/bitcoin-core/gui/issues/897)
💬 maflcko commented on pull request "test: add unit test coverage for the empty leaves path in MerkleComputation":
(https://github.com/bitcoin/bitcoin/pull/33858#issuecomment-3562382383)
lgtm ACK ffcae82a68104c1992964b26c592b62cbca391bf
(https://github.com/bitcoin/bitcoin/pull/33858#issuecomment-3562382383)
lgtm ACK ffcae82a68104c1992964b26c592b62cbca391bf
💬 waketraindev commented on pull request "Add console commands for clearing output and history":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3562385876)
Closed
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3562385876)
Closed