💬 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)
💬 mzumsande commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1394911231)
Is it on purpose to use `super()` above and provide args here and below? Not an expert on this kind of stuff, but [this](https://stackoverflow.com/questions/59538746/when-do-you-need-to-pass-arguments-to-python-super) suggests that providing args is not necessary in python3.
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1394911231)
Is it on purpose to use `super()` above and provide args here and below? Not an expert on this kind of stuff, but [this](https://stackoverflow.com/questions/59538746/when-do-you-need-to-pass-arguments-to-python-super) suggests that providing args is not necessary in python3.
💬 luke-jr commented on pull request "doc: fixup help output for -upnp and -natpmp":
(https://github.com/bitcoin/bitcoin/pull/28874#discussion_r1394985174)
This loses the "when listening and no -proxy" which is AFAIK still true?
(https://github.com/bitcoin/bitcoin/pull/28874#discussion_r1394985174)
This loses the "when listening and no -proxy" which is AFAIK still true?
💬 denavila commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#discussion_r1394987489)
I added RBF spoofing in the latest commit.
(https://github.com/bitcoin/bitcoin/pull/27792#discussion_r1394987489)
I added RBF spoofing in the latest commit.
💬 luke-jr commented on pull request "doc: fixup help output for -upnp and -natpmp":
(https://github.com/bitcoin/bitcoin/pull/28874#discussion_r1394988966)
```suggestion
argsman.AddArg("-upnp", strprintf("Use UPnP to map the listening port (default: %s)", !Assert(!DEFAULT_UPNP)), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
```
Otherwise it would be incorrect (missing "when listening and no -proxy") if `DEFAULT_UPNP` flips to true.
Alternatively, just remove the SoftSetArg stuff...
(https://github.com/bitcoin/bitcoin/pull/28874#discussion_r1394988966)
```suggestion
argsman.AddArg("-upnp", strprintf("Use UPnP to map the listening port (default: %s)", !Assert(!DEFAULT_UPNP)), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
```
Otherwise it would be incorrect (missing "when listening and no -proxy") if `DEFAULT_UPNP` flips to true.
Alternatively, just remove the SoftSetArg stuff...
💬 luke-jr commented on pull request "refactor: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1394990462)
This isn't a refactor.
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1394990462)
This isn't a refactor.
🤔 luke-jr requested changes to a pull request: "gui: getrawtransaction implementation"
(https://github.com/bitcoin-core/gui/pull/777#pullrequestreview-1733238681)
Needs a lot of UX work. Definitely doesn't belong on the Help menu...
(https://github.com/bitcoin-core/gui/pull/777#pullrequestreview-1733238681)
Needs a lot of UX work. Definitely doesn't belong on the Help menu...
💬 sipa commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#discussion_r1394993981)
I think you want `fRequireMinimal=true` true here, otherwise it'll accept non-minimally-encoded values too.
(https://github.com/bitcoin/bitcoin/pull/28824#discussion_r1394993981)
I think you want `fRequireMinimal=true` true here, otherwise it'll accept non-minimally-encoded values too.