💬 furszy commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1456421159)
I don't to see how this part of the message would be helpful to users. "file size being too small" does not tell me anything. Would be better to say that the file is corrupt and/or was crafted in an incompatible format. Also, no need to add the internal exception message to the user facing error.
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1456421159)
I don't to see how this part of the message would be helpful to users. "file size being too small" does not tell me anything. Would be better to say that the file is corrupt and/or was crafted in an incompatible format. Also, no need to add the internal exception message to the user facing error.
📝 instagibbs opened a pull request: "Add max_tx_weight to transaction funding options"
(https://github.com/bitcoin/bitcoin/pull/29264)
This allows a transaction's weight to be bound under
a certain weight if possible an desired. This
can be beneficial for future RBF attempts, or
whenever a more restricted spend topology is
desired.
See https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418/11?u=instagibbs for more discussion.
Seeking concept ACK and maybe some help on approach for testing.
(https://github.com/bitcoin/bitcoin/pull/29264)
This allows a transaction's weight to be bound under
a certain weight if possible an desired. This
can be beneficial for future RBF attempts, or
whenever a more restricted spend topology is
desired.
See https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418/11?u=instagibbs for more discussion.
Seeking concept ACK and maybe some help on approach for testing.
💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1456438606)
Makes sense, thanks!
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1456438606)
Makes sense, thanks!
💬 brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1896635863)
Thanks for the reviews! Since there is a consensus about being useful only for manual connections, I just changed the approach. The flags won't be applied for automatic connections, only for manual ones. I updated the title and description.
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1896635863)
Thanks for the reviews! Since there is a consensus about being useful only for manual connections, I just changed the approach. The flags won't be applied for automatic connections, only for manual ones. I updated the title and description.
💬 brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1456454845)
jsut changed the approach, thanks: https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1896635863
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1456454845)
jsut changed the approach, thanks: https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1896635863
📝 Christewart opened a pull request: "Add leaf_version parameter to `IsOpSuccess()`"
(https://github.com/bitcoin/bitcoin/pull/29265)
This PR adds a `leaf_version` parameter to `IsOpSuccess()`. This is needed to make continued progress on #29171 .
### Problem
As per [BIP341](https://github.com/bitcoin/bips/blob/master/bip-0342.mediawiki#rationale)
>Given that OP_SUCCESSx even causes potentially unparseable scripts to pass, it can be used to introduce multi-byte opcodes, or even a completely new scripting language when prefixed with a specific OP_SUCCESSx opcode.
This is occurring on #29171, in [script_assets_test.json
...
(https://github.com/bitcoin/bitcoin/pull/29265)
This PR adds a `leaf_version` parameter to `IsOpSuccess()`. This is needed to make continued progress on #29171 .
### Problem
As per [BIP341](https://github.com/bitcoin/bips/blob/master/bip-0342.mediawiki#rationale)
>Given that OP_SUCCESSx even causes potentially unparseable scripts to pass, it can be used to introduce multi-byte opcodes, or even a completely new scripting language when prefixed with a specific OP_SUCCESSx opcode.
This is occurring on #29171, in [script_assets_test.json
...
💬 Christewart commented on pull request "Add leaf_version parameter to `IsOpSuccess()`":
(https://github.com/bitcoin/bitcoin/pull/29265#discussion_r1456471781)
For unknown leaf_versions, do we want to return true by default?
(https://github.com/bitcoin/bitcoin/pull/29265#discussion_r1456471781)
For unknown leaf_versions, do we want to return true by default?
💬 furszy commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#discussion_r1456489383)
These methods doesn't seems to fit inside the `WalletStorage` interface. What do you think about creating a new interface for encrypting/decrypting data, then provide it to each spkm?
(https://github.com/bitcoin/bitcoin/pull/28774#discussion_r1456489383)
These methods doesn't seems to fit inside the `WalletStorage` interface. What do you think about creating a new interface for encrypting/decrypting data, then provide it to each spkm?
💬 TheCharlatan commented on pull request "build: always set `-g -O2` in `CORE_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29205#issuecomment-1896833001)
Why not change the behavior around the `CORE_CFLAGS` as well? That seems a bit confusing to me; we now have one variable that always retains `-g -O2` when overriding and another that does not.
(https://github.com/bitcoin/bitcoin/pull/29205#issuecomment-1896833001)
Why not change the behavior around the `CORE_CFLAGS` as well? That seems a bit confusing to me; we now have one variable that always retains `-g -O2` when overriding and another that does not.
💬 sipa commented on pull request "Add leaf_version parameter to `IsOpSuccess()`":
(https://github.com/bitcoin/bitcoin/pull/29265#issuecomment-1896903940)
I think you'd want to pass SigVersion rather than leaf_version. The SigVersion defines what script/signature rules are active, while the leaf_version is one of several ways of encoding that information in scripts. It also answers your comment: we don't care about unknown leaf_versions because they will never occur (under an unknown leaf version, no script execution happens at all).
That said, I think a change like this belongs in the PR that implements a consensus change that needs it. It's gen
...
(https://github.com/bitcoin/bitcoin/pull/29265#issuecomment-1896903940)
I think you'd want to pass SigVersion rather than leaf_version. The SigVersion defines what script/signature rules are active, while the leaf_version is one of several ways of encoding that information in scripts. It also answers your comment: we don't care about unknown leaf_versions because they will never occur (under an unknown leaf version, no script execution happens at all).
That said, I think a change like this belongs in the PR that implements a consensus change that needs it. It's gen
...
💬 Christewart commented on pull request "Add leaf_version parameter to `IsOpSuccess()`":
(https://github.com/bitcoin/bitcoin/pull/29265#issuecomment-1896953936)
> That said, I think a change like this belongs in the PR that implements a consensus change that needs it. It's generally better not
to touch consensus code unless necessary.
Small quibble here, there are other consensus PRs out there that probably would need this change ( #29247 , #29050 , #28550 ).
Its unclear to me right now if I was just "lucky" with my opcode byte selection such that I ran into a test case that failed with witness redeemScript (`4edc`). Perhaps the test vectors are
...
(https://github.com/bitcoin/bitcoin/pull/29265#issuecomment-1896953936)
> That said, I think a change like this belongs in the PR that implements a consensus change that needs it. It's generally better not
to touch consensus code unless necessary.
Small quibble here, there are other consensus PRs out there that probably would need this change ( #29247 , #29050 , #28550 ).
Its unclear to me right now if I was just "lucky" with my opcode byte selection such that I ran into a test case that failed with witness redeemScript (`4edc`). Perhaps the test vectors are
...
💬 ryanofsky commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1896988088)
Concept ACK. This change removes removes `g_signals` and lets kernel applications call validation functions with separate schedulers so signals from different functions don't need to interfere with each other. It also lets kernel applications define their own scheduling logic, which may be useful if they want to control when notifications are processed.
I think naming and organization could be improved though. Like `kernel::ValidationInterfaceQueue` seems seems like it should be called `util:
...
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1896988088)
Concept ACK. This change removes removes `g_signals` and lets kernel applications call validation functions with separate schedulers so signals from different functions don't need to interfere with each other. It also lets kernel applications define their own scheduling logic, which may be useful if they want to control when notifications are processed.
I think naming and organization could be improved though. Like `kernel::ValidationInterfaceQueue` seems seems like it should be called `util:
...
:lock: fanquake locked an issue: "bitcoin.org still lists laanwj's now-revoked PGP key"
(https://github.com/bitcoin/bitcoin/issues/29257)
(https://github.com/bitcoin/bitcoin/issues/29257)
👋 ryanofsky's pull request is ready for review: "Improve new LogDebug/Trace/Info/Warning/Error Macros"
(https://github.com/bitcoin/bitcoin/pull/29256)
(https://github.com/bitcoin/bitcoin/pull/29256)
💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#issuecomment-1897498885)
Addressed @furszy's [feedback](https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1456421159).
(https://github.com/bitcoin/bitcoin/pull/28670#issuecomment-1897498885)
Addressed @furszy's [feedback](https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1456421159).
💬 ariard commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1897647775)
> This new max_offered_htlc parameter is unnecessary. An HTLC sender unilaterally decides whether they send HTLCs or
> not, and can ensure that they never send more outgoing HTLCs than their max_accepted_htlcs. That has always been
> eclair's behavior by the way: our default is max_accepted_htlcs = 30, we have many peers that set their
> max_accepted_htlcs = 483, but we never offer them more than 30 HTLCs.
Good if eclair is already limiting the number of outgoing HTLCs. I dont know if all
...
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1897647775)
> This new max_offered_htlc parameter is unnecessary. An HTLC sender unilaterally decides whether they send HTLCs or
> not, and can ensure that they never send more outgoing HTLCs than their max_accepted_htlcs. That has always been
> eclair's behavior by the way: our default is max_accepted_htlcs = 30, we have many peers that set their
> max_accepted_htlcs = 483, but we never offer them more than 30 HTLCs.
Good if eclair is already limiting the number of outgoing HTLCs. I dont know if all
...
💬 ariard commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1897677061)
Let's do the anti-pinning analysis with the most restrictve package vbyte size:
- 1 commitment tx 268 bytes
- 1 child 1kb
Package size: 1268 bytes.
This is assuming a commitment tx with only 1 HTLC output at max.
With mempool backlog superior to 20 sat/byte and depth=`cltv_expiry_delta`, one can pin 25k sats payment.
Adversary cost: 5360 (commitment tx fee) + 1000 (child rbf-penalty) + 43 (channel opening output)
Adversary gain (per-channel): 18597 satoshis.
If we reduce the size
...
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1897677061)
Let's do the anti-pinning analysis with the most restrictve package vbyte size:
- 1 commitment tx 268 bytes
- 1 child 1kb
Package size: 1268 bytes.
This is assuming a commitment tx with only 1 HTLC output at max.
With mempool backlog superior to 20 sat/byte and depth=`cltv_expiry_delta`, one can pin 25k sats payment.
Adversary cost: 5360 (commitment tx fee) + 1000 (child rbf-penalty) + 43 (channel opening output)
Adversary gain (per-channel): 18597 satoshis.
If we reduce the size
...
⚠️ YezikTech opened an issue: "Lupe Fiasco - Superstar (Live Lounge 3)"
(https://github.com/bitcoin/bitcoin/issues/29268)
https://youtu.be/9TMmPEfRjhs?si=nXqKYFGQcEUvfZ4Z
(https://github.com/bitcoin/bitcoin/issues/29268)
https://youtu.be/9TMmPEfRjhs?si=nXqKYFGQcEUvfZ4Z
✅ achow101 closed an issue: "Lupe Fiasco - Superstar (Live Lounge 3)"
(https://github.com/bitcoin/bitcoin/issues/29268)
(https://github.com/bitcoin/bitcoin/issues/29268)
:lock: achow101 locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/29268)
(https://github.com/bitcoin/bitcoin/issues/29268)