💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174849258)
> non-standardness of blocks

i.e. mined blocks that contain non-standard transactions - what's the accepted term for that?

> I don't get what you are asking here

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.

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174859753)
> We would not mine blocks with such transactions anymore, this is the whole point of this PR.

I don't see how this PR is doing that currently.
I understand that the point is to starve the miners of those pathological transactions before a future soft-fork makes the limit consensus-critical - but for now we have to make absolutely sure we don't accidentally do that and that it can still be mined, right? My question was if we can add that to the end of the test (which we can remove after we a
...

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174862347)
> your code suggestion is strictly functionally equivalent

Yes, that's why I have suggested it:
> I understand that this corresponds to an empty vector, but maybe we can simplify it in the test

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2174867410)
Can you please tell me about the automated check you are referring to? I saw a test in `test/functional/rpc_help.py` which does similar things.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174901563)
> The current implementation is ~66% slower

This is suggesting this PR makes transaction processing slower. It does not. The cost of adding an additional iteration through inputs is trivial compared to the total amount of work we do when processing a transaction. This seems silly to even state it. I don't know why you would fixate on this. Please let's spend our time on more important issues than removing the number of `for` loops used by the program.

> This repeats work currently

I'm a
...

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174904635)
I'm not planning on addressing this, as i think it would unnecessarily duplicate the check for no clear benefit.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174917931)
Exactly. I think it's good to keep the benchmark but its results should not be over interpreted. It is focused on a tiny portion of the total work performed when processing a transaction. I locally have benchmarks of `ProcessTransaction` which i may share in the future. To give you an idea, they suggest (as can be expected from just reading the code) that total work in processing a transaction is *several* orders of magnitude higher than the work done in `AreInputsStandard`. And that's not even
...

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174918740)
Will address if i need to retouch.

💬 dergoegge commented on pull request "test: Add `msgtype` to `msg_generic` slots":
(https://github.com/bitcoin/bitcoin/pull/32833#issuecomment-3018918291)
> Could add a small smoke test, maybe in ./test/functional/p2p_ping.py, but seems fine either way

Not gonna do it here, but if someone wants to do it in a follow up, I can review

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174930831)
> I don't see how this PR is doing that currently.

We only create block templates from the mempool. This PR prevents these transactions from being included in our mempool. As a result, we would never mine a block which include such transactions.

> but for now we have to make absolutely sure we don't accidentally do that and that it can still be mined, right?

I think what you are trying to say here is "that we would still accept a block containing them". That is, that we haven't accident
...

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174931525)
I won't be making this change.

🚀 fanquake merged a pull request: "rest: rename `strURIPart` to `uri_part`"
(https://github.com/bitcoin/bitcoin/pull/32825)

🚀 fanquake merged a pull request: "test: Add `msgtype` to `msg_generic` slots"
(https://github.com/bitcoin/bitcoin/pull/32833)

💬 fanquake commented on pull request "test: Add `msgtype` to `msg_generic` slots":
(https://github.com/bitcoin/bitcoin/pull/32833#issuecomment-3018971414)
Backported to `29.x` in #32810.

🤔 Ahz9619 reviewed a pull request: "[29.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/32810#pullrequestreview-2971058636)
#น้อมรับ

💬 Ahz9619 commented on issue "29.x Having qt(@6) breaks build for qt@5 on macOS 15.0 and 13.7":
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-3018987977)
> ### Is there an existing issue for this?
>
> - [X] I have searched the existing issues
>
> ### Current behaviour
>
> Building bitcoin-qt produces a flood of errors:
>
> ```
> In file included from /Users/sjors/dev/bitcoin/src/qt/csvmodelwriter.cpp:5:
> In file included from /Users/sjors/dev/bitcoin/src/qt/csvmodelwriter.h:8:
> In file included from /usr/local/opt/qt@5/lib/QtCore.framework/Headers/QList:1:
> In file included from /usr/local/opt/qt@5/lib/QtCore.framework/Headers/qlist.h:48:

...

🤔 Ahz9619 reviewed a pull request: "test: Add `msgtype` to `msg_generic` slots"
(https://github.com/bitcoin/bitcoin/pull/32833#pullrequestreview-2971068542)
test/functional/test_framework/messages.py

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174984218)
> I think what you are trying to say here is "that we would still accept a block containing them".

I wasn't just "trying", it's what I said.

> You can verify this through code review

I'd like a unit test to verify that instead, that's what I've been asking if we can extend the unit test to makes sure we still accept these non-standard transactions.

> functional test which checks newly-non-standard transactions are still accepted in blocks

Yes, that's what I was asking to add to th
...

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174984440)
You can also add a warmup round if you think that's more representative:
```C++
bench.warmup(1).run([&] {
bool success{AreInputsStandard(tx_1, coins)};
assert(success);
});
```

Though that doesn't change the numbers for me:

Before:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 140.41 | 7,122,118.63 | 0.4% | 1.10 | `CCo
...

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2174993741)
> this PR makes transaction processing slower. It does not

> This seems silly to even state it.

It's no silly, others have added a dedicated benchmark for this exact method.
And I don't appreciate this patronizing tone.