💬 l0rinc commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-3145931019)
Thanks @pstratem, I did almost exactly the same, the main problem is indeed the loop in https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L3498, probably bounded by `BLOCK_DOWNLOAD_WINDOW` - hence the ~1000 worst cases.
I have a fix for most duplications, I'm running an IBD (and a reindex-chainstate) on my benchmarking servers to see the effect of the deduplications. I expect at most a 1-2% change, but it may still be worth it.
I'll also investigate the effect of lowering `BLOC
...
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-3145931019)
Thanks @pstratem, I did almost exactly the same, the main problem is indeed the loop in https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L3498, probably bounded by `BLOCK_DOWNLOAD_WINDOW` - hence the ~1000 worst cases.
I have a fix for most duplications, I'm running an IBD (and a reindex-chainstate) on my benchmarking servers to see the effect of the deduplications. I expect at most a 1-2% change, but it may still be worth it.
I'll also investigate the effect of lowering `BLOC
...
📝 kevkevinpal opened a pull request: "wallet: remove use of GetWalletDIr and weakly_canonical when creating backup_prefix"
(https://github.com/bitcoin/bitcoin/pull/33122)
This is a followup to https://github.com/bitcoin/bitcoin/pull/32273
This updates the logic of backup_prefix to not use `GetWalletDir` and `weakly_canonical`
17e318a2f125eab61e33b58d869c1035d66c08e4 addresses https://github.com/bitcoin/bitcoin/pull/32273/files#r2237028524
This removes duplicated code in `test/functional/wallet_migration.py`
9c2abe57f899fd6c756e2e660abbfcf321f52822 addresses https://github.com/bitcoin/bitcoin/pull/32273/files#r2237317368
(https://github.com/bitcoin/bitcoin/pull/33122)
This is a followup to https://github.com/bitcoin/bitcoin/pull/32273
This updates the logic of backup_prefix to not use `GetWalletDir` and `weakly_canonical`
17e318a2f125eab61e33b58d869c1035d66c08e4 addresses https://github.com/bitcoin/bitcoin/pull/32273/files#r2237028524
This removes duplicated code in `test/functional/wallet_migration.py`
9c2abe57f899fd6c756e2e660abbfcf321f52822 addresses https://github.com/bitcoin/bitcoin/pull/32273/files#r2237317368
💬 achow101 commented on pull request "refactor: Use immediate lambda to work around GCC bug 117966":
(https://github.com/bitcoin/bitcoin/pull/33113#issuecomment-3145944477)
ACK c7a24c3052557c4747eb8be7c40ee0a30e6b137d
(https://github.com/bitcoin/bitcoin/pull/33113#issuecomment-3145944477)
ACK c7a24c3052557c4747eb8be7c40ee0a30e6b137d
💬 achow101 commented on pull request "test: fix anti-fee-sniping off-by-one error":
(https://github.com/bitcoin/bitcoin/pull/33118#issuecomment-3145944975)
ACK e07e2532b4d7b4a549722e56cb2913a6081ccbaf
(https://github.com/bitcoin/bitcoin/pull/33118#issuecomment-3145944975)
ACK e07e2532b4d7b4a549722e56cb2913a6081ccbaf
👍 pablomartin4btc approved a pull request: "wallet: Set descriptor cache upgraded flag for migrated wallets"
(https://github.com/bitcoin/bitcoin/pull/33031#pullrequestreview-3080735429)
ACK 88b0647f027a608acb61ec32329d19f8e5b0a9fd
It makes sense to set the flag as soon as the DB is created during migration and not leaving it to the reload of the, later migrated, wallet (in `void CWallet::UpgradeDescriptorCache()`) at the end of the entire process.
(https://github.com/bitcoin/bitcoin/pull/33031#pullrequestreview-3080735429)
ACK 88b0647f027a608acb61ec32329d19f8e5b0a9fd
It makes sense to set the flag as soon as the DB is created during migration and not leaving it to the reload of the, later migrated, wallet (in `void CWallet::UpgradeDescriptorCache()`) at the end of the entire process.
💬 kevkevinpal commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2249000910)
addressing this in https://github.com/bitcoin/bitcoin/pull/33122
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2249000910)
addressing this in https://github.com/bitcoin/bitcoin/pull/33122
💬 kevkevinpal commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2249000995)
addressing this in https://github.com/bitcoin/bitcoin/pull/33122
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2249000995)
addressing this in https://github.com/bitcoin/bitcoin/pull/33122
💬 pablomartin4btc commented on pull request "wallet: remove use of GetWalletDir and weakly_canonical when creating backup_prefix":
(https://github.com/bitcoin/bitcoin/pull/33122#discussion_r2249001617)
nit: perhaps you can add the `wallet_path` in the log? otherwise it would be seen as duplicated...
(https://github.com/bitcoin/bitcoin/pull/33122#discussion_r2249001617)
nit: perhaps you can add the `wallet_path` in the log? otherwise it would be seen as duplicated...
🤔 pablomartin4btc reviewed a pull request: "wallet: remove use of GetWalletDir and weakly_canonical when creating backup_prefix"
(https://github.com/bitcoin/bitcoin/pull/33122#pullrequestreview-3080766829)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33122#pullrequestreview-3080766829)
Concept ACK
💬 kevkevinpal commented on pull request "wallet: remove use of GetWalletDir and weakly_canonical when creating backup_prefix":
(https://github.com/bitcoin/bitcoin/pull/33122#discussion_r2249010323)
yea sure thing updated in 6ac5a351c99f0c0f1b80d179fc845fa55973c874
```
2025-08-01T23:29:43.011985Z TestFramework (INFO): Test migrating a wallet with the following path/name: path/to/mywallet/
2025-08-01T23:29:43.072030Z TestFramework (INFO): Test migrating a wallet with the following path/name: path/that/ends/in/..
```
(https://github.com/bitcoin/bitcoin/pull/33122#discussion_r2249010323)
yea sure thing updated in 6ac5a351c99f0c0f1b80d179fc845fa55973c874
```
2025-08-01T23:29:43.011985Z TestFramework (INFO): Test migrating a wallet with the following path/name: path/to/mywallet/
2025-08-01T23:29:43.072030Z TestFramework (INFO): Test migrating a wallet with the following path/name: path/that/ends/in/..
```
💬 D33r-Gee commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#discussion_r2249012242)
Thanks for the review... will update soon addressing this
(https://github.com/bitcoin/bitcoin/pull/33117#discussion_r2249012242)
Thanks for the review... will update soon addressing this
🤔 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
...