👍 alfonsoromanz approved a pull request: "wallet: Allow user to navigate options while encrypting at creation"
(https://github.com/bitcoin-core/gui/pull/722#pullrequestreview-1793165080)
Tested ACK https://github.com/bitcoin-core/gui/pull/722/commits/78660e72001a2561c7ad3026367a69f65414dbd9

<img width=50% height=50% alt="2" src="https://github.com/bitcoin-core/gui/assets/19962151/2c980809-10c1-43a2-bd59-bfe2bf42e285">
<img width=50% height=50% alt="3" src="https://github.com/bitcoin-core/gui/assets/19962151/202aeab0-734e-471c-b796-a5e819bc43ed">
<img width=50% height=50% alt="4" src="https://github.com/bitcoin-core/gui/assets/19962151/f7fafdf0-569d-4d96-83aa-94e0843c1a2e">
...

🤔 stratospher reviewed a pull request: "refactor: share and use `GenerateRandomKey` helper"
(https://github.com/bitcoin/bitcoin/pull/28455#pullrequestreview-1793127590)
Concept ACK. I think this is a useful refactor to have since it's more modular to have `GenerateRandomKey` in `key.cpp` rather than net code.

> I found these by doing grep -nri "MakeNewKey" ./src --binary-files=without-match
do we plan on making this modification to these files aswell?

@kevkevinpal, it makes sense to do this refactor when we're instantiating a CKey object and also filling the value together in the same line. we can't do this in arrays for example, where the instantiation
...

💬 stratospher commented on pull request "refactor: share and use `GenerateRandomKey` helper":
(https://github.com/bitcoin/bitcoin/pull/28455#discussion_r1434172881)
81187d0: we'd need to `#include <key.h>` since we're still using `GenerateRandomKey()` in `src/net.cpp`

💬 farahats9 commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1866531134)
Just wanted to add my 2 cents here as a layman using and holding bitcoin. If the transaction fee rates remain this high for someone using bitcoin to transfer money from wallet A to wallet B then I don't see a future for bitcoin.

The original goal of bitcoin is to make it a currency and a valued asset, if you sacrifice doing this in order to have secondary features then you will fail at both.

If this PR will fix the fee rates back to normal I am on board.

👍 theuni approved a pull request: "build: switch to using LLVM 17.x for macOS builds"
(https://github.com/bitcoin/bitcoin/pull/28880#pullrequestreview-1793294921)
ACK b335710782c2545e6eeed67b5e1763c07eab26b0.

Amazing detective work on the qt determinism fixes 🤯 !

I haven't tested or scrutinized to determine if that's the minimal patching we need. This seems low-impact enough it doesn't really matter if it's a bit heavy-handed.

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1434279569)
Good point, done.

💬 SomberNight commented on issue "macOS App Notarization":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1866611776)
Does the OS not phone home if the application is not notarized?
If so, how can it distinguish notarized+not_stapled vs not_notarized applications?

💬 ryanofsky commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866636473)
@Sjors and @S3RK, can you help me out by being specific about the flows you have in mind? Like in terms of what RPC calls users will make to do a particular operation? I'm trying to fill in the gaps myself, but in the cases I can think of, it seems like adding the concept of a sticky master key doesn't actually make things simpler, just more opaque and confusing.

I understand that if you were designing a new wallet model and API, you would both prefer an approach that required all active desc
...

💬 achow101 commented on pull request "wallet: fix key parsing check for miniscript expressions":
(https://github.com/bitcoin/bitcoin/pull/29027#issuecomment-1866645491)
ACK e1281f1bbd884f15d40053c9bc24794d0ce9a58a

🚀 achow101 merged a pull request: "wallet: fix key parsing check for miniscript expressions"
(https://github.com/bitcoin/bitcoin/pull/29027)

💬 achow101 commented on pull request "refactor: Print verbose serialize compiler error messages":
(https://github.com/bitcoin/bitcoin/pull/29056#issuecomment-1866659702)
ACK fae526345de539ab8f9b80100f6dfbe8e1d3284b

💬 TheCharlatan commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1866669498)
I'm getting an error during the guix build:
```
adding: bitcoin-b335710782c2/lib/libbitcoinconsensus-0.dll.dbg (deflated 76%)
INFO: Building b335710782c2 for platform triple x86_64-apple-darwin:
...using reference timestamp: 1702996424
...running at most 30 jobs
...from worktree directory: '/home/drgrid/Documents/bitcoin'
...bind-mounted in container to: '/bitcoin'
...in build directory: '/home/drgrid/Documents/bitcoin/guix-build-b335710782c2/distsrc-b
...

💬 fanquake commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1866673170)
Can you try cleaning up in your depends / work dirs? I've seen something similar after partial / interrupted builds (with changes unrelated to this one).

🚀 achow101 merged a pull request: "refactor: Print verbose serialize compiler error messages"
(https://github.com/bitcoin/bitcoin/pull/29056)

💬 theuni commented on pull request "build: Remove HAVE_CONSENSUS_LIB":
(https://github.com/bitcoin/bitcoin/pull/29123#issuecomment-1866692642)
@hebasto #24994 counted on linker side-effects to fix things IIRC. As do your other suggestions as far as I can see. If there's an issue with a compiler/stdlib symbol visibility, let's track it down and get the issues reported upstream as opposed to working around them with brittle workarounds.

🤔 stickies-v reviewed a pull request: "Nuke adjusted time (attempt 2)"
(https://github.com/bitcoin/bitcoin/pull/28956#pullrequestreview-1793314902)
Concept ACK, and code LGTM 87c577e9742d7154826c755a7fe320f34fd54c81 (but needs rebase).

It is unfortunate that this is not a strict security improvement (by increasing the dependency on the local system clock, which can also be tampered with), but not having consensus rely on a majority of arbitrary peers (phrased very well [here](https://github.com/bitcoin/bitcoin/pull/25908#issuecomment-1264632475)) is a worthwhile improvement and in my view probably safer. A wrong system clock should be pr
...

💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1434312496)
```suggestion
if (std::abs(median) > m_opts.time_offset_warn_threshold.count()) {
```

💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1434286266)
nit: Especially since there's no git log for this new code, this comment is pretty difficult to understand for future readers. Would suggest adding a link to e.g. https://github.com/bitcoin/bitcoin/commit/93659379bdedc29a87fe351ba2950a2c976aebd9 for context

💬 hebasto commented on pull request "build: Remove HAVE_CONSENSUS_LIB":
(https://github.com/bitcoin/bitcoin/pull/29123#issuecomment-1866699802)
@theuni
> If there's an issue with a compiler/stdlib symbol visibility, let's track it down and get the issues reported upstream as opposed to working around them with brittle workarounds.

https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1862877305:
> The underlying problem is described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36022, and it was not considered as a bug.

👍 hebasto approved a pull request: "build: Remove HAVE_CONSENSUS_LIB"
(https://github.com/bitcoin/bitcoin/pull/29123#pullrequestreview-1793422827)
ACK b37a171dbbcd9a7a8934099be1b508a0ea4f05a8.