💬 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.

📝 maflcko opened a pull request: "test: Use msg_generic in p2p_ping.py"
(https://github.com/bitcoin/bitcoin/pull/32834)
It seems odd to derive `msg_pong_corrupt` from `msg_pong`, but then overwrite the serialize method, when one can just directly use `msg_generic` to pass the raw bytes to send over the wire.

Fix that by using `msg_generic`. This also serves as a regression test against the fix in commit 33480573cbd8d03aefbde100e51f827a2f7de7f7.

(Can be tested by reverting that commit to observe a failure)

💬 maflcko commented on pull request "test: Add `msgtype` to `msg_generic` slots":
(https://github.com/bitcoin/bitcoin/pull/32833#issuecomment-3019053963)
Done in https://github.com/bitcoin/bitcoin/pull/32834

💬 maflcko commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2175017840)
> 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.

Yes, that is the one I was referring to.

💬 Sameralkl commented on pull request "p2p: add more bad ports":
(https://github.com/bitcoin/bitcoin/pull/32826#issuecomment-3019063282)
*bc1quqsmwqwfcdyywc32eqzelqwu4gfvzlfx3cz7mx*

في الأحد، ٢٩ حزيران ٢٠٢٥, ٨:٢١ م DrahtBot ***@***.***> كتب:

> *DrahtBot* left a comment (bitcoin/bitcoin#32826)
> <https://github.com/bitcoin/bitcoin/pull/32826#issuecomment-3016878820>
>
> The following sections might be updated with supplementary metadata
> relevant to reviewers and maintainers.
> Code Coverage & Benchmarks
>
> For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32826.
> Reviews
>
> See the guideline
> <htt
...