💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708104386)
Nice, indeed! I needed to make some change to the unserializer to track in `reordering` the reverse mapping, which I've for now done in the same commit. Happy to split that out if useful.
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708104386)
Nice, indeed! I needed to make some change to the unserializer to track in `reordering` the reverse mapping, which I've for now done in the same commit. Happy to split that out if useful.
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708106586)
@instagibbs I've expanded the comments to explain why the (max,min) sort order is the same as (2^max + 2^min).
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708106586)
@instagibbs I've expanded the comments to explain why the (max,min) sort order is the same as (2^max + 2^min).
💬 theStack commented on pull request "depends: Make default `host` and `build` comparable":
(https://github.com/bitcoin/bitcoin/pull/30584#issuecomment-2274568113)
@hebasto: Can you give a hint on how to test this? As far as I remember, I saw an output where "Cross compiling ....." was set to TRUE in the course of testing another PR on your cmake branch with a toolchain.cmake (probably https://github.com/hebasto/bitcoin/pull/292), but in this PR there is no toolchain.cmake yet and hence I don't know how to reproduce it.
(https://github.com/bitcoin/bitcoin/pull/30584#issuecomment-2274568113)
@hebasto: Can you give a hint on how to test this? As far as I remember, I saw an output where "Cross compiling ....." was set to TRUE in the course of testing another PR on your cmake branch with a toolchain.cmake (probably https://github.com/hebasto/bitcoin/pull/292), but in this PR there is no toolchain.cmake yet and hence I don't know how to reproduce it.
✅ achow101 closed an issue: "Prune undermines the dbcache."
(https://github.com/bitcoin/bitcoin/issues/11315)
(https://github.com/bitcoin/bitcoin/issues/11315)
🚀 achow101 merged a pull request: "Don't empty dbcache on prune flushes: >30% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/28280)
(https://github.com/bitcoin/bitcoin/pull/28280)
💬 ryanofsky commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet` - (`void wallet::UnloadWallet(std::shared_ptr<CWallet> &&): Assertion 'it.second' failed.`)":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2274773413)
> I see the issue during shutdown, inside `UnloadWallets` (we are also not checking the function's return value). But that seems to be a different problem.
I think my suggested change should fix that problem. It's a good observation that if two unloadwallet calls happen at the same time only one of them will call UnloadWallet, so the "multiple unloadwallet RPC calls at the same time" race can't happen. But the "unloadwallet RPC call at the same time as a shutdown" case I mentioned can happen
...
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2274773413)
> I see the issue during shutdown, inside `UnloadWallets` (we are also not checking the function's return value). But that seems to be a different problem.
I think my suggested change should fix that problem. It's a good observation that if two unloadwallet calls happen at the same time only one of them will call UnloadWallet, so the "multiple unloadwallet RPC calls at the same time" race can't happen. But the "unloadwallet RPC call at the same time as a shutdown" case I mentioned can happen
...
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708486665)
I have split the commit in two, with additional comments, and I renamed `grow_inc` to `reconsider_pot`.
Roughly what is going on is that:
* In "include topological pot subsets automatically", all of every new work item's `pot` set is searched for topologically-valid subsets (which can be moved to `inc` instead)
* In "avoid recomputing potential set on every split" (before my last push), only the part of the new item's `pot` set that *wasn't* in the parent work item's `pot` set is searched f
...
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708486665)
I have split the commit in two, with additional comments, and I renamed `grow_inc` to `reconsider_pot`.
Roughly what is going on is that:
* In "include topological pot subsets automatically", all of every new work item's `pot` set is searched for topologically-valid subsets (which can be moved to `inc` instead)
* In "avoid recomputing potential set on every split" (before my last push), only the part of the new item's `pot` set that *wasn't* in the parent work item's `pot` set is searched f
...
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708487431)
Done.
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708487431)
Done.
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708488215)
Just a rough estimate based on benchmarks. Added a comment.
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708488215)
Just a rough estimate based on benchmarks. Added a comment.
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708488330)
Just a rough estimate based on benchmarks. Added a comment.
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1708488330)
Just a rough estimate based on benchmarks. Added a comment.
💬 maflcko commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#issuecomment-2275044162)
I just copy-pasted the logic flow from the mlc function (https://github.com/bitcoin/bitcoin/blame/fa9cff60fca74a7ff60042c5e632502b6c4c2899/test/lint/test_runner/src/main.rs#L432-L472) So maybe @willcl-ark may be qualified to review this?
(https://github.com/bitcoin/bitcoin/pull/30553#issuecomment-2275044162)
I just copy-pasted the logic flow from the mlc function (https://github.com/bitcoin/bitcoin/blame/fa9cff60fca74a7ff60042c5e632502b6c4c2899/test/lint/test_runner/src/main.rs#L432-L472) So maybe @willcl-ark may be qualified to review this?
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30606)
This PR adds the option to create a wallet from a seed. The seed of a wallet can be obtained using the RPC command `getblsctseed`.
Two options to create a wallet:
- Using the `navio-wallet` tool together with the option `seed`. Example:
`navio-wallet -blsct -chain=blsctregtest -wallet=wallet_name -seed="71187dc33c03914c630e87b205777d51f7bd2749bc1164795e0134544d43d9ee" create`
- Using the rpc command `createwallet`. Example:
`navio-cli --blsctregtest -named createwallet wallet_name=walle
...
(https://github.com/bitcoin/bitcoin/pull/30606)
This PR adds the option to create a wallet from a seed. The seed of a wallet can be obtained using the RPC command `getblsctseed`.
Two options to create a wallet:
- Using the `navio-wallet` tool together with the option `seed`. Example:
`navio-wallet -blsct -chain=blsctregtest -wallet=wallet_name -seed="71187dc33c03914c630e87b205777d51f7bd2749bc1164795e0134544d43d9ee" create`
- Using the rpc command `createwallet`. Example:
`navio-cli --blsctregtest -named createwallet wallet_name=walle
...
💬 maflcko commented on pull request "Drop log category in `SeedStartup`":
(https://github.com/bitcoin/bitcoin/pull/29480#issuecomment-2275131328)
> This `LogPrint(BCLog::RAND, ...)` is never logged because the `SeedStartup` function is called at a very early stage, such as during the instantiation of the static `CSignatureCache` object, before any log categories are added. This PR fixes this behavior.
This is outdated and the steps to reproduce no longer work. Currently, it needs something like:
```diff
diff --git a/src/init.cpp b/src/init.cpp
index faaf3353d0..164f624b4e 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -194,6 +
...
(https://github.com/bitcoin/bitcoin/pull/29480#issuecomment-2275131328)
> This `LogPrint(BCLog::RAND, ...)` is never logged because the `SeedStartup` function is called at a very early stage, such as during the instantiation of the static `CSignatureCache` object, before any log categories are added. This PR fixes this behavior.
This is outdated and the steps to reproduce no longer work. Currently, it needs something like:
```diff
diff --git a/src/init.cpp b/src/init.cpp
index faaf3353d0..164f624b4e 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -194,6 +
...
💬 maflcko commented on pull request "Drop log category in `SeedStartup`":
(https://github.com/bitcoin/bitcoin/pull/29480#issuecomment-2275153443)
review ACK 88468a8afcd908cd26f4e6ecfb86c80d1c56fe73
but it would be good to update the context here, given my previous reply.
Also, it may be good to use `LogDebug` in the only remaining RAND-debug-log call:
```
$ git grep '::RAND'
src/logging.cpp: {"rand", BCLog::RAND},
src/random.cpp: LogPrint(BCLog::RAND, "Feeding %i bytes of dynamic environment data into RNG\n", hasher.Size() - old_size);
```
This also clarifies that the this is the only remaining call, which seems like
...
(https://github.com/bitcoin/bitcoin/pull/29480#issuecomment-2275153443)
review ACK 88468a8afcd908cd26f4e6ecfb86c80d1c56fe73
but it would be good to update the context here, given my previous reply.
Also, it may be good to use `LogDebug` in the only remaining RAND-debug-log call:
```
$ git grep '::RAND'
src/logging.cpp: {"rand", BCLog::RAND},
src/random.cpp: LogPrint(BCLog::RAND, "Feeding %i bytes of dynamic environment data into RNG\n", hasher.Size() - old_size);
```
This also clarifies that the this is the only remaining call, which seems like
...
💬 hebasto commented on pull request "depends: Make default `host` and `build` comparable":
(https://github.com/bitcoin/bitcoin/pull/30584#issuecomment-2275201061)
@theStack
> Can you give a hint on how to test this? As far as I remember, I saw an output where "Cross compiling ....." was set to TRUE in the course of testing another PR on your cmake branch with a toolchain.cmake (probably [hebasto#292](https://github.com/hebasto/bitcoin/pull/292)), but in this PR there is no toolchain.cmake yet and hence I don't know how to reproduce it.
You could print variable values out:
```
cd depends
gmake print-host
gmake print-build
cd ..
```
By default, w
...
(https://github.com/bitcoin/bitcoin/pull/30584#issuecomment-2275201061)
@theStack
> Can you give a hint on how to test this? As far as I remember, I saw an output where "Cross compiling ....." was set to TRUE in the course of testing another PR on your cmake branch with a toolchain.cmake (probably [hebasto#292](https://github.com/hebasto/bitcoin/pull/292)), but in this PR there is no toolchain.cmake yet and hence I don't know how to reproduce it.
You could print variable values out:
```
cd depends
gmake print-host
gmake print-build
cd ..
```
By default, w
...
💬 maflcko commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1708929418)
nit in d07d558da945a47069b91bfce58b13831da61f23: Same?
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1708929418)
nit in d07d558da945a47069b91bfce58b13831da61f23: Same?
💬 maflcko commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1708927378)
style nit in 16ea431f1f3a4ae57729ba265ff080b64b711c5d: Could add the missing trailing comma now? Otherwise this line will have to be touched again in the future, if something is appended.
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1708927378)
style nit in 16ea431f1f3a4ae57729ba265ff080b64b711c5d: Could add the missing trailing comma now? Otherwise this line will have to be touched again in the future, if something is appended.
💬 maflcko commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1708917038)
As a follow-up idea, it could make sense to reduce the timeout here and instead scale it with the self.timeout_factor, so that the timeout is magically adjusted to the speed of the test-env it is running in
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1708917038)
As a follow-up idea, it could make sense to reduce the timeout here and instead scale it with the self.timeout_factor, so that the timeout is magically adjusted to the speed of the test-env it is running in
👍 maflcko approved a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2227173636)
Left a minor follow-up idea and a few style nits, but nothing blocking. lgtm
review ACK edef8c78e3a436e7187937b6e1bc906392ef9891 💽
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+
...
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2227173636)
Left a minor follow-up idea and a few style nits, but nothing blocking. lgtm
review ACK edef8c78e3a436e7187937b6e1bc906392ef9891 💽
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+
...
💬 maflcko commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1708922064)
style nit in 07e7bd873bd512c9d488efbaf452e54efc92ac0a: Could clarify that this is *seconds*, not nano- or other seconds (like in other tracepoints)?
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1708922064)
style nit in 07e7bd873bd512c9d488efbaf452e54efc92ac0a: Could clarify that this is *seconds*, not nano- or other seconds (like in other tracepoints)?