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

👍 dergoegge approved a pull request: "p2p: Add witness mutation check inside FillBlock"
(https://github.com/bitcoin/bitcoin/pull/32646#pullrequestreview-2971152007)
Code review ACK 28299ce77636d7563ec545d043cf1b61bd2f01c1

👍 dergoegge approved a pull request: "test: Use msg_generic in p2p_ping.py"
(https://github.com/bitcoin/bitcoin/pull/32834#pullrequestreview-2971157730)
utACK fa3f100010f1a0273891ed0d14ddb3c5ccacbd4b