💬 ryanofsky commented on pull request "[DRAFT] ipc: add windows support":
(https://github.com/bitcoin/bitcoin/pull/32387#issuecomment-2848129853)
Updated 27328195136754babe8d8f5d99f4f0b6a5ab2e55 -> f215e742045401ab386f0cd67d06b358558ecf89 ([`pr/ipc-win.4`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-win.4) -> [`pr/ipc-win.5`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-win.5), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-win.4..pr/ipc-win.5)) fixing many windows bugs.
With this update, IPC code is mostly working on windows: `mpexample` and `mptest` programs work, `test_bitcoin` IPC calls over socketpair
...
  (https://github.com/bitcoin/bitcoin/pull/32387#issuecomment-2848129853)
Updated 27328195136754babe8d8f5d99f4f0b6a5ab2e55 -> f215e742045401ab386f0cd67d06b358558ecf89 ([`pr/ipc-win.4`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-win.4) -> [`pr/ipc-win.5`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-win.5), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-win.4..pr/ipc-win.5)) fixing many windows bugs.
With this update, IPC code is mostly working on windows: `mpexample` and `mptest` programs work, `test_bitcoin` IPC calls over socketpair
...
🤔 w0xlt reviewed a pull request: "Refactor BnB tests"
(https://github.com/bitcoin/bitcoin/pull/29532#pullrequestreview-2812922420)
ACK https://github.com/bitcoin/bitcoin/pull/29532/commits/85368aafa0d1772626d5f615e272b1c1a580b42f
  (https://github.com/bitcoin/bitcoin/pull/29532#pullrequestreview-2812922420)
ACK https://github.com/bitcoin/bitcoin/pull/29532/commits/85368aafa0d1772626d5f615e272b1c1a580b42f
💬 donaldevine commented on something "":
(https://github.com/bitcoin/bitcoin/commit/3ba7449f6c335026b752366c53f9c309f09e6c64#commitcomment-156304878)
Allowing multiple OP_RETURN Outputs in a single transaction will have serious implications for Node Resources including higher memory requirements during transaction validation and more complex relay policies. Allowing multiple OP_RETURN outputs represents a relaxation of data storage policies that will have massive downstream effects. NACK
  (https://github.com/bitcoin/bitcoin/commit/3ba7449f6c335026b752366c53f9c309f09e6c64#commitcomment-156304878)
Allowing multiple OP_RETURN Outputs in a single transaction will have serious implications for Node Resources including higher memory requirements during transaction validation and more complex relay policies. Allowing multiple OP_RETURN outputs represents a relaxation of data storage policies that will have massive downstream effects. NACK
💬 kevkevinpal commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072235911)
Do you think the summary would still be useful if you're running `--loglevel=DEBUG`?
I could put an if block around it so we still get the summary for debug mode at the end.
I agree though for `logging.info` and above it seems pretty redundant
  (https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072235911)
Do you think the summary would still be useful if you're running `--loglevel=DEBUG`?
I could put an if block around it so we still get the summary for debug mode at the end.
I agree though for `logging.info` and above it seems pretty redundant
💬 kevkevinpal commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072240638)
no there is not, pushed [c54df63](https://github.com/bitcoin/bitcoin/pull/32354/commits/c54df63f7d5047bf8efc345f102942cbb86fb019)
  (https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072240638)
no there is not, pushed [c54df63](https://github.com/bitcoin/bitcoin/pull/32354/commits/c54df63f7d5047bf8efc345f102942cbb86fb019)
💬 kevkevinpal commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072240720)
I just made into a debug log in [c54df63](https://github.com/bitcoin/bitcoin/pull/32354/commits/c54df63f7d5047bf8efc345f102942cbb86fb019)
  (https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072240720)
I just made into a debug log in [c54df63](https://github.com/bitcoin/bitcoin/pull/32354/commits/c54df63f7d5047bf8efc345f102942cbb86fb019)
💬 achow101 commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2848316721)
ACK 65fcfbb2b38bef20a58daa6c828c51890180611d
  (https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2848316721)
ACK 65fcfbb2b38bef20a58daa6c828c51890180611d
💬 achow101 commented on pull request "wallet: Translate [default wallet] string in progress messages":
(https://github.com/bitcoin/bitcoin/pull/31296#issuecomment-2848318178)
ACK a46ec1dece8d8a1b16ff1fc3d2932bca130bf67f
  (https://github.com/bitcoin/bitcoin/pull/31296#issuecomment-2848318178)
ACK a46ec1dece8d8a1b16ff1fc3d2932bca130bf67f
💬 achow101 commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-2848323697)
Approach NACK
While I agree that we should support v3 transactions, I think this implementation is incomplete without any v3 support in the wallet's coin selection. As it is now, this appears to enable a massive footgun - users can use `createrawtransaction` to create a v3 transaction, fund it with `fundrawtransaction`, and then wind up with a v3 transaction that they cannot broadcast. This is because the topological restrictions of v3 transactions are not being taken into account during coin
...
  (https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-2848323697)
Approach NACK
While I agree that we should support v3 transactions, I think this implementation is incomplete without any v3 support in the wallet's coin selection. As it is now, this appears to enable a massive footgun - users can use `createrawtransaction` to create a v3 transaction, fund it with `fundrawtransaction`, and then wind up with a v3 transaction that they cannot broadcast. This is because the topological restrictions of v3 transactions are not being taken into account during coin
...
💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2848379263)
> Crashed with long time spacing (are you sure you need 2^64 seconds?)
>
> `build/bin/bitcoind -signet -signetchallenge=6a4c09011effff00000000004c0151 -debug -debugexclude=libevent`
>
> `bitcoin-cli -signet -generate`
>
> log:
>
> ```
>
> 2025-05-02T19:18:23Z [http] Received a POST request for / from 127.0.0.1:39282
> 2025-05-02T19:18:23Z [rpc] ThreadRPCServer method=getnewaddress user=__cookie__
> 2025-05-02T19:18:24Z [http] Received a POST request for / from 127.0.0.1:39284
>
...
  (https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2848379263)
> Crashed with long time spacing (are you sure you need 2^64 seconds?)
>
> `build/bin/bitcoind -signet -signetchallenge=6a4c09011effff00000000004c0151 -debug -debugexclude=libevent`
>
> `bitcoin-cli -signet -generate`
>
> log:
>
> ```
>
> 2025-05-02T19:18:23Z [http] Received a POST request for / from 127.0.0.1:39282
> 2025-05-02T19:18:23Z [rpc] ThreadRPCServer method=getnewaddress user=__cookie__
> 2025-05-02T19:18:24Z [http] Received a POST request for / from 127.0.0.1:39284
>
...
💬 shahsb commented on pull request "fuzz: Suppress abort message on Windows":
(https://github.com/bitcoin/bitcoin/pull/32409#discussion_r2072293276)
```suggestion
#ifdef _WIN32
```
`_WIN32` is a compiler flag and could be used for both windows platform i.e windows_x86 (32-bit) as well as windows_x64 (64-bit)
  (https://github.com/bitcoin/bitcoin/pull/32409#discussion_r2072293276)
```suggestion
#ifdef _WIN32
```
`_WIN32` is a compiler flag and could be used for both windows platform i.e windows_x86 (32-bit) as well as windows_x64 (64-bit)
💬 shahsb commented on pull request "fuzz: Suppress abort message on Windows":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2848379451)
Concept ACK
  (https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2848379451)
Concept ACK
💬 monlovesmango commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072307529)
I think it should be this:
```suggestion
num_targets_completed = log_fuzz_progress(num_targets_completed, len(targets), done_stat, target, target_max_len)
```
My preference would probably change the argument `targets` to `test_list` (and also update the name in args.generate) because this is more consistent with the other modes. However I understand if this is out of scope.
  (https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072307529)
I think it should be this:
```suggestion
num_targets_completed = log_fuzz_progress(num_targets_completed, len(targets), done_stat, target, target_max_len)
```
My preference would probably change the argument `targets` to `test_list` (and also update the name in args.generate) because this is more consistent with the other modes. However I understand if this is out of scope.
💬 kevkevinpal commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072312807)
I actually had to update this since `test_list` wasnt defined in this function, so I just got the `num_of_targets` at the top and used it here [3af8f62](https://github.com/bitcoin/bitcoin/pull/32354/commits/3af8f625fa1087fc659f5cbf0c5f568f15d89ee1)
lemme know if that looks good!
  (https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072312807)
I actually had to update this since `test_list` wasnt defined in this function, so I just got the `num_of_targets` at the top and used it here [3af8f62](https://github.com/bitcoin/bitcoin/pull/32354/commits/3af8f625fa1087fc659f5cbf0c5f568f15d89ee1)
lemme know if that looks good!
💬 monlovesmango commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072319868)
lgtm!
  (https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2072319868)
lgtm!
💬 monlovesmango commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-2848432297)
ACK 3af8f625fa1087fc659f5cbf0c5f568f15d89ee1
  (https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-2848432297)
ACK 3af8f625fa1087fc659f5cbf0c5f568f15d89ee1
💬 hebasto commented on pull request "fuzz: Suppress abort message on Windows":
(https://github.com/bitcoin/bitcoin/pull/32409#discussion_r2072334684)
The current code is consistent with our entire code base.
  (https://github.com/bitcoin/bitcoin/pull/32409#discussion_r2072334684)
The current code is consistent with our entire code base.
⚠️ QuantumAlchemist03 opened an issue: "Enhancement: Persist User Settings Across Sessions in Bitcoin Core GUI"
(https://github.com/bitcoin/bitcoin/issues/32410)
Currently, the Bitcoin Core GUI does not retain user preferences such as window size, position, and selected tabs between sessions. This requires users to reconfigure their interface each time they launch the application, which can be inconvenient.
  (https://github.com/bitcoin/bitcoin/issues/32410)
Currently, the Bitcoin Core GUI does not retain user preferences such as window size, position, and selected tabs between sessions. This requires users to reconfigure their interface each time they launch the application, which can be inconvenient.
💬 QuantumAlchemist03 commented on issue "Enhancement: Persist User Settings Across Sessions in Bitcoin Core GUI":
(https://github.com/bitcoin/bitcoin/issues/32410#issuecomment-2848488817)
Implement a mechanism to save user interface settings upon application exit and restore them upon startup. This could involve storing preferences in a configuration file or utilizing platform-specific settings storage.
  (https://github.com/bitcoin/bitcoin/issues/32410#issuecomment-2848488817)
Implement a mechanism to save user interface settings upon application exit and restore them upon startup. This could involve storing preferences in a configuration file or utilizing platform-specific settings storage.
✅ QuantumAlchemist03 closed an issue: "Enhancement: Persist User Settings Across Sessions in Bitcoin Core GUI"
(https://github.com/bitcoin/bitcoin/issues/32410)
  (https://github.com/bitcoin/bitcoin/issues/32410)
💬 romanz commented on pull request "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)":
(https://github.com/bitcoin/bitcoin/pull/32305#issuecomment-2848492497)
tACK 7d301184016a3f59c2e363dff631263cdbe21da0
Tested with both https://github.com/romanz/electrs and https://github.com/romanz/bindex-rs.
  (https://github.com/bitcoin/bitcoin/pull/32305#issuecomment-2848492497)
tACK 7d301184016a3f59c2e363dff631263cdbe21da0
Tested with both https://github.com/romanz/electrs and https://github.com/romanz/bindex-rs.