💬 Sjors commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#issuecomment-2701672307)
ACK 11a2d3a63e90cdc1920ede3c67d52a9c72860e6b

Mainnet params same as my last review except a size bump

For the test networks, I resynced all of them with `-assumevalid=0` and verifed the chain work for the new assume valid block and the `getchaintxstats` result. I didn't check the sizes.

👍 luke-jr approved a pull request: "torcontrol: Limit reconnect timeout to max seconds and log delay in whole seconds"
(https://github.com/bitcoin/bitcoin/pull/31979#pullrequestreview-2661968177)
utACK f708498293c27f63507cfa08c74909ba3d1aa675

💬 Sjors commented on pull request "doc: remove note about macOS self-signing":
(https://github.com/bitcoin/bitcoin/pull/32003#issuecomment-2701690670)
ACK c873ab6f23e027af1c5837256ce3c9eccaf409cb

The binaries we ship are signed and notarized now. If the user compiles from source, self-signing happens automatically.

If the user (cross) compiles from source to put it on a different computer, they'll still need to do this on the target machine. But this is probably not the right place for it.

Removing these instructions here is also safer: if someone messes with the binaries, the user will get an error from macOS. They might then follow
...

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