💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2701702223)
So that we're clear on this, here is a test of each of the binaries from master, 28.1, and this PR on latest MacOS on arm64. All binaries downloaded through Safari.

* 28.1 on MacOS 15.3.1 on Arm64
<details>
### Unsigned binaries:
* Finder double click:
* `bitcoin-cli`: Error dialog with "Apple could not verify "bitcoin-cli" is free of malware and may harm your Mac or compromise your privacy". Two buttons, "Done" and "Move to Trash".
* `bitcoin-qt`: Error dialog with "Apple could not
...

🤔 fjahr reviewed a pull request: "qa: clarify and document assumeutxo tests"
(https://github.com/bitcoin/bitcoin/pull/31907#pullrequestreview-2661853159)
Code review ACK e5ff4e416ecc8a51367022eb8a7a291f8cbc0c65

Would be fine for me to leave comments for when these files are retouched. The title and description of the PR should still be updated to reflect the new content though.

💬 fjahr commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1981855480)
This could probably use a hint about `VarIntMode`? Could be left for a follow-up though since maybe in the future we may want both.

💬 fjahr commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1981926993)
``` suggestion
# txid + coins per txid + vout + coin code (height*2 + coinbase)
```

💬 fjahr commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1981855608)
nit-ish: Reference is not fully correct

``` suggestion
# test cases from serialize_tests.cpp:varints_bitpatterns
```

💬 darosior commented on pull request "qa: clarify and document one assumeutxo test case with malleated snapshot":
(https://github.com/bitcoin/bitcoin/pull/31907#issuecomment-2701746604)
Thanks for the reviews. Updated title and description to only mention one of the test cases since i dropped the other commit. I don't think it's worth addressing the nits unless people feel too strongly.

💬 achow101 commented on pull request "doc: remove note about macOS self-signing":
(https://github.com/bitcoin/bitcoin/pull/32003#issuecomment-2701770015)
It may still be useful to mention `xattr -d com.apple.quarantine` for the binaries since it's necessary to do that to run them from finder. Although I'm not sure it even makes sense to try to run our binaries from finder. Even with `bitcoin-qt`, it still opens up a terminal that has to be open while the GUI is running.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2701794581)
@ryanofsky thanks. I'm probably going to leave that to followup, to preserve ACKs and limit the scope of this PR.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1982076973)
Fixed

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#issuecomment-2701888252)
Added a commit that should fix the fuzz failure.

💬 fjahr commented on pull request "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`":
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r1982113087)
Understood. I think I would have chosen a different name for the `ConstPubkeyProvider`, like `GetConstPrivKey` maybe, but completely optional.

💬 fjahr commented on pull request "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`":
(https://github.com/bitcoin/bitcoin/pull/31243#issuecomment-2701945658)
utACK 1bfabb4f4ec07c8fb0dfc2515d413b5196e01348

💬 hodlinator commented on pull request "RFC: Add `operator""_uint256` compile-time user-defined literal":
(https://github.com/bitcoin/bitcoin/pull/31991#discussion_r1982117901)
Thanks for trying that out!
I like that the reversal is more directly tied to the literal than when you were using `_hex`.

Hm... now that I compare:
```
"000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880"_uint256
```
and
```
uint256{"000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880"}
```
...does one character less really justify adding a literal?

Could shorten it to `_u256`, but that has a different meaning than `u8` in `_hex_u8` (one single unsigned
...

💬 l0rinc commented on pull request "RFC: Add `operator""_uint256` compile-time user-defined literal":
(https://github.com/bitcoin/bitcoin/pull/31991#discussion_r1982128012)
> does one character less really justify adding a literal?

I'm not counting chars, but that the noise is at the end now, it starts with the value.
I.e. that we see `.blockhash = "000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880"...` directly now.

💬 fjahr commented on pull request "Benchmark Chainstate::ConnectBlock duration":
(https://github.com/bitcoin/bitcoin/pull/31689#issuecomment-2701970565)
utACK 7edaf8b64cb2d59ada22042fee62a417e52368b8

💬 TheCharlatan commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)":
(https://github.com/bitcoin/bitcoin/pull/31989#discussion_r1982137766)
Are the TODOs going to be addressed here?

💬 mzumsande commented on pull request "fuzz: provide more realistic values to the base58(check) decoders":
(https://github.com/bitcoin/bitcoin/pull/31917#discussion_r1982140636)
this is changed back and fourth between the two commits, probably a rebase error.

💬 mzumsande commented on pull request "fuzz: provide more realistic values to the base58(check) decoders":
(https://github.com/bitcoin/bitcoin/pull/31917#discussion_r1982155510)
why is the check changed from `encoded_string.empty()` to `decoded.empty()` in the first commit - especially if this function is being rewritten in the next commit anyway?

💬 theuni commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2702063374)
Sorry for waiting to late to chime in on this, but I'm afraid I'm a concept NACK. We discussed this at length at CoreDev, and I'll briefly try to sum up my thoughts:

- I believe this is the wrong abstraction level. SockMan is essentially mimicking the behavior of a generic io multiplexing framework (libevent, libuv, etc.), but without the feature-set of those libs.
- The internals and api are heavily biased on Core's p2p layer, and aren't well-suited for abstracting. In particular, its event
...

💬 l0rinc commented on pull request "fuzz: provide more realistic values to the base58(check) decoders":
(https://github.com/bitcoin/bitcoin/pull/31917#discussion_r1982213549)
Thanks for the review. It was a rebase error indeed, thanks for catching it.