⚠️ zzzi2p opened an issue: "I2P: Change encryption type"
(https://github.com/bitcoin/bitcoin/issues/29197)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Similar to signature types, I2P supports multiple encryption types.
SAM defaults to the oldest type for both, for backward compatibility.
Unfortunately I forgot about this for encryption types.

The qbittorrent / libtorrent projects just discovered encryption types
in this issue:
https://github.com/qbittorrent/qBittorrent/issues/19625

The encryption type is a property of the sessi
...

💬 maflcko commented on pull request "build: remove `--enable-lto`":
(https://github.com/bitcoin/bitcoin/pull/29185#issuecomment-1880098822)
unrelated: I wonder if one of the sanitizer CI tasks should have LTO enabled. Otherwise it is just oss-fuzz, which enables it, so errors and warnings are easier to miss.

💬 jonatack commented on issue "I2P: Change encryption type":
(https://github.com/bitcoin/bitcoin/issues/29197#issuecomment-1880122065)
Thank you, @zzzi2p. Am looking now at reproducing and fixing this.

📝 reardencode opened a pull request: "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)"
(https://github.com/bitcoin/bitcoin/pull/29198)
This pull request contains a the same implementation of OP_CHECKTEMPLATEVERIFY (BIP119) as @jamesob's [Covenant Tools](https://github.com/bitcoin/bitcoin/pull/28550), an implementation of [OP_CHECKSIGFROMSTACK(VERIFY)](https://github.com/bitcoin/bips/pull/1535) and of [OP_INTERNALKEY](https://github.com/bitcoin/bips/pull/1534).

There are no testnet or mainnet activation parameters proposed in this pull request. I am deeply uninterested in the details of activation semantics.

This combinati
...

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