💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#issuecomment-1865704915)
thanks for the thoughtful feedback @amitiuttarwar, @0xB10C! I've updated the PR to address your comments.

Main changes were: ([git diff](https://github.com/bitcoin/bitcoin/compare/d3c24183483e43dd38209dbaf3c4ac725e13d4a3..1b49149b3ed7366141f0090692256e82032cb9af))
1. adding an option to restart the node with an empty addrman (method 1 suggested here https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1418564026) to prevent addpeeraddress test leaking into getaddrmaninfo and getrawaddr
...

💬 jonasschnelli commented on issue "macOS App Notarization":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865735331)
@maaku
My tests in 2020 (https://github.com/bitcoin/bitcoin/pull/18187#issuecomment-592453829) showed me, that regardless of stapling, macOS does an onlinecheck of the notarization (if the user has internet connection).

Not sure if that has changed. Might be worth to redo https://github.com/bitcoin/bitcoin/pull/18187#issuecomment-592453829.

That unnecessary "calling-apple" was the reason to not do notarization back in 2020.

💬 maaku commented on issue "macOS App Notarization":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1865811312)
@jonasschnelli Sounds like bug 57278824, which was fixed back in 2020 just a few months after you ran your test:

> There’s one further wrinkle here: Once Brigitte removed the com.apple.security.cs.disable-library-validation entitlement from their app the problem didn’t go away! And that’s the result of a historical bug in Gatekeeper (r. 57278824). Prior to 10.15.4 Gatekeeper does not recognise the implied library validation that you get when you enabled the hardened runtime. So, even though y
...

:lock: fanquake locked an issue: "Transfer from closed coinbase acc."
(https://github.com/bitcoin/bitcoin/issues/29126)

💬 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.