💬 achow101 commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#issuecomment-3564962500)
ACK 288b8c30be42f2879d1a1f3d5ec3cac13f87ace4
(https://github.com/bitcoin/bitcoin/pull/33770#issuecomment-3564962500)
ACK 288b8c30be42f2879d1a1f3d5ec3cac13f87ace4
🚀 achow101 merged a pull request: "init: Require explicit -asmap filename"
(https://github.com/bitcoin/bitcoin/pull/33770)
(https://github.com/bitcoin/bitcoin/pull/33770)
💬 ryanofsky commented on issue "should concurrent IPC requests directed to the same thread cause a crash?":
(https://github.com/bitcoin/bitcoin/issues/33923#issuecomment-3564979973)
> shouldn't Bitcoin Core simply not crash if two requests arrive for the same thread?
Yes this shouldn't crash. Hopefully, if you are using a version of bitcoin after https://github.com/bitcoin-core/libmultiprocess/pull/214 (part of #33518 in master and part of #33519 in 30.x branch) you should just see "thread busy" errors instead of crashes, and it is a bug if there are still crashes.
> ideally, a naive client should simply be forced to wait a bit longer if they send concurrent requests agai
...
(https://github.com/bitcoin/bitcoin/issues/33923#issuecomment-3564979973)
> shouldn't Bitcoin Core simply not crash if two requests arrive for the same thread?
Yes this shouldn't crash. Hopefully, if you are using a version of bitcoin after https://github.com/bitcoin-core/libmultiprocess/pull/214 (part of #33518 in master and part of #33519 in 30.x branch) you should just see "thread busy" errors instead of crashes, and it is a bug if there are still crashes.
> ideally, a naive client should simply be forced to wait a bit longer if they send concurrent requests agai
...
✅ 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