π€ pablomartin4btc reviewed a pull request: "doc: Fix 'getdescriptoractivity' RPCHelpMan"
(https://github.com/bitcoin/bitcoin/pull/33119#pullrequestreview-3080781894)
cr ACK b17918d66743d6ee1cd2df7fd0a6c05dc87f71d8
Maybe you can add the field validation in `rpc_getdescriptoractivity.py` (ie `test_receive_then_spend`).
(https://github.com/bitcoin/bitcoin/pull/33119#pullrequestreview-3080781894)
cr ACK b17918d66743d6ee1cd2df7fd0a6c05dc87f71d8
Maybe you can add the field validation in `rpc_getdescriptoractivity.py` (ie `test_receive_then_spend`).
π€ furszy reviewed a pull request: "index: fix wrong assert of current_tip == m_best_block_index"
(https://github.com/bitcoin/bitcoin/pull/32878#pullrequestreview-3080784277)
ACK 3aef38f
Test fails without the fix commit and passes with it.
(https://github.com/bitcoin/bitcoin/pull/32878#pullrequestreview-3080784277)
ACK 3aef38f
Test fails without the fix commit and passes with it.
π¬ achow101 commented on pull request "wallet: Set descriptor cache upgraded flag for migrated wallets":
(https://github.com/bitcoin/bitcoin/pull/33031#discussion_r2249018616)
Multiple import of the same module is a no-op, it uses whatever is already imported.
(https://github.com/bitcoin/bitcoin/pull/33031#discussion_r2249018616)
Multiple import of the same module is a no-op, it uses whatever is already imported.
β οΈ byLAEV opened an issue: "By LAEV y El Cartel de Bitcoiners #carteldebitcoiners #hanbot #laev #mirceapopescu "
(https://github.com/bitcoin/bitcoin/issues/33123)
I am from Costa Rica from San Ramon de Alajuela, I clarify that I have never received or asked for 5k or 10k of bitcoin from anyone and they have never sent me bitcoins from anyone or through hanbot... Hannah wiggins aka hanbot must be clear about that situation write to the mercitudinous@protonmail.com by LAEV lerry alexander Elizondo Villalobos
(https://github.com/bitcoin/bitcoin/issues/33123)
I am from Costa Rica from San Ramon de Alajuela, I clarify that I have never received or asked for 5k or 10k of bitcoin from anyone and they have never sent me bitcoins from anyone or through hanbot... Hannah wiggins aka hanbot must be clear about that situation write to the mercitudinous@protonmail.com by LAEV lerry alexander Elizondo Villalobos
β
fanquake closed an issue: "By LAEV y El Cartel de Bitcoiners #carteldebitcoiners #hanbot #laev #mirceapopescu "
(https://github.com/bitcoin/bitcoin/issues/33123)
(https://github.com/bitcoin/bitcoin/issues/33123)
π€ murchandamus reviewed a pull request: "test: add assertions to SRD max weight test"
(https://github.com/bitcoin/bitcoin/pull/33058#pullrequestreview-3080840430)
ACK cc33e4578946c68d6d333f35c48f8cbf3f75f6cf
> This test case has a number of invaluable utxos and some valuable ones. Would it not be better to show that after the min-heap is added, that the implementation has a bias towards selecting valuable utxos? I think these types of implementation details would be valuable to test.
No, a good test checks that the tested function produces the expected outcomes without relying on knowledge of implementation details.
In this case, the relevant feature
...
(https://github.com/bitcoin/bitcoin/pull/33058#pullrequestreview-3080840430)
ACK cc33e4578946c68d6d333f35c48f8cbf3f75f6cf
> This test case has a number of invaluable utxos and some valuable ones. Would it not be better to show that after the min-heap is added, that the implementation has a bias towards selecting valuable utxos? I think these types of implementation details would be valuable to test.
No, a good test checks that the tested function produces the expected outcomes without relying on knowledge of implementation details.
In this case, the relevant feature
...
π¬ Sammie05 commented on pull request "doc: Fix 'getdescriptoractivity' RPCHelpMan":
(https://github.com/bitcoin/bitcoin/pull/33119#issuecomment-3146165616)
Thanks for the fix, As mentioned above, adding a test for this in rpc_getdescriptoractivity.py would make the PR even more robust.
ACK from me for this doc update
(https://github.com/bitcoin/bitcoin/pull/33119#issuecomment-3146165616)
Thanks for the fix, As mentioned above, adding a test for this in rpc_getdescriptoractivity.py would make the PR even more robust.
ACK from me for this doc update
π€ pablomartin4btc reviewed a pull request: "qt: clear command history when clearing the console"
(https://github.com/bitcoin-core/gui/pull/882#pullrequestreview-3080917778)
I don't think this change is practical. Clearing the console should remove the visible output, but it shouldn't also clear the command history. In most terminal environments (e.g., bash, zsh, cmd, PowerShell, etc.), using clear or Ctrl+L clears the screen while preserving command history, which remains accessible via the arrow keys. Users often rely on this history to repeat or modify previous commands, and removing it could unintentionally disrupt that workflow. I suggest keeping the command hi
...
(https://github.com/bitcoin-core/gui/pull/882#pullrequestreview-3080917778)
I don't think this change is practical. Clearing the console should remove the visible output, but it shouldn't also clear the command history. In most terminal environments (e.g., bash, zsh, cmd, PowerShell, etc.), using clear or Ctrl+L clears the screen while preserving command history, which remains accessible via the arrow keys. Users often rely on this history to repeat or modify previous commands, and removing it could unintentionally disrupt that workflow. I suggest keeping the command hi
...
π€ BrandonOdiwuor reviewed a pull request: "cmake: Switch to generated `ts_files.cmake` file"
(https://github.com/bitcoin/bitcoin/pull/33115#pullrequestreview-3080938233)
Code Review ACK f2bfe852236f45b9ea81d6b3f06c6a0097be3ddd
Environment: macOS 15.5, Qt 6.9.0
Build & GUI Test: Built successfully; GUI tested with generated ts_files.cmake via updated update_translations.py. No errors.
Translation Test: Kiswahili (sw) loaded correctly in GUI. Screenshot:
<img width="1098" height="559" alt="Screenshot 2025-08-02 at 09 02 04" src="https://github.com/user-attachments/assets/265a5447-be34-4eeb-94ca-2e9d37ac1cb3" />
Also tested the updated `update_transalt
...
(https://github.com/bitcoin/bitcoin/pull/33115#pullrequestreview-3080938233)
Code Review ACK f2bfe852236f45b9ea81d6b3f06c6a0097be3ddd
Environment: macOS 15.5, Qt 6.9.0
Build & GUI Test: Built successfully; GUI tested with generated ts_files.cmake via updated update_translations.py. No errors.
Translation Test: Kiswahili (sw) loaded correctly in GUI. Screenshot:
<img width="1098" height="559" alt="Screenshot 2025-08-02 at 09 02 04" src="https://github.com/user-attachments/assets/265a5447-be34-4eeb-94ca-2e9d37ac1cb3" />
Also tested the updated `update_transalt
...
π¬ maflcko commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2249160955)
There are still some spacing issues here. See also the report https://github.com/maflcko/b-c-gui-translations-review/blob/ab6dc43a4c543c072d916a268a0bc34562ce3f81/README.md
cc https://github.com/maflcko/DrahtBot/issues/49
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2249160955)
There are still some spacing issues here. See also the report https://github.com/maflcko/b-c-gui-translations-review/blob/ab6dc43a4c543c072d916a268a0bc34562ce3f81/README.md
cc https://github.com/maflcko/DrahtBot/issues/49
π¬ waketraindev commented on pull request "qt: clear command history when clearing the console":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3146305007)
> I don't think this change is practical. Clearing the console should remove the visible output, but it shouldn't also clear the command history. In most terminal environments (e.g., bash, zsh, cmd, PowerShell, etc.), using clear or Ctrl+L clears the screen while preserving command history, which remains accessible via the arrow keys. Users often rely on this history to repeat or modify previous commands, and removing it could unintentionally disrupt that workflow. I suggest keeping the command
...
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3146305007)
> I don't think this change is practical. Clearing the console should remove the visible output, but it shouldn't also clear the command history. In most terminal environments (e.g., bash, zsh, cmd, PowerShell, etc.), using clear or Ctrl+L clears the screen while preserving command history, which remains accessible via the arrow keys. Users often rely on this history to repeat or modify previous commands, and removing it could unintentionally disrupt that workflow. I suggest keeping the command
...
π¬ waketraindev commented on pull request "qt: clear command history when clearing the console":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3146310289)
If there is concern about preserving history by default, a compromise could be to clear command history only when holding Shift while clicking the "Clear" button, similar to how modifier keys are used in other GUIs for alternate actions. But personally, my expectation as a user is that "Clear" fully resets the console, including input history.
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3146310289)
If there is concern about preserving history by default, a compromise could be to clear command history only when holding Shift while clicking the "Clear" button, similar to how modifier keys are used in other GUIs for alternate actions. But personally, my expectation as a user is that "Clear" fully resets the console, including input history.
π¬ jesterhodl commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2249174462)
Thanks, I'll review those
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2249174462)
Thanks, I'll review those
π chareice opened a pull request: "wallet: Improve wallet creation error message with usage hint"
(https://github.com/bitcoin/bitcoin/pull/33124)
Improve wallet creation error message with usage hint
(https://github.com/bitcoin/bitcoin/pull/33124)
Improve wallet creation error message with usage hint
π¬ maflcko commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2249189479)
this has to be guarded, see e.g. the `logthreadnames` below
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2249189479)
this has to be guarded, see e.g. the `logthreadnames` below
π¬ maflcko commented on pull request "wallet: remove use of GetWalletDir and weakly_canonical when creating backup_prefix":
(https://github.com/bitcoin/bitcoin/pull/33122#discussion_r2249190191)
walletname -> wallet name [compound term should be two words for clarity]
if contains slashes -> if it contains slashes [missing pronoun βitβ]
(https://github.com/bitcoin/bitcoin/pull/33122#discussion_r2249190191)
walletname -> wallet name [compound term should be two words for clarity]
if contains slashes -> if it contains slashes [missing pronoun βitβ]
π¬ fanquake commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3146425091)
MSAN failure (https://cirrus-ci.com/task/6435705422872576?logs=ci#L1863) is #33114.
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3146425091)
MSAN failure (https://cirrus-ci.com/task/6435705422872576?logs=ci#L1863) is #33114.
π fanquake opened a pull request: "ci: Use mlc `v1` and ignore `depends/work`"
(https://github.com/bitcoin/bitcoin/pull/33125)
Otherwise you might see failures like this, if there are build artifacts in your depends dir:
```bash
[Err ] ./depends/work/build/x86_64-w64-mingw32/qt/6.7.3-64961f1d77c/qtbase/tests/auto/corelib/serialization/qxmlstream/XML-Test-Suite/xmlconf/xmlconf-20031030.htm (39, 55) => /Member/#confidential - Target not found.
[Err ] ./depends/work/build/x86_64-w64-mingw32/qt/6.7.3-64961f1d77c/qtbase/tests/auto/corelib/serialization/qxmlstream/XML-Test-Suite/xmlconf/xmlconf-20020521.htm (39, 55) => /Me
...
(https://github.com/bitcoin/bitcoin/pull/33125)
Otherwise you might see failures like this, if there are build artifacts in your depends dir:
```bash
[Err ] ./depends/work/build/x86_64-w64-mingw32/qt/6.7.3-64961f1d77c/qtbase/tests/auto/corelib/serialization/qxmlstream/XML-Test-Suite/xmlconf/xmlconf-20031030.htm (39, 55) => /Member/#confidential - Target not found.
[Err ] ./depends/work/build/x86_64-w64-mingw32/qt/6.7.3-64961f1d77c/qtbase/tests/auto/corelib/serialization/qxmlstream/XML-Test-Suite/xmlconf/xmlconf-20020521.htm (39, 55) => /Me
...
π¬ yancyribbens commented on pull request "test: add assertions to SRD max weight test":
(https://github.com/bitcoin/bitcoin/pull/33058#issuecomment-3146449310)
> A test being based on knowing that SRD uses the min-heap approach internally would be 1) more complicated, 2) harder to read, 3) more error prone, and 4) a layer violation.
I'm not advocating to test that SRD uses a min-heap. However, the min-heap was added https://github.com/bitcoin/bitcoin/pull/26720 because:
> As the selection of the 1515 small UTXOs exceeds the max allowed tx size, the expectation here is to receive a selection result that only contain the big UTXO. Which is not hap
...
(https://github.com/bitcoin/bitcoin/pull/33058#issuecomment-3146449310)
> A test being based on knowing that SRD uses the min-heap approach internally would be 1) more complicated, 2) harder to read, 3) more error prone, and 4) a layer violation.
I'm not advocating to test that SRD uses a min-heap. However, the min-heap was added https://github.com/bitcoin/bitcoin/pull/26720 because:
> As the selection of the 1515 small UTXOs exceeds the max allowed tx size, the expectation here is to receive a selection result that only contain the big UTXO. Which is not hap
...
π Ataraxia009 opened a pull request: "Allowing multi client support in guix-build"
(https://github.com/bitcoin/bitcoin/pull/33126)
Using `CLIENT_NAME` variable in guix build files to better support forked clients
(https://github.com/bitcoin/bitcoin/pull/33126)
Using `CLIENT_NAME` variable in guix build files to better support forked clients