📝 fanquake opened a pull request: "test: build Valgrind (3.20) from source & use Clang 16"
(https://github.com/bitcoin/bitcoin/pull/27444)
I was originally going to update to using Clang-16 & Valgrind 3.19 ([via Ubuntu 23.04](https://packages.ubuntu.com/lunar/valgrind)), however ran into issues with 3.19 & aarch64 (testing on top of #27364). Instead, I decided to switch to building the latest version of Valgrind from source (which takes minimal effort). This seems to have fixed all issues.
(https://github.com/bitcoin/bitcoin/pull/27444)
I was originally going to update to using Clang-16 & Valgrind 3.19 ([via Ubuntu 23.04](https://packages.ubuntu.com/lunar/valgrind)), however ran into issues with 3.19 & aarch64 (testing on top of #27364). Instead, I decided to switch to building the latest version of Valgrind from source (which takes minimal effort). This seems to have fixed all issues.
💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1162194379)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1162194379)
Fixed.
💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1162194453)
I've made this clearer by using `has_value()`.
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1162194453)
I've made this clearer by using `has_value()`.
📝 sipa opened a pull request: "Update src/secp256k1 subtree to release v0.3.1"
(https://github.com/bitcoin/bitcoin/pull/27445)
There is no urgent need for any of the changes in v0.3.1 (compared to the v0.3.0 that's currently subtreed), but if anyone may compile Bitcoin Core from source using Clang v14+, this will prevent known timing leaks in the signing/keygen logic.
(https://github.com/bitcoin/bitcoin/pull/27445)
There is no urgent need for any of the changes in v0.3.1 (compared to the v0.3.0 that's currently subtreed), but if anyone may compile Bitcoin Core from source using Clang v14+, this will prevent known timing leaks in the signing/keygen logic.
💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1162194642)
I prefer the `else if` style, but have collapsed this and `PurposeToString` to be more readable.
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1162194642)
I prefer the `else if` style, but have collapsed this and `PurposeToString` to be more readable.
💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1162194740)
Done.
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1162194740)
Done.
💬 achow101 commented on pull request "contrib: followups to #27358 (verify-binaries)":
(https://github.com/bitcoin/bitcoin/pull/27440#issuecomment-1502548625)
ACK ad841608d4edf6151b60e483793f60ba9f03cdbf
(https://github.com/bitcoin/bitcoin/pull/27440#issuecomment-1502548625)
ACK ad841608d4edf6151b60e483793f60ba9f03cdbf
🚀 achow101 merged a pull request: "contrib: followups to #27358 (verify-binaries)"
(https://github.com/bitcoin/bitcoin/pull/27440)
(https://github.com/bitcoin/bitcoin/pull/27440)
💬 achow101 commented on pull request "doc: correct sqlite & qrencode versions used in depenendencies.md":
(https://github.com/bitcoin/bitcoin/pull/27441#issuecomment-1502549784)
ACK a12d9cfa46ad5f5a5144daabbc146d0175642c69
(https://github.com/bitcoin/bitcoin/pull/27441#issuecomment-1502549784)
ACK a12d9cfa46ad5f5a5144daabbc146d0175642c69
🚀 achow101 merged a pull request: "doc: correct sqlite & qrencode versions used in depenendencies.md"
(https://github.com/bitcoin/bitcoin/pull/27441)
(https://github.com/bitcoin/bitcoin/pull/27441)
📝 benthecarman opened a pull request: "Allow configuirng target block time for a signet"
(https://github.com/bitcoin/bitcoin/pull/27446)
This pull request introduces the ability to configure the block time of a custom Bitcoin signet network, allowing developers to more easily simulate various network scenarios and test their applications in a controlled environment. The change helps to improve the flexibility of signet, making it more useful for diverse testing purposes. For example, I am trying to setup a signet with a 30 second block time and this caused a bunch of difficulty adjustments to happen making the network inconsisten
...
(https://github.com/bitcoin/bitcoin/pull/27446)
This pull request introduces the ability to configure the block time of a custom Bitcoin signet network, allowing developers to more easily simulate various network scenarios and test their applications in a controlled environment. The change helps to improve the flexibility of signet, making it more useful for diverse testing purposes. For example, I am trying to setup a signet with a 30 second block time and this caused a bunch of difficulty adjustments to happen making the network inconsisten
...
💬 ajtowns commented on pull request "Allow configuirng target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1502605806)
Configurable consensus parameters seems like a great way to make the network inconsistent in general (in particular, if you get the parameter wrong, you'll end up marking valid blocks invalid, with various consequences).
What's the actual problem you're trying to solve here? Would it work to have the signet miner only generate blocks when the mempool has entries, and sit idle when it's empty, for instance? If you're trying to generate many blocks, then regtest is already better as it doesn't
...
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1502605806)
Configurable consensus parameters seems like a great way to make the network inconsistent in general (in particular, if you get the parameter wrong, you'll end up marking valid blocks invalid, with various consequences).
What's the actual problem you're trying to solve here? Would it work to have the signet miner only generate blocks when the mempool has entries, and sit idle when it's empty, for instance? If you're trying to generate many blocks, then regtest is already better as it doesn't
...
✅ kallewoof closed a pull request: "rpc/doc: describe using combo(privkey) to get checksum and then list …"
(https://github.com/bitcoin/bitcoin/pull/24161)
(https://github.com/bitcoin/bitcoin/pull/24161)
✅ kallewoof closed a pull request: "rpc: add require_checksum flag to deriveaddresses"
(https://github.com/bitcoin/bitcoin/pull/24162)
(https://github.com/bitcoin/bitcoin/pull/24162)
💬 ajtowns commented on pull request "validation: Replace MinBIP9WarningHeight with MinBIP9WarningStartTime":
(https://github.com/bitcoin/bitcoin/pull/27427#discussion_r1162303007)
Setting it to `N` will prevent warnings that would otherwise be triggered by version bits being set in blocks up to `N+2015`; so you pick a value where a reorg back to `N+2015` or lower is sufficiently unlikely (for similar values of "sufficiently unlikely" as we'd use to bury successfully activated soft forks) and that `getblockchaininfo` from above that height doesn't already include an `Unknown new rules activated` warning. (If it does, it's obviously important to verify that the new rules th
...
(https://github.com/bitcoin/bitcoin/pull/27427#discussion_r1162303007)
Setting it to `N` will prevent warnings that would otherwise be triggered by version bits being set in blocks up to `N+2015`; so you pick a value where a reorg back to `N+2015` or lower is sufficiently unlikely (for similar values of "sufficiently unlikely" as we'd use to bury successfully activated soft forks) and that `getblockchaininfo` from above that height doesn't already include an `Unknown new rules activated` warning. (If it does, it's obviously important to verify that the new rules th
...
💬 ajtowns commented on pull request "validation: Replace MinBIP9WarningHeight with MinBIP9WarningStartTime":
(https://github.com/bitcoin/bitcoin/pull/27427#discussion_r1162304494)
(Maybe it would be better to do `std::max(MinBIP9WarningHeight, nMinerConfirmationWindow) - nMinerConfirmationWindow` in `WarningBitsConditionChecker::BeginTime()` ; then you're just ignoring version bits prior to `MinBIP9WarningHeight`)
(https://github.com/bitcoin/bitcoin/pull/27427#discussion_r1162304494)
(Maybe it would be better to do `std::max(MinBIP9WarningHeight, nMinerConfirmationWindow) - nMinerConfirmationWindow` in `WarningBitsConditionChecker::BeginTime()` ; then you're just ignoring version bits prior to `MinBIP9WarningHeight`)
💬 1440000bytes commented on pull request "Allow configuirng target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1162338423)
```suggestion
argsman.AddArg("-signetblocktime", "Difficulty adjustment will target a block time of the given amount in seconds (only for signet networks; defaults to 10 minutes)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);
```
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1162338423)
```suggestion
argsman.AddArg("-signetblocktime", "Difficulty adjustment will target a block time of the given amount in seconds (only for signet networks; defaults to 10 minutes)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);
```
💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1162375326)
Yes this is nice to have. Since the key format could never change due to backward compatibility requirements, this is not an issue. You can resolve this comment.
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1162375326)
Yes this is nice to have. Since the key format could never change due to backward compatibility requirements, this is not an issue. You can resolve this comment.
💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1162378013)
I guess that's ok, but `AddHDKey` already has an early return for existing keys.
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1162378013)
I guess that's ok, but `AddHDKey` already has an early return for existing keys.
👍 1440000bytes approved a pull request: "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet"
(https://github.com/bitcoin/bitcoin/pull/27279#pullrequestreview-1378615445)
utACK https://github.com/bitcoin/bitcoin/pull/27279/commits/7ccdd741fe1544c13b2a9b7baa5c5727e84d6e55
(https://github.com/bitcoin/bitcoin/pull/27279#pullrequestreview-1378615445)
utACK https://github.com/bitcoin/bitcoin/pull/27279/commits/7ccdd741fe1544c13b2a9b7baa5c5727e84d6e55