💬 l0rinc commented on issue "More control of maintenance processes at startup/restart":
(https://github.com/bitcoin/bitcoin/issues/29662#issuecomment-2939919799)
Hey @domZippilli, LevelDB compaction sucks, especially since it's single threaded. It's why RocksDB has spread it to multiple threads, reducing compaction burden considerably - switching Bitcoin Core experimentally to RocksDB results in an instant ~20% speedup because of this. Unfortunately we can't just switch over, RocksDB is too complicated, at best we can slowly strangle out LevelDB usage, but we can't just drop it.
I have measured a similar compaction scenario in a related post, can you pl
...
(https://github.com/bitcoin/bitcoin/issues/29662#issuecomment-2939919799)
Hey @domZippilli, LevelDB compaction sucks, especially since it's single threaded. It's why RocksDB has spread it to multiple threads, reducing compaction burden considerably - switching Bitcoin Core experimentally to RocksDB results in an instant ~20% speedup because of this. Unfortunately we can't just switch over, RocksDB is too complicated, at best we can slowly strangle out LevelDB usage, but we can't just drop it.
I have measured a similar compaction scenario in a related post, can you pl
...
💬 brunoerg commented on pull request "test: wallet: cover wallet passphrase with a null char":
(https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2126558187)
Done
(https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2126558187)
Done
💬 brunoerg commented on pull request "test: wallet: cover wallet passphrase with a null char":
(https://github.com/bitcoin/bitcoin/pull/32675#issuecomment-2939979267)
3b90060a2df2b65369117f3ee8afe26be631867d..7cfbb8575e1ffbad5c48e2c461b45dd6ac63d064 address https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2124757170
(https://github.com/bitcoin/bitcoin/pull/32675#issuecomment-2939979267)
3b90060a2df2b65369117f3ee8afe26be631867d..7cfbb8575e1ffbad5c48e2c461b45dd6ac63d064 address https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2124757170
💬 maflcko commented on pull request "guix: warn and abort when SOURCE_DATE_EPOCH is set":
(https://github.com/bitcoin/bitcoin/pull/32678#issuecomment-2939981992)
lgtm ACK 5c4a0f8009cef758be9412428515bfed57b0c923
(https://github.com/bitcoin/bitcoin/pull/32678#issuecomment-2939981992)
lgtm ACK 5c4a0f8009cef758be9412428515bfed57b0c923
💬 maflcko commented on pull request "test: wallet: cover wallet passphrase with a null char":
(https://github.com/bitcoin/bitcoin/pull/32675#issuecomment-2939987081)
lgtm ACK 7cfbb8575e1ffbad5c48e2c461b45dd6ac63d064
(https://github.com/bitcoin/bitcoin/pull/32675#issuecomment-2939987081)
lgtm ACK 7cfbb8575e1ffbad5c48e2c461b45dd6ac63d064
🤔 danielabrozzoni reviewed a pull request: "rpc: Note in fundrawtransaction doc, fee rate is for package"
(https://github.com/bitcoin/bitcoin/pull/32607#pullrequestreview-2896689686)
ACK f98e1aaf34e347088caa54403521e3b5cb55dd40
(https://github.com/bitcoin/bitcoin/pull/32607#pullrequestreview-2896689686)
ACK f98e1aaf34e347088caa54403521e3b5cb55dd40
💬 l0rinc commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2940026257)
> Also, is this with 32-bit or 64-bit userspace?
64-bit userspace (AArch64)
```bash
$ file bitcoind
bitcoind: ELF 64-bit LSB pie executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=7e059ec01f7460042910ca4ed15270382269c9d5, for GNU/Linux 3.7.0, with debug_info, not stripped
```
<details>
<summary>additional details</summary>
```bash
$ getconf LONG_BIT
64
$ dpkg --print-architecture
arm64
$ uname -m
aarch64
...
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2940026257)
> Also, is this with 32-bit or 64-bit userspace?
64-bit userspace (AArch64)
```bash
$ file bitcoind
bitcoind: ELF 64-bit LSB pie executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=7e059ec01f7460042910ca4ed15270382269c9d5, for GNU/Linux 3.7.0, with debug_info, not stripped
```
<details>
<summary>additional details</summary>
```bash
$ getconf LONG_BIT
64
$ dpkg --print-architecture
arm64
$ uname -m
aarch64
...
💬 fanquake commented on pull request "depends: drop `ltcg` for Windows Qt":
(https://github.com/bitcoin/bitcoin/pull/32496#issuecomment-2940144750)
Qt + lto works for native, but is broken for Darwin cross builds, so dropped the second commit. This is now just a followup to
(https://github.com/bitcoin/bitcoin/pull/32496#issuecomment-2940144750)
Qt + lto works for native, but is broken for Darwin cross builds, so dropped the second commit. This is now just a followup to
💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2126699985)
> 2) to modify all call-sites to pass in a rate-limiting argument is cumbersome
Absolutely agree that would not be an improvement.
> I think using a boolean for rate-limiting vs no-rate-limiting is better than a uint64_t
I think either will work fine, the uint64_t generalizes a bit nicer but we probably won't be needing the generalization in the future, so whichever ends up being the most elegant implementation now is what I think we should go for.
> I am partial to introducing a new
...
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2126699985)
> 2) to modify all call-sites to pass in a rate-limiting argument is cumbersome
Absolutely agree that would not be an improvement.
> I think using a boolean for rate-limiting vs no-rate-limiting is better than a uint64_t
I think either will work fine, the uint64_t generalizes a bit nicer but we probably won't be needing the generalization in the future, so whichever ends up being the most elegant implementation now is what I think we should go for.
> I am partial to introducing a new
...
🤔 rkrux reviewed a pull request: "descriptors: MuSig2"
(https://github.com/bitcoin/bitcoin/pull/31244#pullrequestreview-2896780371)
Partial code review 981b2abd79e3a7a4701c9d8794057b5523acb177
I just started and am still reviewing.
(https://github.com/bitcoin/bitcoin/pull/31244#pullrequestreview-2896780371)
Partial code review 981b2abd79e3a7a4701c9d8794057b5523acb177
I just started and am still reviewing.
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126630415)
```diff
- "musig() is only allowed in tr()"
+ "musig() is only allowed in tr() and rawtr()"
```
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126630415)
```diff
- "musig() is only allowed in tr()"
+ "musig() is only allowed in tr() and rawtr()"
```
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126634254)
```diff
- musig(): musig() derivation requires all participants to be xpubs
+ musig(): derivation requires all participants to be xpubs or xprvs
```
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126634254)
```diff
- musig(): musig() derivation requires all participants to be xpubs
+ musig(): derivation requires all participants to be xpubs or xprvs
```
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126642131)
Bitwise operations on bools, sorry to be the one raising this multiple times. :)
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126642131)
Bitwise operations on bools, sorry to be the one raising this multiple times. :)
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126700491)
Can add `Assume` and throw if the derivation type is hardened, it's not allowed because IIUC, `m_derive` refers to the derivation type of this musig provider only.
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126700491)
Can add `Assume` and throw if the derivation type is hardened, it's not allowed because IIUC, `m_derive` refers to the derivation type of this musig provider only.
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126695356)
`IsRangedParticipants` is called multiple times in this class, can evaluate it once in the constructor itself and store the result in a const. I don't suppose a `MuSigPubkeyProvider` is updated with more `m_participants` after creation.
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126695356)
`IsRangedParticipants` is called multiple times in this class, can evaluate it once in the constructor itself and store the result in a const. I don't suppose a `MuSigPubkeyProvider` is updated with more `m_participants` after creation.
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126684020)
Currently, [BIP 390](https://github.com/bitcoin/bips/blob/master/bip-0390.mediawiki) doesn't have a mention of multipath participant keys. As per [BIP 389](https://github.com/bitcoin/bips/blob/master/bip-0389.mediawiki):
> Descriptors that contain multiple Key Expressions that each have a /<NUM;NUM;...;NUM> must have tuples of exactly the same length so that they are derived in lockstep in the same way that /* paths in multiple Key expressions are handled.
By using this lockstep approach,
...
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126684020)
Currently, [BIP 390](https://github.com/bitcoin/bips/blob/master/bip-0390.mediawiki) doesn't have a mention of multipath participant keys. As per [BIP 389](https://github.com/bitcoin/bips/blob/master/bip-0389.mediawiki):
> Descriptors that contain multiple Key Expressions that each have a /<NUM;NUM;...;NUM> must have tuples of exactly the same length so that they are derived in lockstep in the same way that /* paths in multiple Key expressions are handled.
By using this lockstep approach,
...
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126702224)
```diff
- Either way, we can passthrough to it's GetPubKey
+ Either way, we can passthrough to its GetPubKey
```
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126702224)
```diff
- Either way, we can passthrough to it's GetPubKey
+ Either way, we can passthrough to its GetPubKey
```
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126663894)
> There are some scenarios where I think it's ambiguous whether they should be allowed.
"Repeated participant public keys are not allowed. The aggregate public key is produced by using the KeyAgg algorithm on all KEYs specified in the expression after performing all specified derivation."
The current language^ in [BIP 390](https://github.com/bitcoin/bips/blob/master/bip-0390.mediawiki) gives me an impression that the public key derived from the KEY expression should not be repeated that su
...
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126663894)
> There are some scenarios where I think it's ambiguous whether they should be allowed.
"Repeated participant public keys are not allowed. The aggregate public key is produced by using the KeyAgg algorithm on all KEYs specified in the expression after performing all specified derivation."
The current language^ in [BIP 390](https://github.com/bitcoin/bips/blob/master/bip-0390.mediawiki) gives me an impression that the public key derived from the KEY expression should not be repeated that su
...
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126734205)
Nit: The calculation of aggregate public key & its provider can be done in the constructor itself unless it's anticipated that for some reason dummy MuSigPubkeyProviders will be used later.
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2126734205)
Nit: The calculation of aggregate public key & its provider can be done in the constructor itself unless it's anticipated that for some reason dummy MuSigPubkeyProviders will be used later.
👍 fanquake approved a pull request: "cmake: Replace deprecated `qt6_add_translation` with `qt6_add_lrelease`"
(https://github.com/bitcoin/bitcoin/pull/32651#pullrequestreview-2897024505)
ACK 18cf727429e967053d6a7468c9a40dc14185af34
(https://github.com/bitcoin/bitcoin/pull/32651#pullrequestreview-2897024505)
ACK 18cf727429e967053d6a7468c9a40dc14185af34