💬 hebasto commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1880038943)
> If someone created a separate pull request to bump xcode in the macOS CI, that'd be great.
Please see #29195.
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1880038943)
> If someone created a separate pull request to bump xcode in the macOS CI, that'd be great.
Please see #29195.
📝 hebasto opened a pull request: "ci: Do not set inane value for `LD_LIBRARY_PATH`"
(https://github.com/bitcoin/bitcoin/pull/29196)
For example, in the native macOS CI job, the `LD_LIBRARY_PATH=/Users/runner/work/bitcoin/bitcoin/depends/x86_64-apple-darwin/lib` makes no sense.
(https://github.com/bitcoin/bitcoin/pull/29196)
For example, in the native macOS CI job, the `LD_LIBRARY_PATH=/Users/runner/work/bitcoin/bitcoin/depends/x86_64-apple-darwin/lib` makes no sense.
💬 sipa commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1880059347)
utACK fb5bfed26a564014b83ccfc96ff00b630930fc61
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1880059347)
utACK fb5bfed26a564014b83ccfc96ff00b630930fc61
💬 theStack commented on pull request "refactor(tidy): Use C++20 contains method":
(https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1880061924)
Nice! Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1880061924)
Nice! Concept ACK
👍 theStack approved a pull request: "RPC/Blockchain: scanblocks: Accept named param for filter_false_positives"
(https://github.com/bitcoin/bitcoin/pull/29184#pullrequestreview-1807818372)
ACK 5779010ed7be1cbe9b98a91c7487d3d14b7cf24d
(https://github.com/bitcoin/bitcoin/pull/29184#pullrequestreview-1807818372)
ACK 5779010ed7be1cbe9b98a91c7487d3d14b7cf24d
💬 kristapsk commented on pull request "RPC: add new `listmempooltransactions`":
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-1880094062)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-1880094062)
Concept ACK
⚠️ 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
...
(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.
(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.
(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
...
(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.
(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
(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)
(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
(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()});
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~
|
...
(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
(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.
(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?
(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.
(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).
(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).