💬 ryanofsky commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#discussion_r2248934018)
In commit "interfaces: expose UTXO snapshot loading in node interface" (de11e3b89f8aba9a90a3419086807e5fee0f941d)
Note with #10102 if the GUI & node are running in separate processes, it could be a awkward for the gui process to create an AutoFile object and pass it to the node like this. Technically it could probably work: the AutoFile could be passed as a file descriptor or a (path, position) tuple. But it doesn't seem like a clean separation of responsibilities if the GUI process is readin
...
(https://github.com/bitcoin/bitcoin/pull/33117#discussion_r2248934018)
In commit "interfaces: expose UTXO snapshot loading in node interface" (de11e3b89f8aba9a90a3419086807e5fee0f941d)
Note with #10102 if the GUI & node are running in separate processes, it could be a awkward for the gui process to create an AutoFile object and pass it to the node like this. Technically it could probably work: the AutoFile could be passed as a file descriptor or a (path, position) tuple. But it doesn't seem like a clean separation of responsibilities if the GUI process is readin
...
💬 pstratem commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-3145893262)
We're calling CBlockHeader::GetHash hundreds of times for the same CBlockHeader object from ActivateBestChain.
I chased the calls with this commit and found all the results where from ProcessNewBlock ActivateBestChain
https://github.com/pstratem/bitcoin/commit/7de23309167d3f713f90cc552e1bd431464e4c49
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-3145893262)
We're calling CBlockHeader::GetHash hundreds of times for the same CBlockHeader object from ActivateBestChain.
I chased the calls with this commit and found all the results where from ProcessNewBlock ActivateBestChain
https://github.com/pstratem/bitcoin/commit/7de23309167d3f713f90cc552e1bd431464e4c49
📝 mzumsande opened a pull request: "test: fix p2p_leak_tx.py"
(https://github.com/bitcoin/bitcoin/pull/33121)
This fixes two issues with `p2p_leak_tx.py`:
1.) #33090: As far as I can see, this is just the randomness of `NextInvToInbounds`/ `rand_exp_duration`, which has a probability of `e^-(60s/5s) = 6.14×10^−6` to result in a period > 60s (our waiting time), so that the test would fail every 160k runs... Doubling the timeout should be sufficient to lower the probability drastically.
2.) The subtest `test_notfound_on_unannounced_tx` has some (w)txid confusion: we send a `MSG_TX`-type getdata with
...
(https://github.com/bitcoin/bitcoin/pull/33121)
This fixes two issues with `p2p_leak_tx.py`:
1.) #33090: As far as I can see, this is just the randomness of `NextInvToInbounds`/ `rand_exp_duration`, which has a probability of `e^-(60s/5s) = 6.14×10^−6` to result in a period > 60s (our waiting time), so that the test would fail every 160k runs... Doubling the timeout should be sufficient to lower the probability drastically.
2.) The subtest `test_notfound_on_unannounced_tx` has some (w)txid confusion: we send a `MSG_TX`-type getdata with
...
💬 mzumsande commented on issue "test: spurious failure in p2p_leak_tx.py --v1transport":
(https://github.com/bitcoin/bitcoin/issues/33090#issuecomment-3145915708)
See #33121 for a fix and an explanation attempt.
(https://github.com/bitcoin/bitcoin/issues/33090#issuecomment-3145915708)
See #33121 for a fix and an explanation attempt.
💬 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)