💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2552304930)
@stringintech great catch! thank you! I've updated the PR to use the diff suggested above.
I forgot the 1 edge case scenario where this is possible - https://github.com/bitcoin/bitcoin/issues/32173#issuecomment-2767030982.
I ran bitcoind with an inconsistent block index state from the functional test in the issue (`BLOCK_FAILED_CHILD` happens with valid parents) and current logic wouldn't work as you mentioned. but it works on the latest push now.
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2552304930)
@stringintech great catch! thank you! I've updated the PR to use the diff suggested above.
I forgot the 1 edge case scenario where this is possible - https://github.com/bitcoin/bitcoin/issues/32173#issuecomment-2767030982.
I ran bitcoind with an inconsistent block index state from the functional test in the issue (`BLOCK_FAILED_CHILD` happens with valid parents) and current logic wouldn't work as you mentioned. but it works on the latest push now.
💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2552305472)
now that I think about it again, makes sense to call the rest of the code with last disconnected block (and not pindex) for a consistent + graceful shutdown. thanks for catching this!
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2552305472)
now that I think about it again, makes sense to call the rest of the code with last disconnected block (and not pindex) for a consistent + graceful shutdown. thanks for catching this!
💬 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.