š¬ sipa commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177844858)
@l0rinc We should certainly not gratuitously make changes the worsen performance, but I think there have been reasonable arguments given here:
* It results in simpler code changes, specifically ones that impact consensus logic less.
* The slowdown in negligible compared the overall cost of transaction validation. It's good to be aware that there is a slowdown, but also reasonable to dismiss it on the basis that it's entirely justifiable in the presence of a much more important concern (reducin
...
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177844858)
@l0rinc We should certainly not gratuitously make changes the worsen performance, but I think there have been reasonable arguments given here:
* It results in simpler code changes, specifically ones that impact consensus logic less.
* The slowdown in negligible compared the overall cost of transaction validation. It's good to be aware that there is a slowdown, but also reasonable to dismiss it on the basis that it's entirely justifiable in the presence of a much more important concern (reducin
...
š¬ darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177851038)
> Do we have data on how many main-chain transactions historically exceed the new threshold of 2500 legacy sigops?
Iād like to confirm that the new policy would block no real-world traffic and is purely a preventive DoS measure ahead of any future BIP-54 soft-fork activation.
I don't think past usage is necessarily the best predictor, i think it's more convincing to reason through what real-world traffic could look like. The BIP [goes through this reasoning](https://github.com/bitcoin/bips/b
...
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177851038)
> Do we have data on how many main-chain transactions historically exceed the new threshold of 2500 legacy sigops?
Iād like to confirm that the new policy would block no real-world traffic and is purely a preventive DoS measure ahead of any future BIP-54 soft-fork activation.
I don't think past usage is necessarily the best predictor, i think it's more convincing to reason through what real-world traffic could look like. The BIP [goes through this reasoning](https://github.com/bitcoin/bips/b
...
š¬ saravadeanil commented on issue "[BTC signet v22.0] websocket not working as expect":
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024436337)
I am trying to perform rawtx using websocket.
It think it works on bitcoin testnet & mainnet network with version v22.0.0.
I am wondering why it's not working with signet.
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024436337)
I am trying to perform rawtx using websocket.
It think it works on bitcoin testnet & mainnet network with version v22.0.0.
I am wondering why it's not working with signet.
š¬ sipa commented on issue "[BTC signet v22.0] websocket not working as expect":
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024444002)
Can you elaborate on what you're using? Because as I mentioned, Bitcoin Core has no websocket support. You may be using JSON-RPC, or ZMQ, or a P2P connection...
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024444002)
Can you elaborate on what you're using? Because as I mentioned, Bitcoin Core has no websocket support. You may be using JSON-RPC, or ZMQ, or a P2P connection...
š¬ saravadeanil commented on issue "[BTC signet v22.0] websocket not working as expect":
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024449821)
@sipa Sorry for the confusion, I am using zmqpubrawtx port.
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024449821)
@sipa Sorry for the confusion, I am using zmqpubrawtx port.
š brunoerg opened a pull request: "test: check P2SH sigop count for coinbase tx"
(https://github.com/bitcoin/bitcoin/pull/32850)
We currently do not test that `GetP2SHSigOpCount` returns 0 for coinbase transactions (see line L129 at https://corecheck.dev/mutation/src/consensus/tx_verify.cpp). This PR addresses it.
(https://github.com/bitcoin/bitcoin/pull/32850)
We currently do not test that `GetP2SHSigOpCount` returns 0 for coinbase transactions (see line L129 at https://corecheck.dev/mutation/src/consensus/tx_verify.cpp). This PR addresses it.
š¬ l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177866217)
Thanks for the response, @sipa.
Is it unreasonable to ask for a benchmark showing that the slowdown is indeed negligible in the worst case (given that it's meant to avoid an attack)?
Benchmarks were added for this exact scenario, if they're not representative (I can agree with that) - let's make them representative first. My hunch is also that the slowdown is minor, but I would prefer more than a "just trust me broā¢" here...
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177866217)
Thanks for the response, @sipa.
Is it unreasonable to ask for a benchmark showing that the slowdown is indeed negligible in the worst case (given that it's meant to avoid an attack)?
Benchmarks were added for this exact scenario, if they're not representative (I can agree with that) - let's make them representative first. My hunch is also that the slowdown is minor, but I would prefer more than a "just trust me broā¢" here...
š¬ sipa commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177894356)
> Is it unreasonable to ask for a benchmark showing that the slowdown is indeed negligible in the worst case (given that it's meant to avoid an attack)?
Transactions can easily take milliseconds. A single signature check can take 10s of microseconds, and a single transaction input can do hundreds of those. The worst case is likely way worse than that still, which can easily be concluded without even thinking hard about it.
It's not unreasonable to ask for benchmarks when things aren't obvi
...
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177894356)
> Is it unreasonable to ask for a benchmark showing that the slowdown is indeed negligible in the worst case (given that it's meant to avoid an attack)?
Transactions can easily take milliseconds. A single signature check can take 10s of microseconds, and a single transaction input can do hundreds of those. The worst case is likely way worse than that still, which can easily be concluded without even thinking hard about it.
It's not unreasonable to ask for benchmarks when things aren't obvi
...
š¬ l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177901518)
Yes, I'm already running such a script, that's how I knew about the first coinjoin posted earlier.
Thanks for posting the result, I have one last question in this regard: I have of course read the bip-0054 reasoning, but given that this is meant to avoid an attack, I still wanted to understand how long it takes on a typical device to validate the remaining 2'500 signatures in the worst case? I'm asking because based on https://github.com/bitcoin/bitcoin/issues/32832 I want to know if I would
...
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177901518)
Yes, I'm already running such a script, that's how I knew about the first coinjoin posted earlier.
Thanks for posting the result, I have one last question in this regard: I have of course read the bip-0054 reasoning, but given that this is meant to avoid an attack, I still wanted to understand how long it takes on a typical device to validate the remaining 2'500 signatures in the worst case? I'm asking because based on https://github.com/bitcoin/bitcoin/issues/32832 I want to know if I would
...
š¬ l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177904988)
Thanks for the explanation, please resolve this comment.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177904988)
Thanks for the explanation, please resolve this comment.
š l0rinc approved a pull request: "policy: make pathological transactions packed with legacy sigops non-standard"
(https://github.com/bitcoin/bitcoin/pull/32521#pullrequestreview-2975960421)
Have a few remaining concerns, but removing my nack. Thanks for the explanations.
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32521#pullrequestreview-2975960421)
Have a few remaining concerns, but removing my nack. Thanks for the explanations.
Concept ACK
š¬ darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177915783)
As i am retouching i re-read this commit message and i didn't find any such typo. Could you be more precised? (Although i don't plan on addressing if i don't retouch again.)
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177915783)
As i am retouching i re-read this commit message and i didn't find any such typo. Could you be more precised? (Although i don't plan on addressing if i don't retouch again.)
š¬ Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3024553755)
> a PR that restricts default policy should have a corresponding post on the mailing list
Indeed.
> The primary motivation should be the same as what's in BIP 54: to avoid long validation times.
That's not necessarily true though. Transactions that exceed this limit, but are still below the maximum standardness size, are very expensive to make. It's not obvious that such an expense is worth it just to slow down a few nodes on the network once. But once the proposed soft fork is active,
...
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3024553755)
> a PR that restricts default policy should have a corresponding post on the mailing list
Indeed.
> The primary motivation should be the same as what's in BIP 54: to avoid long validation times.
That's not necessarily true though. Transactions that exceed this limit, but are still below the maximum standardness size, are very expensive to make. It's not obvious that such an expense is worth it just to slow down a few nodes on the network once. But once the proposed soft fork is active,
...
š¬ l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177922559)
> In addition, this limit may only be run into by pathological
transactions which pad the Script with sigops but do not use actual signatures when spending, as
otherwise they would run into the standard transaction size limit.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177922559)
> In addition, this limit may only be run into by pathological
transactions which pad the Script with sigops but do not use actual signatures when spending, as
otherwise they would run into the standard transaction size limit.
š¬ maflcko commented on pull request "bitcoin-cli: Add -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2177924690)
missed this?
(https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2177924690)
missed this?
š¤ pablomartin4btc reviewed a pull request: "doc: clarify that the "-j N" goes after the "--build build" part"
(https://github.com/bitcoin/bitcoin/pull/32846#pullrequestreview-2975993807)
ACK 0e9f409db3b7b08aef75ce39765b018b69cc8e9d
(https://github.com/bitcoin/bitcoin/pull/32846#pullrequestreview-2975993807)
ACK 0e9f409db3b7b08aef75ce39765b018b69cc8e9d
š¬ pablomartin4btc commented on pull request "doc: update unix build doc with build flags":
(https://github.com/bitcoin/bitcoin/pull/32269#discussion_r2177928251)
I do agree but that's consistent with most `doc/build*.md` files (`doc/build-freebsd.md`, `doc/build-netbsd.md`, `doc/build-openbsd.md`, `doc/build-osx.md`).
(https://github.com/bitcoin/bitcoin/pull/32269#discussion_r2177928251)
I do agree but that's consistent with most `doc/build*.md` files (`doc/build-freebsd.md`, `doc/build-netbsd.md`, `doc/build-openbsd.md`, `doc/build-osx.md`).
š¬ hebasto commented on pull request "depends: Override host compilers for FreeBSD and OpenBSD":
(https://github.com/bitcoin/bitcoin/pull/32716#issuecomment-3024565853)
This also is related: https://codeberg.org/guix/guix/issues/556.
(https://github.com/bitcoin/bitcoin/pull/32716#issuecomment-3024565853)
This also is related: https://codeberg.org/guix/guix/issues/556.
š¬ darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177930662)
Sure, makes sense.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177930662)
Sure, makes sense.
š¬ darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177930774)
Right, the witnesses are never inspected by `AreInputsStandard`. Dropped the unnecessary signing logic, to get a neat 13 lines reduction in the diff.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177930774)
Right, the witnesses are never inspected by `AreInputsStandard`. Dropped the unnecessary signing logic, to get a neat 13 lines reduction in the diff.