💬 Farxial commented on issue "Add parallelism for downloading the blockchain":
(https://github.com/bitcoin/bitcoin/issues/27442#issuecomment-1501147793)
@pinheadmz
Sometime I tried `assumevalid`, but it didn't help me. May be I tried wrong? OK then, I will try this on next full synching, if you insist. _Does this work with **prune** blockchain synching?_
I can't find how to download from multiple peers on request `headers first sync`. Can you tell a launch option, please?
And, of course, it would be nice to find such an option in the output of `bitcoind --help`.
@sipa
I see...
Thanks for the info both of you.
(https://github.com/bitcoin/bitcoin/issues/27442#issuecomment-1501147793)
@pinheadmz
Sometime I tried `assumevalid`, but it didn't help me. May be I tried wrong? OK then, I will try this on next full synching, if you insist. _Does this work with **prune** blockchain synching?_
I can't find how to download from multiple peers on request `headers first sync`. Can you tell a launch option, please?
And, of course, it would be nice to find such an option in the output of `bitcoind --help`.
@sipa
I see...
Thanks for the info both of you.
💬 pinheadmz commented on issue "Add parallelism for downloading the blockchain":
(https://github.com/bitcoin/bitcoin/issues/27442#issuecomment-1501149253)
By default, the first `751565` blocks are assumed valid, without any user setting:
https://github.com/bitcoin/bitcoin/blob/db720b5a703c90625fa7a4773bd2db5672427cbe/src/kernel/chainparams.cpp#L107
Parallel download is also the default, see `FindNextBlocksToDownload()`:
https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L1298
(https://github.com/bitcoin/bitcoin/issues/27442#issuecomment-1501149253)
By default, the first `751565` blocks are assumed valid, without any user setting:
https://github.com/bitcoin/bitcoin/blob/db720b5a703c90625fa7a4773bd2db5672427cbe/src/kernel/chainparams.cpp#L107
Parallel download is also the default, see `FindNextBlocksToDownload()`:
https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L1298
💬 sipa commented on issue "anchors.dat doesn't support V2 addresses":
(https://github.com/bitcoin/bitcoin/issues/20511#issuecomment-1501155107)
@pinheadmz I think so.
(https://github.com/bitcoin/bitcoin/issues/20511#issuecomment-1501155107)
@pinheadmz I think so.
👍 jarolrod approved a pull request: "doc: correct sqlite & qrencode versions used in depenendencies.md"
(https://github.com/bitcoin/bitcoin/pull/27441#pullrequestreview-1376932590)
ACK a12d9cfa46ad5f5a5144daabbc146d0175642c69
(https://github.com/bitcoin/bitcoin/pull/27441#pullrequestreview-1376932590)
ACK a12d9cfa46ad5f5a5144daabbc146d0175642c69
:lock: fanquake locked an issue: "I forgot the password of one of my wallets on Bitcoin Core App"
(https://github.com/bitcoin/bitcoin/issues/27421)
(https://github.com/bitcoin/bitcoin/issues/27421)
🚀 fanquake merged a pull request: "doc: FreeBSD DataDirectoryGroupReadable Setting"
(https://github.com/bitcoin/bitcoin/pull/26741)
(https://github.com/bitcoin/bitcoin/pull/26741)
👍 martinus approved a pull request: "refactor: Make `CCheckQueue` RAII-styled"
(https://github.com/bitcoin/bitcoin/pull/26762#pullrequestreview-1377151234)
Code review ACK 51f91c2c5bd, as far as I can say now deadlocks are not possible any more when shutting down. I started an initial sync with `-assumevalid=0` so script checks are done, stopping the process a few times with Ctrl+C, all good; shutdown is still quick. `-par` still works too.
(https://github.com/bitcoin/bitcoin/pull/26762#pullrequestreview-1377151234)
Code review ACK 51f91c2c5bd, as far as I can say now deadlocks are not possible any more when shutting down. I started an initial sync with `-assumevalid=0` so script checks are done, stopping the process a few times with Ctrl+C, all good; shutdown is still quick. `-par` still works too.
💬 martinus commented on pull request "refactor: Make `CCheckQueue` RAII-styled":
(https://github.com/bitcoin/bitcoin/pull/26762#discussion_r1161479180)
nit, just a thought: maybe it would make sense to have a method like `signalStop()` or similar in `CCheckQueue` that only sets `m_request_stop` to true, and call this as soon as we know we don't need the queue's result any more. That way the threads will stop processing and stop wasting CPU while shutting down.
On the other hand the queue will usually pretty quickly stop working unless it has lots of work items, so I guess in most cases this wouldn't make any difference.
(https://github.com/bitcoin/bitcoin/pull/26762#discussion_r1161479180)
nit, just a thought: maybe it would make sense to have a method like `signalStop()` or similar in `CCheckQueue` that only sets `m_request_stop` to true, and call this as soon as we know we don't need the queue's result any more. That way the threads will stop processing and stop wasting CPU while shutting down.
On the other hand the queue will usually pretty quickly stop working unless it has lots of work items, so I guess in most cases this wouldn't make any difference.
💬 MarcoFalke commented on pull request "test: LLVM/Clang 16 for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1161589201)
```suggestion
cmake -B "${BASE_SCRATCH_DIR}"/msan/build/ -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi' -DCMAKE_BUILD_TYPE=Release -DLLVM_USE_SANITIZER=MemoryWithOrigins -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF -S "${BASE_SCRATCH_DIR}"/msan/llvm-project/runtimes
```
nit?
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1161589201)
```suggestion
cmake -B "${BASE_SCRATCH_DIR}"/msan/build/ -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi' -DCMAKE_BUILD_TYPE=Release -DLLVM_USE_SANITIZER=MemoryWithOrigins -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF -S "${BASE_SCRATCH_DIR}"/msan/llvm-project/runtimes
```
nit?
💬 fanquake commented on pull request "test: LLVM/Clang 16 for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1161597309)
Yea. Fixed
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1161597309)
Yea. Fixed
💬 achow101 commented on pull request "doc: Add example of how to mix private and public keys in descriptors":
(https://github.com/bitcoin/bitcoin/pull/27414#issuecomment-1501769224)
Concept ACK
Please squash your commits
(https://github.com/bitcoin/bitcoin/pull/27414#issuecomment-1501769224)
Concept ACK
Please squash your commits
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1161726047)
yes, that was something I touched while I was verifying the `req` class member.
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1161726047)
yes, that was something I touched while I was verifying the `req` class member.
💬 achow101 commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#issuecomment-1501822228)
ACK 68eed5df8656bed1be6526b014e58d3123102b03
(https://github.com/bitcoin/bitcoin/pull/26699#issuecomment-1501822228)
ACK 68eed5df8656bed1be6526b014e58d3123102b03
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1161727508)
I'll change it. Thanks.
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1161727508)
I'll change it. Thanks.
💬 achow101 commented on pull request "wallet: Avoid underpaying transaction fees when signing taproot spends":
(https://github.com/bitcoin/bitcoin/pull/23502#issuecomment-1501832645)
> How would this behave in the context of changeless transactions? Would we be overestimating the target and dropping an excess to the fees?
All overages due to size estimation differences are dropped to fees. The output values are set based upon the estimated size and they cannot be changed after signing. Once signing is completed and the final transaction created, if it ends up being smaller than estimated, whatever could have been saved will not be saved.
> Does that get recognized when
...
(https://github.com/bitcoin/bitcoin/pull/23502#issuecomment-1501832645)
> How would this behave in the context of changeless transactions? Would we be overestimating the target and dropping an excess to the fees?
All overages due to size estimation differences are dropped to fees. The output values are set based upon the estimated size and they cannot be changed after signing. Once signing is completed and the final transaction created, if it ends up being smaller than estimated, whatever could have been saved will not be saved.
> Does that get recognized when
...
💬 achow101 commented on pull request "wallet: Avoid underpaying transaction fees when signing taproot spends":
(https://github.com/bitcoin/bitcoin/pull/23502#discussion_r1161737139)
I don't quite follow your question.
The estimated stack size is used to choose which result to return as multiple results could be valid. In normal signing, we use the smallest valid stack. In estimating, we use the largest.
(https://github.com/bitcoin/bitcoin/pull/23502#discussion_r1161737139)
I don't quite follow your question.
The estimated stack size is used to choose which result to return as multiple results could be valid. In normal signing, we use the smallest valid stack. In estimating, we use the largest.
💬 jonatack commented on pull request "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet":
(https://github.com/bitcoin/bitcoin/pull/27279#discussion_r1161754205)
I followed the indentation style used for the same `warnings` field in RPCs createmultisig, addmultisigaddress, importmulti, importdescriptors, etc., as yes, `clang-format` would change the whole function.
(https://github.com/bitcoin/bitcoin/pull/27279#discussion_r1161754205)
I followed the indentation style used for the same `warnings` field in RPCs createmultisig, addmultisigaddress, importmulti, importdescriptors, etc., as yes, `clang-format` would change the whole function.
💬 achow101 commented on pull request "bumpfee: avoid making bumped transactions with too low fee when replacing outputs":
(https://github.com/bitcoin/bitcoin/pull/27308#discussion_r1161754652)
Done
(https://github.com/bitcoin/bitcoin/pull/27308#discussion_r1161754652)
Done
💬 achow101 commented on pull request "bumpfee: avoid making bumped transactions with too low fee when replacing outputs":
(https://github.com/bitcoin/bitcoin/pull/27308#discussion_r1161754817)
Changed to `temp_mtx`
(https://github.com/bitcoin/bitcoin/pull/27308#discussion_r1161754817)
Changed to `temp_mtx`
💬 achow101 commented on pull request "bumpfee: avoid making bumped transactions with too low fee when replacing outputs":
(https://github.com/bitcoin/bitcoin/pull/27308#discussion_r1161757888)
> If I understand this test right, it checks whether the absolute fee of a smaller replacement transaction sufficiently surpasses that of a larger original transaction.
No, it doesn't check any feerates since we can't see the candidate bump transaction. The calculations here are just for figuring out a feerate that would fail the modified checks and checking that `bumpfee` fails as expected.
> I looked around in `wallet_bumpfee.py`, and had the impression that we generally seem to assume t
...
(https://github.com/bitcoin/bitcoin/pull/27308#discussion_r1161757888)
> If I understand this test right, it checks whether the absolute fee of a smaller replacement transaction sufficiently surpasses that of a larger original transaction.
No, it doesn't check any feerates since we can't see the candidate bump transaction. The calculations here are just for figuring out a feerate that would fail the modified checks and checking that `bumpfee` fails as expected.
> I looked around in `wallet_bumpfee.py`, and had the impression that we generally seem to assume t
...