💬 michaelfolkson commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1880139852)
This should be opened to bitcoin-inquisition rather than this repo at this stage? I thought that was the whole point of bitcoin-inquisition.

I'm interested in why you think LN-Symmetry would be better implemented not using APO but perhaps that discussion can be had elsewhere.

🤔 ElGhaly35 reviewed a pull request: "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)"
(https://github.com/bitcoin/bitcoin/pull/29198#pullrequestreview-1807854857)
Appreciate still the server

💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1880144215)
> This should be opened to bitcoin-inquisition rather than this repo at this stage? I thought that was the whole point of bitcoin-inquisition.

Let's focus on code review. There is no strict process suggesting that code first flow through inquisition.

> I'm interested in why you think LN-Symmetry would be better implemented not using APO but perhaps that discussion can be had elsewhere.

Happy to discuss [on delving](https://delvingbitcoin.org/t/lnhance-bips-and-implementation/376)

🤔 jonatack reviewed a pull request: "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)"
(https://github.com/bitcoin/bitcoin/pull/29198#pullrequestreview-1807864193)
Concept ACK

💬 jonatack commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1444065001)
Per the Tidy CI task https://cirrus-ci.com/task/5701906522177536?logs=ci#L3173

```
script/interpreter.cpp:1320:27: error: use emplace_back instead of push_back [modernize-use-emplace,-warnings-as-errors]
1320 | stack.push_back(std::vector<unsigned char>{execdata.m_internal_key.begin(), execdata.m_internal_key.end()});
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~
|
...

🤔 moonsettler reviewed a pull request: "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)"
(https://github.com/bitcoin/bitcoin/pull/29198#pullrequestreview-1807867192)
Concept ACK

💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1444069529)
tyvm! fixed, rebased, and a couple of formatting things fixed.

💬 callebtc commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1444070159)
`case OP_NOP1:` was removed (by accident?). Should it be added back to L691?

💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#discussion_r1444070926)
Git diff. Making life unnecessarily hard sometimes.

🤔 1440000bytes reviewed a pull request: "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)"
(https://github.com/bitcoin/bitcoin/pull/29198#pullrequestreview-1807876376)
Concept ACK

CHECKSIGFROMSTACK [complements](https://github.com/bitcoin/bips/blob/8697678dd8cbe6f5a5aa1a291f682273240ab541/bip-csfs.mediawiki#motivation) CHECKTEMPLATEVERIFY and helps in ticking more boxes. INTERNALKEY saves 32 bytes in [some cases](https://github.com/bitcoin/bips/blob/4d448ccd02262b91c5d8c8bf91fb0fd4bc7dd5b1/bip-internalkey.mediawiki#motivation).

💬 jonatack commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1880180224)
In commit 0ac5cf9141441ea539d2f345d0d516d24a2f77bd

```
$ ./src/test/test_bitcoin -t transaction_tests
Running 7 test cases...
test/transaction_tests.cpp:192: error: in "transaction_tests/tx_valid": mapFlagNames is missing a script verification flag
```

💬 hsjoberg commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1880182827)
Concept ACK; this would make pretty much all sides happy.

💬 bordalix commented on pull request "RPC: add new `listmempooltransactions`":
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-1880184679)
Concept ACK

📝 jonatack opened a pull request: "net: create I2P sessions using both ECIES-X25519 and ElGamal encryption"
(https://github.com/bitcoin/bitcoin/pull/29200)
A Bitcoin Core node may only connect to a peer destination via I2P if both sides have sessions with the same encryption type. Encryption type is a property of the session, not the destination. Sessions may support multiple encryption types.

As Bitcoin Core is not currently setting the encryption type when creating I2P sessions, it uses the older default, ElGamal (type 0).

This pull updates our I2P session creation to use both ECIES-X25519 and ElGamal (types 4 and 0, respectively). This a
...

💬 zzzi2p commented on pull request "net: create I2P sessions using both ECIES-X25519 and ElGamal encryption":
(https://github.com/bitcoin/bitcoin/pull/29200#issuecomment-1880235978)
ACK 9d728916b27e18efc6f8839770ed5ec14789fc08

👍 theStack approved a pull request: "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo"
(https://github.com/bitcoin/bitcoin/pull/29058#pullrequestreview-1807916160)
ACK fb5bfed26a564014b83ccfc96ff00b630930fc61

Reviewed the code and verified the intended behavior for manual connections (`-connect`, `-addnode`) and addrfetch connections (`-seednode`) by specifying both a p2p-v2-supporting peer and a p2p-v1 peer in different bitcoind runs. For the v1 peer, the first (v2) connection attempt fails, and a successful v1 reconnect happens after, just as expected.

👍 kristapsk approved a pull request: "net: create I2P sessions using both ECIES-X25519 and ElGamal encryption"
(https://github.com/bitcoin/bitcoin/pull/29200#pullrequestreview-1807916410)
cr utACK 9d728916b27e18efc6f8839770ed5ec14789fc08

⚠️ Kongpc256 opened an issue: "The"
(https://github.com/bitcoin/bitcoin/issues/29201)

💬 ajtowns commented on pull request "test: Add test case for spending bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#discussion_r1444113496)
> 2/3 may require mining them into blocks directly if the outputs are non-standard to make

`self.generateblock(node, address, [tx.serialize().hex()])` is already mining the tx-to-be-spent directly into a block.

2-of-3 shouldn't make any difference vs 1-of-3 or even 1-of-1 afaics. If the tx-to-be-spent is already confirmed, there shouldn't be any problem having a spend of a 20-of-20 checkmultisig in the mempool, so I don't think n/m=4 is actually an edge case. I think a 0-of-0 checkmultisig
...

💬 ajtowns commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1880277106)
> I believe most of them use libbitcoinconsensus for testing script execution, with the only exception of `floresta`? While I found it beneficial for cross-testing complex scripts, the absence of core policy rule checking became a notable drawback.

Would something along the lines of https://github.com/ajtowns/bitcoin/commits/202309-evalscript/ be an interesting alternative?

```sh
$ ./bitcoin-util -script_flags=STANDARD evalscript '515193'
{
"script": {
"asm": "1 1 OP_ADD",
"
...