💬 brandonblack commented on issue "MuSig2 support":
(https://github.com/bitcoin/bitcoin/issues/23326#issuecomment-1813094549)
In FROST
> > > Is it possible to support descriptors that include only the "aggregated xpub" and not every key as in the proposed `agg()` expression, leading to a O(1) size instead of O(number-of-keys)?
> >
> >
> > You should be able to use Shamir secret sharing between the private key data so that a spending threshold can recover all pubkey data needed to reconstruct the agg() expression.
>
> As I understand it, the `agg()` expression requires all `n` public keys. I don't see how a t
...
(https://github.com/bitcoin/bitcoin/issues/23326#issuecomment-1813094549)
In FROST
> > > Is it possible to support descriptors that include only the "aggregated xpub" and not every key as in the proposed `agg()` expression, leading to a O(1) size instead of O(number-of-keys)?
> >
> >
> > You should be able to use Shamir secret sharing between the private key data so that a spending threshold can recover all pubkey data needed to reconstruct the agg() expression.
>
> As I understand it, the `agg()` expression requires all `n` public keys. I don't see how a t
...
💬 jonatack commented on pull request "p2p: peer connection bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28248#discussion_r1394675893)
After adding unit tests, this logic seems needed not in `GetAddedNodeInfo()` but only in `AddNode()`, where it is simpler and may be sufficient to compare by `CNetAddr` for cjdns entries only, in order to handle them correctly.
```diff
bool CConnman::AddNode(const AddedNodeParams& add)
{
- const CService resolved(LookupNumeric(add.m_added_node, GetDefaultPort(add.m_added_node)));
+ const CService resolved{MaybeFlipIPv6toCJDNS(LookupNumeric(add.m_added_node, GetDefaultPort(add.m_ad
...
(https://github.com/bitcoin/bitcoin/pull/28248#discussion_r1394675893)
After adding unit tests, this logic seems needed not in `GetAddedNodeInfo()` but only in `AddNode()`, where it is simpler and may be sufficient to compare by `CNetAddr` for cjdns entries only, in order to handle them correctly.
```diff
bool CConnman::AddNode(const AddedNodeParams& add)
{
- const CService resolved(LookupNumeric(add.m_added_node, GetDefaultPort(add.m_added_node)));
+ const CService resolved{MaybeFlipIPv6toCJDNS(LookupNumeric(add.m_added_node, GetDefaultPort(add.m_ad
...
💬 mzumsande commented on pull request "p2p: peer connection bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-1813126001)
> Maybe it would be prudent to allow feelers to addnodes.
I think it's fine to skip feelers for addnode peers. When we would skip these addresses for automatic connection, I don't see what making a feeler connection to them would achieve, considering that the point of feelers is to have better options for future automatic connections.
(https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-1813126001)
> Maybe it would be prudent to allow feelers to addnodes.
I think it's fine to skip feelers for addnode peers. When we would skip these addresses for automatic connection, I don't see what making a feeler connection to them would achieve, considering that the point of feelers is to have better options for future automatic connections.
💬 whitslack commented on pull request "policy: Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1813162870)
What's the point of having a non-malleable version? Okay, so a miner cannot add extra pushes to the input script that spends the anchor, but there is nothing to stop the miner from mining an alternative child transaction that spends *only* the anchor. Thus, the party that had broadcast the original child transaction will be forced to construct a new spending transaction with a different TxID anyway. Indeed, anyone (not only a miner) is free to propose a replacement child that spends *only* the a
...
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1813162870)
What's the point of having a non-malleable version? Okay, so a miner cannot add extra pushes to the input script that spends the anchor, but there is nothing to stop the miner from mining an alternative child transaction that spends *only* the anchor. Thus, the party that had broadcast the original child transaction will be forced to construct a new spending transaction with a different TxID anyway. Indeed, anyone (not only a miner) is free to propose a replacement child that spends *only* the a
...
📝 willcl-ark converted_to_draft a pull request: "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes"
(https://github.com/bitcoin/bitcoin/pull/28824)
Closes: #27795
Closes: #7996
Previously `ScriptToAsmStr` returned hex-encoded values, except if data length was <= 4 bytes, in which case it displayed using decimal encoding.
Remove the decimal encoding carve-out for small pushes and always display hex results from `decodescript` RPC.
There were [other suggestions](https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1692870456) for approaches in #27795, but IMO moving to hex-only-always makes the most sense.
(https://github.com/bitcoin/bitcoin/pull/28824)
Closes: #27795
Closes: #7996
Previously `ScriptToAsmStr` returned hex-encoded values, except if data length was <= 4 bytes, in which case it displayed using decimal encoding.
Remove the decimal encoding carve-out for small pushes and always display hex results from `decodescript` RPC.
There were [other suggestions](https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1692870456) for approaches in #27795, but IMO moving to hex-only-always makes the most sense.
👍 theuni approved a pull request: "doc: remove x86_64 build assumption from depends doc"
(https://github.com/bitcoin/bitcoin/pull/28884#pullrequestreview-1732859147)
ACK 821a8a11256ccf26fe8b0255cea4ec04dddd8f18
(https://github.com/bitcoin/bitcoin/pull/28884#pullrequestreview-1732859147)
ACK 821a8a11256ccf26fe8b0255cea4ec04dddd8f18
👋 denavila's pull request is ready for review: "wallet: Deniability API (Unilateral Transaction Meta-Privacy)"
(https://github.com/bitcoin/bitcoin/pull/27792)
(https://github.com/bitcoin/bitcoin/pull/27792)
💬 instagibbs commented on pull request "policy: Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1813235545)
> Thus, the party that had broadcast the original child transaction will be forced to construct a new spending transaction with a different TxID anyway
Canonical example here which motivated the change was that of LN channel splicing being used to cpfp bump another transaction, would could be another channel's commitment transaction itself. This requires the ephemeral anchor-spending tx(the splice) to retain txid stability otherwise funds are locked up in the new funding output.
I understa
...
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1813235545)
> Thus, the party that had broadcast the original child transaction will be forced to construct a new spending transaction with a different TxID anyway
Canonical example here which motivated the change was that of LN channel splicing being used to cpfp bump another transaction, would could be another channel's commitment transaction itself. This requires the ephemeral anchor-spending tx(the splice) to retain txid stability otherwise funds are locked up in the new funding output.
I understa
...
🤔 mzumsande reviewed a pull request: "net: Attempts to connect to all resolved addresses on `addnode`"
(https://github.com/bitcoin/bitcoin/pull/28834#pullrequestreview-1732970797)
Concept ACK, will review soon.
(https://github.com/bitcoin/bitcoin/pull/28834#pullrequestreview-1732970797)
Concept ACK, will review soon.
💬 psztorc commented on pull request "[WIP] BIP300 (Drivechains) consensus-level logic":
(https://github.com/bitcoin/bitcoin/pull/28311#issuecomment-1813260102)
Peter's "equivocation attack" is just him misunderstanding BMM -- specifically wrongly assuming it is like Namecoin (as he says in his linked article).
Unlike Namecoin, which is its own Blockchain and can survive on its own, BMM assumes that L1 can be found -- the L2 therefore has full access to all of L1 already. L2 already sees everything on L1, and its order.
(L2, of course, already sees everything on L2. And it knows what sidechain number it is.)
Unfortunately, I don't think there's anyt
...
(https://github.com/bitcoin/bitcoin/pull/28311#issuecomment-1813260102)
Peter's "equivocation attack" is just him misunderstanding BMM -- specifically wrongly assuming it is like Namecoin (as he says in his linked article).
Unlike Namecoin, which is its own Blockchain and can survive on its own, BMM assumes that L1 can be found -- the L2 therefore has full access to all of L1 already. L2 already sees everything on L1, and its order.
(L2, of course, already sees everything on L2. And it knows what sidechain number it is.)
Unfortunately, I don't think there's anyt
...
📝 kevkevinpal opened a pull request: "refactor: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/28885)
## In this PR I am addressing some comments in https://github.com/bitcoin/bitcoin/pull/27501 as a followup.
## Below are commits and corresponding comments they address
#### [80f7ccd](https://github.com/kevkevinpal/bitcoin/pull/6/commits/80f7ccdefda97cba9a7a913bf9c8ae19e368f7d9) test: Directly constructing 2 entry map for getprioritisedtransactions
- https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1219668280
- https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1219669146
...
(https://github.com/bitcoin/bitcoin/pull/28885)
## In this PR I am addressing some comments in https://github.com/bitcoin/bitcoin/pull/27501 as a followup.
## Below are commits and corresponding comments they address
#### [80f7ccd](https://github.com/kevkevinpal/bitcoin/pull/6/commits/80f7ccdefda97cba9a7a913bf9c8ae19e368f7d9) test: Directly constructing 2 entry map for getprioritisedtransactions
- https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1219668280
- https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1219669146
...
📝 TheCharlatan opened a pull request: "refactor: Replace sets of txiter with CTxMemPoolEntryRefs"
(https://github.com/bitcoin/bitcoin/pull/28886)
Currently the mempool returns and consumes sets of multiindex iterators in its public API. A likely motivation for this over working with references to the actual values is that the multi index interface works with these iterators and not with pointers or references to the actual values.
However, using the iterator type in the `setEntries` set provides little benefit in practice as applied currently. Its purpose, ownership, and safety semantics often remain ambiguous, and it is hardly used fo
...
(https://github.com/bitcoin/bitcoin/pull/28886)
Currently the mempool returns and consumes sets of multiindex iterators in its public API. A likely motivation for this over working with references to the actual values is that the multi index interface works with these iterators and not with pointers or references to the actual values.
However, using the iterator type in the `setEntries` set provides little benefit in practice as applied currently. Its purpose, ownership, and safety semantics often remain ambiguous, and it is hardly used fo
...
💬 mmelotti commented on pull request "refactor: Replace sets of txiter with CTxMemPoolEntryRefs":
(https://github.com/bitcoin/bitcoin/pull/28886#discussion_r1394888660)
thanks! really think entry is much better.
(https://github.com/bitcoin/bitcoin/pull/28886#discussion_r1394888660)
thanks! really think entry is much better.
💬 TheCharlatan commented on pull request "RFC: Remove boost usage from kernel api / headers":
(https://github.com/bitcoin/bitcoin/pull/28335#issuecomment-1813344340)
Rebased on #28886. which allowed for dropping one of the big commits in the previous patchset as well as slimming down the other patches.
(https://github.com/bitcoin/bitcoin/pull/28335#issuecomment-1813344340)
Rebased on #28886. which allowed for dropping one of the big commits in the previous patchset as well as slimming down the other patches.
💬 whitslack commented on pull request "policy: Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1813346081)
> This requires the ephemeral anchor-spending tx(the splice) to retain txid stability otherwise funds are locked up in the new funding output.
The point I was missing is that a non-malleable anchor ensures that the *other* output(s) do **not** get spent in a transaction whose TxID was not known in advance of signing. Thank you for the answer.
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1813346081)
> This requires the ephemeral anchor-spending tx(the splice) to retain txid stability otherwise funds are locked up in the new funding output.
The point I was missing is that a non-malleable anchor ensures that the *other* output(s) do **not** get spent in a transaction whose TxID was not known in advance of signing. Thank you for the answer.
💬 D33r-Gee commented on issue "26.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1813363758)
Great guide thanks for writing it!
would it be possible to remove the `$` to make the copying of the snippets work directly in terminal?
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1813363758)
Great guide thanks for writing it!
would it be possible to remove the `$` to make the copying of the snippets work directly in terminal?
💬 willcl-ark commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1813371106)
Marked as draft for now and pushed what I have worked on so far.
I haven't looked at implementing undecodable bytes or nested scripts yet.
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1813371106)
Marked as draft for now and pushed what I have worked on so far.
I haven't looked at implementing undecodable bytes or nested scripts yet.
💬 luke-jr commented on pull request "doc: fixup help output for -upnp and -natpmp":
(https://github.com/bitcoin/bitcoin/pull/28874#discussion_r1394978163)
This is incorrect if `DEFAULT_UPNP` were to be flipped. Maybe `Assert(DEFAULT_UPNP)`?
(https://github.com/bitcoin/bitcoin/pull/28874#discussion_r1394978163)
This is incorrect if `DEFAULT_UPNP` were to be flipped. Maybe `Assert(DEFAULT_UPNP)`?
🤔 mzumsande reviewed a pull request: "test/BIP324: functional tests for v2 P2P encryption"
(https://github.com/bitcoin/bitcoin/pull/24748#pullrequestreview-1733127844)
I had a look at `p2p_v2_earlykeyresponse.py` and it would fail the handshake rarely (but the test still passes because the error is thrown in the p2p thread!).
I could reproduce it by changing `GenerateRandomGarbage()` similar to TheStack above, but this time to large values:
`ret.resize(V2Transport::MAX_GARBAGE_LEN-19);` or larger. I haven't analyzed any further so far.
In general, it's annoying that asserts or failures in the p2p thread don't necessarily fail the functional test that spaw
...
(https://github.com/bitcoin/bitcoin/pull/24748#pullrequestreview-1733127844)
I had a look at `p2p_v2_earlykeyresponse.py` and it would fail the handshake rarely (but the test still passes because the error is thrown in the p2p thread!).
I could reproduce it by changing `GenerateRandomGarbage()` similar to TheStack above, but this time to large values:
`ret.resize(V2Transport::MAX_GARBAGE_LEN-19);` or larger. I haven't analyzed any further so far.
In general, it's annoying that asserts or failures in the p2p thread don't necessarily fail the functional test that spaw
...
💬 mzumsande commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1394961498)
one more instance of os.urandom (`random.randbytes(garbage_len)` should work)
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1394961498)
one more instance of os.urandom (`random.randbytes(garbage_len)` should work)