✅ ryanofsky closed an issue: "Default ASmap file path is not used unless -asmap is set"
(https://github.com/bitcoin/bitcoin/issues/33386)
(https://github.com/bitcoin/bitcoin/issues/33386)
💬 ryanofsky commented on issue "Default ASmap file path is not used unless -asmap is set":
(https://github.com/bitcoin/bitcoin/issues/33386#issuecomment-3565003991)
Closing since this should be resolved with #33770
(https://github.com/bitcoin/bitcoin/issues/33386#issuecomment-3565003991)
Closing since this should be resolved with #33770
💬 ryanofsky commented on pull request "init: Split file path handling out of -asmap option":
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3565097582)
Yes conflict with #33770 is superficial, and both could be merged. They both solved #33386 in different ways, but are otherwise independent.
I don't think this PR is a good idea, but don't oppose it if others like it. I don't think separating `-asmap` and `-asmapfile` options is user-friendly or consistent, even though I understand the problem this is trying to solve. I just think the problem is minor and temporary.
Right now since #33770 is merged there's no issue with the `-asmap` option
...
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3565097582)
Yes conflict with #33770 is superficial, and both could be merged. They both solved #33386 in different ways, but are otherwise independent.
I don't think this PR is a good idea, but don't oppose it if others like it. I don't think separating `-asmap` and `-asmapfile` options is user-friendly or consistent, even though I understand the problem this is trying to solve. I just think the problem is minor and temporary.
Right now since #33770 is merged there's no issue with the `-asmap` option
...
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3565452558)
Just repushed this branch with `POST_CHANGE_WORK` reduced to `5 * ACCEPTABLE_ITERS`. I gathered some simulation data (by replaying transactions/blocks that my datalogger recorded in 2023) in this [gist](https://gist.github.com/sdaftuar/77d4e67d0b5e1ef59e6d2ecf4d43484e), if anyone is curious about the benchmarks I was looking at.
The main piece of information to consider is that in November and December 2023, there were clusters that CSS had trouble linearizing, and so the 99th percentile of
...
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3565452558)
Just repushed this branch with `POST_CHANGE_WORK` reduced to `5 * ACCEPTABLE_ITERS`. I gathered some simulation data (by replaying transactions/blocks that my datalogger recorded in 2023) in this [gist](https://gist.github.com/sdaftuar/77d4e67d0b5e1ef59e6d2ecf4d43484e), if anyone is curious about the benchmarks I was looking at.
The main piece of information to consider is that in November and December 2023, there were clusters that CSS had trouble linearizing, and so the 99th percentile of
...
💬 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.