Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 musaHaruna commented on pull request "wallet, rpc: add UTXO set check and incremental rescan to importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553066371)
The UTXO set check is performed before the main rescan to attempt incremental reconciliation of missing funds in small block chunks, potentially reducing the amount of blocks that need to be rescanned in the case of a large blockchain. Doing it after the main rescan would generally be redundant, because the rescan will already incorporate transactions missed due to incorrect timestamps, and any discrepancy with the UTXO set would likely have been resolved. The current order allows us to detect a
...
💬 musaHaruna commented on pull request "wallet, rpc: add UTXO set check and incremental rescan to importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553068563)
Yeah I agree I have changed it to ` verify_balance` I thought just `verify` is till a bit vague
💬 hebasto commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3566611864)
All conflicts caused by the recent rebase have been fixed, and CI is green now.
💬 musaHaruna commented on pull request "wallet, rpc: add UTXO set check and incremental rescan to importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553075650)
Fixed
💬 musaHaruna commented on pull request "wallet, rpc: add UTXO set check and incremental rescan to importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553076297)
Fixed
💬 musaHaruna commented on pull request "wallet, rpc: add UTXO set check and incremental rescan to importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553076860)
Fixed
💬 musaHaruna commented on pull request "wallet, rpc: add UTXO set check and incremental rescan to importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553077585)
Fixed
💬 musaHaruna commented on pull request "wallet, rpc: add UTXO set check and incremental rescan to importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553095074)
Yeah, I think it should be, I will look into how I can do that, am open to suggestions on how that can be done!
💬 yuvicc commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#issuecomment-3566677137)
Rebased to fix the CI error. For reference check https://github.com/bitcoin/bitcoin/issues/33884.
💬 musaHaruna commented on pull request "wallet, rpc: add UTXO set check and incremental rescan to importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553121377)
This assertion ensures that the newly created watch-only wallets start from a completely clean state with no prior transaction history or birthtime. That way, we can be sure that any transactions discovered after the import come solely from the descriptor import and the UTXO scan, not from pre-existing wallet data.
💬 musaHaruna commented on pull request "wallet, rpc: add UTXO set check and incremental rescan to importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553122533)
I think this is part of backupwallet RPC and I don't think I touch the backupwallet rpc codenthe entire PR.
👍 benthecarman approved a pull request: "Revert "gui, qt: brintToFront workaround for Wayland""
(https://github.com/bitcoin-core/gui/pull/914#pullrequestreview-3496739218)
fixes for me!
🤔 l0rinc reviewed a pull request: "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count"
(https://github.com/bitcoin/bitcoin/pull/32497#pullrequestreview-3496417072)
Latest push:
* rebases against latest master;
* reverts code back to simply change `leaves.resize(block.vtx.size())` constructs before calling `ComputeMerkleRoot` to `.reserve((block.vtx.size() + 1) & ~1ULL)`;
** added godbolt reproducer for each important combination of compiler and architecture
* Separates the test changes to a separate commit explaining the motivation in the commit message
* updated commit messages and PR descriptions.
💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2552816773)
We're comparing it against the results of `BlockWitnessMerkleRoot` which returns `GetWitnessHash`.
https://github.com/bitcoin/bitcoin/pull/16865 didn't specify why they're comparing `GetHash` against `GetWitnessHash` here, I deduced it's a coincidence. I don't mind keeping `GetHash` if you think it was deliberate.
💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2553107836)
> Accessing txs.size() one time less at runtime

The compiler should be able to deduce that it's the same (see assembly below): https://en.wikipedia.org/wiki/Common_subexpression_elimination

-----

I have considered a few alternatives:
```C++
BOOST_AUTO_TEST_CASE(merkle_test_even_sizing)
{
for (size_t size = 0; size <= 1'000'000; ++size) {
size_t reference = size + (size & 1U);

BOOST_CHECK_EQUAL(reference, (size + 1) & ~size_t{1});
BOOST_CHECK_EQUAL(re
...
💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2553128138)
Thanks, ended up reverting `ToMerkleLeaves` completely to simplify the review
💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2553128897)
Reverted to the original implementation, it's the least-invasive change with simplest diff
💬 l0rinc commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3566713903)
Thanks for the explanation @achow101!

> renaming PRs are generally limited to things that are particularly misleading and actively causing confusion.

I assumed that was the case here - if not, maybe the parent PR could also just avoid the rename in the first place.

> I get that having smaller PRs can make it feel like things are moving faster for a project, but it also means that there is an intermediate state where we will have merged a bunch of prep work but not the feature itself.

...
🤔 hebasto reviewed a pull request: "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO"
(https://github.com/bitcoin/bitcoin/pull/33702#pullrequestreview-3496784212)
I've tested fa2fd0ba1fbd4085df24a2c5472636957db28521 on a Windows 11 laptop using VS 2022 17.14.21.

Functional tests work as expected.
🤔 sipa reviewed a pull request: "Cluster mempool"
(https://github.com/bitcoin/bitcoin/pull/33629#pullrequestreview-3496807348)
ACK 17cf9ff7efdbab07644fc2f9017fcac1b0757c38