💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2552305597)
taken. thanks!
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2552305597)
taken. thanks!
💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#issuecomment-3565803301)
thanks for the great catches @stringintech and the nicer tests @stickies-v! I've pushed an [update](https://github.com/bitcoin/bitcoin/compare/cfadc8038c08f9804c81af7950164483761f1db5..b22d8b89c29a0a0d385d38d8ab74e5b940ddf7d5).
(https://github.com/bitcoin/bitcoin/pull/32950#issuecomment-3565803301)
thanks for the great catches @stringintech and the nicer tests @stickies-v! I've pushed an [update](https://github.com/bitcoin/bitcoin/compare/cfadc8038c08f9804c81af7950164483761f1db5..b22d8b89c29a0a0d385d38d8ab74e5b940ddf7d5).
💬 ajtowns commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3565835341)
> Hi AJ, are these the test vectors you had in mind?
I was thinking functional tests, to also make sure there isn't some obscure policy rule that the unit tests miss that nevertheless ends up blocking successful spends of things that should be spendable. Unit tests are good too though.
"CMS NOT" isn't super interesting though, since it's easily satisfiable with empty sigs (apart from mempool/consensus inconsistency bugs). The things that (per this PR's arguments) should be spendable are `1
...
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3565835341)
> Hi AJ, are these the test vectors you had in mind?
I was thinking functional tests, to also make sure there isn't some obscure policy rule that the unit tests miss that nevertheless ends up blocking successful spends of things that should be spendable. Unit tests are good too though.
"CMS NOT" isn't super interesting though, since it's easily satisfiable with empty sigs (apart from mempool/consensus inconsistency bugs). The things that (per this PR's arguments) should be spendable are `1
...
💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2552443566)
Oops - thanks!
Fixed in [8457a71a64..78d6402458](https://github.com/bitcoin/bitcoin/compare/8457a71a6420792a1b9c7c8645c9e5c607a4b90c..78d6402458d7d14533444d893c989f0534a3896f) (also removed 2 duplicate test cases above).
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2552443566)
Oops - thanks!
Fixed in [8457a71a64..78d6402458](https://github.com/bitcoin/bitcoin/compare/8457a71a6420792a1b9c7c8645c9e5c607a4b90c..78d6402458d7d14533444d893c989f0534a3896f) (also removed 2 duplicate test cases above).
💬 musaHaruna commented on pull request "wallet, rpc: add UTXO set check and incremental rescan to importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553065653)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/33392#discussion_r2553065653)
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_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
...
(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
(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.
(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
(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
(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
(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
(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!
(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.
(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.
(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.
(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!
(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.
(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.
(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
...
(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
...