💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1868553486)
We're reading the previous outpoints of that block's inputs, which are in many other previous blocks. So, not sure this is feasible.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1868553486)
We're reading the previous outpoints of that block's inputs, which are in many other previous blocks. So, not sure this is feasible.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-2515910524)
Rebased. Since #30039 reading inputs is much faster, so the effect of this is somewhat less significant. It's still a significant speedup though so still worth it. Especially for worst case where the cache is completely empty, like on startup or right after it gets flushed due to size.
It is also refactored significantly. The main thread now writes everything before notifying threads, and then joins in working. This lets us do significantly less work in the critical section and parallelize mo
...
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-2515910524)
Rebased. Since #30039 reading inputs is much faster, so the effect of this is somewhat less significant. It's still a significant speedup though so still worth it. Especially for worst case where the cache is completely empty, like on startup or right after it gets flushed due to size.
It is also refactored significantly. The main thread now writes everything before notifying threads, and then joins in working. This lets us do significantly less work in the critical section and parallelize mo
...
💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2515985691)
Rebased to test performance with #30039.
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2515985691)
Rebased to test performance with #30039.
💬 andremralves commented on issue "Add `sat_to_btc()` and conversely `btc_to_sat()` util functions in functional tests":
(https://github.com/bitcoin/bitcoin/issues/31345#issuecomment-2516042652)
> Could I take the issue?
Hi @le0nAg. If you’d like, you can take this issue. There are still some updates to make, as noted in the PR comments. I’ll probably focus on another issue, so you could fork my branch and continue the work in a new PR.
(https://github.com/bitcoin/bitcoin/issues/31345#issuecomment-2516042652)
> Could I take the issue?
Hi @le0nAg. If you’d like, you can take this issue. There are still some updates to make, as noted in the PR comments. I’ll probably focus on another issue, so you could fork my branch and continue the work in a new PR.
💬 ryanofsky commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2516059562)
re: https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2515554249
> Not sure if it's worth sharing here but hey what's an 1300th comment.
Thanks for the report! Was able to reproduce this and created issue https://github.com/chaincodelabs/libmultiprocess/issues/123 to track it.
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2516059562)
re: https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2515554249
> Not sure if it's worth sharing here but hey what's an 1300th comment.
Thanks for the report! Was able to reproduce this and created issue https://github.com/chaincodelabs/libmultiprocess/issues/123 to track it.
💬 Bambibrown commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1868688590)
```suggestion
const auto ### _**sequences{miniminer_manual.Linearize().inclusion_order};**_
```
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1868688590)
```suggestion
const auto ### _**sequences{miniminer_manual.Linearize().inclusion_order};**_
```
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2516244533)
Rebased after #31112.
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2516244533)
Rebased after #31112.
💬 Sjors commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2516248571)
Rebased after https://github.com/bitcoin/bitcoin/pull/31112.
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2516248571)
Rebased after https://github.com/bitcoin/bitcoin/pull/31112.
💬 le0nAg commented on issue "Add `sat_to_btc()` and conversely `btc_to_sat()` util functions in functional tests":
(https://github.com/bitcoin/bitcoin/issues/31345#issuecomment-2516322672)
Ok, thanks! I took yesterday and today to understand at which point is the PR.
Tomorrow I will fork it
(https://github.com/bitcoin/bitcoin/issues/31345#issuecomment-2516322672)
Ok, thanks! I took yesterday and today to understand at which point is the PR.
Tomorrow I will fork it
👍 maflcko approved a pull request: "test: Prove+document ConstevalFormatString/tinyformat parity"
(https://github.com/bitcoin/bitcoin/pull/30933#pullrequestreview-2477481144)
lgtm, but it would be better to merge the conflicting non-draft pull with two acks first. This will also avoid having to touch some lines twice, which are touched here.
(https://github.com/bitcoin/bitcoin/pull/30933#pullrequestreview-2477481144)
lgtm, but it would be better to merge the conflicting non-draft pull with two acks first. This will also avoid having to touch some lines twice, which are touched here.
💬 maflcko commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1868849318)
Could do a third listwalletdir without the symlink to check that the test is working by checking for absence of `error scanning`?
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1868849318)
Could do a third listwalletdir without the symlink to check that the test is working by checking for absence of `error scanning`?
💬 maflcko commented on pull request "cmake: use python from venv if available":
(https://github.com/bitcoin/bitcoin/pull/31411#issuecomment-2516416418)
I had the same thought. Shouldn't a venv "just work" out of the box, because they set the `PATH` appropriately?
(https://github.com/bitcoin/bitcoin/pull/31411#issuecomment-2516416418)
I had the same thought. Shouldn't a venv "just work" out of the box, because they set the `PATH` appropriately?
💬 maflcko commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2516421998)
For reference the CI failure is:
```
subprocess.CalledProcessError: Command '['C:\\hostedtoolcache\\windows\\Python\\3.12.7\\x64\\python.exe', 'D:/a/bitcoin/bitcoin\\contrib\\signet\\miner', "--cli='D:\\a\\bitcoin\\bitcoin\\build\\src\\Release\\bitcoin-cli.exe' -nonamed '-datadir=D:\\a\\_temp\\test_runner_₿_🏃_20241203_223434\\tool_signet_miner_211\\node0'", 'generate', '--address=tb1q2ndfasp67k5wp30fkt63tw9gf465lcjf7rm5fc', "--grind-cmd='D:\\a\\bitcoin\\bi
...
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2516421998)
For reference the CI failure is:
```
subprocess.CalledProcessError: Command '['C:\\hostedtoolcache\\windows\\Python\\3.12.7\\x64\\python.exe', 'D:/a/bitcoin/bitcoin\\contrib\\signet\\miner', "--cli='D:\\a\\bitcoin\\bitcoin\\build\\src\\Release\\bitcoin-cli.exe' -nonamed '-datadir=D:\\a\\_temp\\test_runner_₿_🏃_20241203_223434\\tool_signet_miner_211\\node0'", 'generate', '--address=tb1q2ndfasp67k5wp30fkt63tw9gf465lcjf7rm5fc', "--grind-cmd='D:\\a\\bitcoin\\bi
...
💬 summraznboi commented on issue "Cluster mempool tracking issue":
(https://github.com/bitcoin/bitcoin/issues/30289#issuecomment-2516463194)
Wow, no ancestor/descendant limit would be great for unblocking UX hurdles in creating transactions! (Commenting to watch, also curious how someone like myself could help out here).
(https://github.com/bitcoin/bitcoin/issues/30289#issuecomment-2516463194)
Wow, no ancestor/descendant limit would be great for unblocking UX hurdles in creating transactions! (Commenting to watch, also curious how someone like myself could help out here).
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2516465730)
Rebased 8598bc9e5d3fb7ebc08cf0c6422b3e44c56230d6 -> 6090df267dfece6192b567fed6582445aa811e7f ([kernelApi_8](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_8) -> [kernelApi_9](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_8..kernelApi_9))
* Alligned process block pre-checks through https://github.com/bitcoin/bitcoin/pull/31175
* Clamp the work threads number so we properly handle the value range through
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2516465730)
Rebased 8598bc9e5d3fb7ebc08cf0c6422b3e44c56230d6 -> 6090df267dfece6192b567fed6582445aa811e7f ([kernelApi_8](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_8) -> [kernelApi_9](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_8..kernelApi_9))
* Alligned process block pre-checks through https://github.com/bitcoin/bitcoin/pull/31175
* Clamp the work threads number so we properly handle the value range through
...
💬 carnhofdaki commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2516525566)
Concept NACK.
I use a simple script setting datadir according to the current working directory. If the change would be backward-compatible I don't care. But running multiple custom signets and never needed to make a mess. The proposed naming convention (`signet_XXXXXXX`) seems like a mess to me.
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2516525566)
Concept NACK.
I use a simple script setting datadir according to the current working directory. If the change would be backward-compatible I don't care. But running multiple custom signets and never needed to make a mess. The proposed naming convention (`signet_XXXXXXX`) seems like a mess to me.
💬 l0rinc commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1869004667)
I was wondering how it will behave with `%c` which would add a `\0` which could end the const char* prematurely, but since we have a string it seems to work correctly.
Are there any checks we could do with the result here? Or make `TfmFormatZeroes` void if we don't need the actual result. Maybe something like this:
```suggestion
auto result{TfmFormatZeroes<NumArgs>(fmt.fmt)};
BOOST_CHECK_EQUAL(result.empty(), std::string_view{fmt.fmt}.empty());
```
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1869004667)
I was wondering how it will behave with `%c` which would add a `\0` which could end the const char* prematurely, but since we have a string it seems to work correctly.
Are there any checks we could do with the result here? Or make `TfmFormatZeroes` void if we don't need the actual result. Maybe something like this:
```suggestion
auto result{TfmFormatZeroes<NumArgs>(fmt.fmt)};
BOOST_CHECK_EQUAL(result.empty(), std::string_view{fmt.fmt}.empty());
```
💬 l0rinc commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1869005752)
Is it coincidental that every single format type accepts `0` as a valid substitution?
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1869005752)
Is it coincidental that every single format type accepts `0` as a valid substitution?
💬 l0rinc commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1868990468)
👍 for checking `%n` as well!
nit: we might inline `0` here:
```suggestion
BOOST_CHECK_EXCEPTION(tfm::format(std::string{"%n"}, 0), tfm::format_error,
```
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1868990468)
👍 for checking `%n` as well!
nit: we might inline `0` here:
```suggestion
BOOST_CHECK_EXCEPTION(tfm::format(std::string{"%n"}, 0), tfm::format_error,
```
💬 hebasto commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1869077815)
Thanks! Done.
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1869077815)
Thanks! Done.