💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1890976319)
Force pushed to deconflict opcodes with VAULT, add deployment info output, and slightly improve tx_invalid tests.

💬 RandyMcMillan commented on pull request "Fix typos":
(https://github.com/bitcoin/bitcoin/pull/29246#issuecomment-1890985686)
ACK a1b1798b1652e93caf66e5e1c8353bcd7951f1c4

💬 TheCharlatan commented on pull request "refactor(tidy): Use C++20 contains method":
(https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1891007986)
Re https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1882971897

> but I'm not sure for this PR the benefits outweigh the costs?

FWIW I would approve this PR if it would only contain the changes that are automatically picked up by tidy and applied by running with `-fix`, making resolving potential conflicts purely manual.

💬 hebasto commented on issue "ubsan: misaligned-pointer-use in crc32c/src/crc32c_arm64.cc":
(https://github.com/bitcoin/bitcoin/issues/29178#issuecomment-1891012697)
I can reproduce the issue on Debian 11, aarch64.

💬 benthecarman commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1451787079)
does this mean you can do schnorr sigs with segwit v0 and p2sh? is that intended?

💬 benthecarman commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1451787098)
if schnorr is allowed in non-taproot then why forbid ecdsa for taproot?

💬 benthecarman commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1451787208)
msg size is also not verifiied here

💬 benthecarman commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1451787153)
should different message sizes be left for future upgrades

💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1451788888)
Yes, intended to support both ECDSA and BIP340 sigs in segwitv0 and legacy.

No need for message size check here because BIP340 supports arbitrary length message.

💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1451789060)
Decision was somewhat arbitrary on my part. ECDSA doesn't really add anything in Tapscrpit where Schnorr adds ability to use well specified MPC protocols like MuSig2 and FROST in legacy, so I settled on adding Schnorr for legacy but not adding ECDSA for Tapscript.

In Tapscript, I wonder if we'll eventually want to fork in 33-byte 02/03 keys as x-y Schnorr, so I was a bit hesitant to consume those prefixes now.

💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1451789140)
ECDSA requires 32-byte message. OP_SHA256 ahead would enable scripts to handle other data sizes. Making that explicit seems like the better option for ECDSA to enable shorter scripts when pre-hashing is not needed, but also allow pre-hashed data when needed.

💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1891047926)
Thanks for the comments @benthecarman !

💬 hebasto commented on pull request "wallet: Reset chain notifications handler if AttachChain fails":
(https://github.com/bitcoin/bitcoin/pull/29243#issuecomment-1891059064)
I've rerun the "Win64 native" CI job just to assure that the "wallet_assumeutxo.py --descriptors" test passes again.

💬 hebasto commented on issue "ubsan: misaligned-pointer-use in crc32c/src/crc32c_arm64.cc":
(https://github.com/bitcoin/bitcoin/issues/29178#issuecomment-1891063873)
A possible solution suggested in https://github.com/bitcoin-core/crc32c-subtree/pull/6.

💬 hebasto commented on pull request "Revert "build: Fix regression in "ARMv8 CRC32 intrinsics" test"":
(https://github.com/bitcoin/bitcoin/pull/29226#issuecomment-1891064076)
An alternative approach suggested in https://github.com/bitcoin-core/crc32c-subtree/pull/6.

💬 hebasto commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1891065737)
> Seems confusing to have a test runner that calls another runner (`all-lint.py`), which calls a subset of the lint tests.

However, it is quite convenient to run `./test/lint/all-lint.py` locally, no?

💬 alfonsoromanz commented on pull request "Fix typos":
(https://github.com/bitcoin/bitcoin/pull/29246#issuecomment-1891073266)
ACK a1b1798b1652e93caf66e5e1c8353bcd7951f1c4

📝 0xBEEFCAF3 opened a pull request: "Reenable OP_CAT "
(https://github.com/bitcoin/bitcoin/pull/29247)
Renabling OP_CAT, as per the [draft BIP](https://github.com/bitcoin/bips/pull/1525)

This MR involves reinstating the legacy opcode by replacing OP_SUCCESS126. Currently, there are no proposed activation parameters.

### Relevant Links
[Btcd implementation](https://github.com/btcsuite/btcd/pull/2095)
[Signet MR](https://github.com/bitcoin-inquisition/bitcoin/pull/39)
[Mailing list post](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-October/022049.html)

👍 andrewtoth approved a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1820309688)
ACK 6c603490023317a84c7c96ef4b64f4f96ea03d1f modulo nits

For `peerman_tests.cpp`, we should prefer list initialization and constness for variables when possible.

For commit message of `net: move state dependent peer services flags` (b934bfd327f68d39aebb2d9ab52b39b7cd29525d),
the last sentence could be:
```
Additionally, this encapsulation enables us
to customize the connections decision-making process based on
new users' configurations in the future.
```

💬 andrewtoth commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1451813298)
nit: `int64_t best_block_time{};`