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

👍 theStack approved a pull request: "test: Treat msg_version.relay as unsigned, Remove `struct` packing in messages.py"
(https://github.com/bitcoin/bitcoin/pull/29067#pullrequestreview-1807941957)
Code-review ACK fa3b4dbf45b2b7ff1bdac6f68cf23275102cc775

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

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

💬 ajtowns commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1880309647)
Not an objection, but this feels backwards to me: presumably the user has manually listed those peers as addnode/connect because it's important that their node be connected to those peers, but if there are any bugs in the "v2 didn't work, retry as v1" logic, it's exactly those peers that will be inaccessible. But if we're confident there aren't bugs in that logic, why not just apply it to all peers? The "if you don't like it, just set -v2transport=0" approach to avoiding bugs/risk works just as
...