💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1865817132)
IIUC the approach proposed by @ryanofsky is to basically figure out the key on demand instead of storing it in the DB.

So this works for generating tr() descriptors for existing wallets, but it doesn't work that well for #24861. The desired flow is to have a wallet with HD key but without descriptors and then retrieve an xpub to construct multisig descriptor. How would it work if we don't have an HD key associated with the wallet?

In my opinion, approach proposed in this PR resolves a regr
...

💬 naumenkogs commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1865838565)
ACK 87c577e9742d7154826c755a7fe320f34fd54c81

💬 naumenkogs commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1433684385)
dde4e1c6b0cbb49b84e75b9d0d1a92161ba5a499
if `m_index` is 4, there are 4 samples but we will still compute the median, so the comment is inaccurate.

💬 naumenkogs commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1433686298)
83c790e3abd7d2434894fb964c21796f1d024e9b

the comment could be more useful, rephrasing the meaning of this info.

💬 mmgen commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1865905242)
> @conduition No need to switch to a different inscription form, assuming you mean 100% of non-mining nodes adopt it. The mining nodes (miners) are financially incentivized to accept the inscriptions, so they will not adopt such code, so inscriptions txs will be sent directly to miners out of band (such as via API) to dark/private miner mempools.

Yes, this is the most important point, the reason that this patch will accomplish absolutely nothing.

💬 Sjors commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1865915848)
@achow101 wrote:

> I've tried a couple of different approaches, and they all have drawbacks in some form:
>
> Generate a new hd key and use that for the descriptor.

Another drawback of that approach is that it requires a new backup, e.g. for anyone who just wants to add taproot support to a wallet that doesn't have it yet.

@s3rk wrote:

> The desired flow is to have a wallet with HD key but without descriptors and then retrieve an xpub to construct multisig descriptor. How would it
...

💬 fanquake commented on issue "macOS App Notarization":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1866001929)
> Not if the notarization is stapled to the binary, no. That should prevent phoning home. I have not used Wireshark to verify this, but that's what the documentation claims.

Can you link to this documentation you're talking about? Or is that the developer thread above?

Reading https://developer.apple.com/documentation/security/notarizing_macos_software_before_distribution/customizing_the_notarization_workflow#3087720, it's not clear that no phone home happens if there is a ticket present,
...

💬 gruve-p commented on issue "macOS App Notarization":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1866038316)
Might be relevant:

- https://blog.jacopo.io/en/post/apple-ocsp/
- https://lapcatsoftware.com/articles/ocsp.html

✅ glozow closed a pull request: "test: fix typo"
(https://github.com/bitcoin/bitcoin/pull/29121)

💬 glozow commented on pull request "test: fix typo":
(https://github.com/bitcoin/bitcoin/pull/29121#issuecomment-1866076264)
Thanks for your interest in Bitcoin Core! Feel free to read CONTRIBUTING.md for advice on contributing https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#pull-request-philosophy

✅ glozow closed a pull request: "Fix spelling errors"
(https://github.com/bitcoin/bitcoin/pull/29107)

💬 glozow commented on pull request "Fix spelling errors":
(https://github.com/bitcoin/bitcoin/pull/29107#issuecomment-1866082831)
I'm going to close this in an effort focus review attention on more important PRs, sorry (see https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#pull-request-philosophy). I think it makes more sense to fix typos as we work on those areas of code.

💬 fanquake commented on pull request "Use hardened runtime on macOS release builds.":
(https://github.com/bitcoin/bitcoin/pull/29127#issuecomment-1866088324)
> The release maintainer, or any authorized Apple Developer, will need to run xcrun notarytool

How does someone do this on Linux, without macOS hardware/Xcode?

💬 glozow commented on pull request "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)":
(https://github.com/bitcoin/bitcoin/pull/28175#issuecomment-1866099123)
Agree with the intent of avoiding legal problems, but NACK on adding this text. Unless we have some kind of legal guidance saying this text would protect us beyond what our existing license-related docs say, I don't see any reason to discourage specific tools in the contributing guidelines. I agree with the above that trying to speculate/innovate can do more harm than good.

I think we should close this for now and reconsider if/when a a lawyer advises us to do something like this.

💬 hebasto commented on pull request "build: Remove HAVE_CONSENSUS_LIB":
(https://github.com/bitcoin/bitcoin/pull/29123#issuecomment-1866125765)
> Since it is always part of the internal library, the `HAVE_CONSENSUS_LIB` define used by the tests is essentially
> meaningless, since the module is always available.

It seems worth noting that we do not use the `HAVE_CONSENSUS_LIB` macro in `/src/test/fuzz/script_bitcoin_consensus.cpp` in the master branch.

💬 kristapsk commented on pull request "Fix spelling errors":
(https://github.com/bitcoin/bitcoin/pull/29107#issuecomment-1866131660)
IMHO it would been simpler to just merge such trivial changes, but ok.

💬 hebasto commented on pull request "build: Remove HAVE_CONSENSUS_LIB":
(https://github.com/bitcoin/bitcoin/pull/29123#issuecomment-1866158665)
Concept ACK on moving the `script/bitcoinconsensus.cpp` out from the internal `libbitcoin_consensus.a` library.

@theuni

> I think another way of looking at this is that `script/bitcoinconsensus.cpp` is moving out into its own convenience lib, it just happens to be a single file. We could imagine if the api were written in multiple translation units, we'd actually want to build that convenience lib.

That is very similar to what I suggested in https://github.com/bitcoin/bitcoin/pull/2499
...

💬 furszy commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1866160684)
> Yes, that's what the compiler error says, but that function doesn't seem to be in that namespace. Its other callers don't qualify it, and if I do qualify it, the build fails for me.

Seems that the function was moved to the `util` namespace on #28075.
Try cleaning up your project and rebasing it on master. (git clean -fdx && git pull --rebase origin master && make).
Will need to do some modifications here. `LockDirectory()` does not return a boolean anymore.

📝 brunoerg opened a pull request: "wallet, rpc: add BIP44 `account` in `createwallet` "
(https://github.com/bitcoin/bitcoin/pull/29129)
This PR adds an `account` parameter in `createwallet` RPC. It's the
BIP44 account that will be used in `SetupDescriptorScriptPubKeyMans`
to fetch the descriptors from the external signer.

👋 fanquake's pull request is ready for review: "build: switch to using LLVM 17.x for macOS builds"
(https://github.com/bitcoin/bitcoin/pull/28880)