Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 maflcko commented on pull request "build: Bump guix time-machine to unlock riscv64 metal":
(https://github.com/bitcoin/bitcoin/pull/29078#issuecomment-1855611758)
Is there an easy way to print all affected/changed/bumped dependencies in the Bitcoin Core build graph?
💬 hebasto commented on pull request "Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1855630481)
Rebased and implemented @Sjors's [suggestions](https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1854029824).

Thank you @Sjors for testing!
🤔 Sjors reviewed a pull request: "wallet: Have the wallet store the key for automatically generated descriptors"
(https://github.com/bitcoin/bitcoin/pull/26728#pullrequestreview-1781522971)
059d847810051b52d33b5b16711a031c3fd199a9 looks good, except that you need to call `SetActiveHDKey()` in `CWallet::EncryptWallet` (to avoid needlessly running the upgrade code).

> Note that key rotation on encryption can happen only once in a wallet's lifetime - when the wallet is encrypted the first. We do not do key rotation when the passphrase changes.

TIL you can't turn off encryption once it's turned on (`walletpassphrasechange` does not allow `""` as the password, unlike `createwallet
...
💬 Sjors commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1426540509)
ce5495a4b27fe9646c9933dc39f0cb4e8510eaff: I still think it's nicer to check

```cpp
// Upgrade the wallet to have a global HD key, or in the special case
// where the user downgraded and then encrypted / decrypted.
if(!pwallet->IsWalletFlagSet(WALLET_FLAG_GLOBAL_HD_KEY) || enc_status != IsCrypted()) {
```

here rather than inside `UpgradeToGlobalHDKey`, so that it's clear to people reading this code when they need to open `wallet.cpp` and look at the function body.

Alternatively, you
...
💬 hebasto commented on pull request "Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1855651903)
> For anyone testing with HWI, consider also testing #24313.

Here is the combined [`231130-replace-bp+pr24313`](https://github.com/hebasto/bitcoin/tree/231130-replace-bp%2Bpr24313) branch.
💬 maflcko commented on pull request "doc: explain what the wallet password does":
(https://github.com/bitcoin/bitcoin/pull/28974#issuecomment-1855661692)
I wonder if this should instead be put in the GUI window and RPC help for the encrypt action, otherwise it seems easy to miss?

The GUI spreads the information over three pop-up windows, which doesn't seem great, when it can be put into just one Window.
📝 maflcko opened a pull request: "fuzz: Limit p2p fuzz targets to MAX_PROTOCOL_MESSAGE_LENGTH"
(https://github.com/bitcoin/bitcoin/pull/29079)
Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65039
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1426611988)
Thank you, `HasNonStandardInput` makes more sense, rename to `HasNonStandardInput` in 94cd47f3460
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1426612278)
Yes, fixed now
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1426612493)
Fixed in latest push
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1426612726)
Fixed
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1426612919)
Taken
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1426613203)
Yes, taken
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1426613876)
Thank you fixed in 94cd47f3460
💬 maflcko commented on pull request "refactor: Remove pre-C++20 code, fs::path cleanup":
(https://github.com/bitcoin/bitcoin/pull/29040#discussion_r1426614021)
> In theory, no conversions are actually required, right? In theory UniValue could use u8string instead of string internally, and other parts of the codebase could too, so nothing would need to be converted?

Not sure if it makes sense to update the whole codebase from `std::string` to `std::u8string`, just so that UniValue can accept an `std::u8string`. I think it would be fine for UniValue to accept both string types, though, then one would have to be "converted"/copied to the other (I don't
...
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1426614030)
Fixed
📝 hebasto opened a pull request: "ci: Set `HOMEBREW_NO_AUTO_UPDATE` to avoid unrelated failures"
(https://github.com/bitcoin/bitcoin/pull/29080)
CI builds, which use macOS image version `20231025.2`, suffer from Homebrew update failures.

This PR prevents such behavior.
💬 stickies-v commented on pull request "refactor: Remove pre-C++20 code, fs::path cleanup":
(https://github.com/bitcoin/bitcoin/pull/29040#issuecomment-1855708025)
> but I think I can get over it.

[🎅 🎄 ](https://github.com/stickies-v/bitcoin/commit/fa5ba5a4252a0af16e3845ac7d0e68f9010d2a54)
💬 ismaelsadeeq commented on pull request "ci: Set `HOMEBREW_NO_AUTO_UPDATE` to avoid unrelated failures":
(https://github.com/bitcoin/bitcoin/pull/29080#issuecomment-1855712986)
utACK f3800ba21914dc91174c096291690847c1b47a92