🤔 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

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2175046245)
Fixed in #32825 (rebased to resolve a small conflict).

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3019110833)
Rebased over `master` after https://github.com/bitcoin/bitcoin/pull/32825 is merged (to resolve a small conflict).

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

Checked that this catches the issue fixed in #32833 as expected (i.e. p2p_ping.py fails if the `msgtype` slot is removed again from `msg_generic`).

💬 l0rinc commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3019159919)
> Could clarify in the title or in the text that this is for blocks after assumevalid?

Did I miscalculate it? Since the benchmark was running until 900k blocks of which only 13843 blocks needed script validation (unless I ran into https://github.com/bitcoin/bitcoin/issues/31494), isn't that 13843/900000=0.0153811111 i.e. 1.5% of the blocks had any script validation, yet almost 90% of the time was spent there?

🚀 fanquake merged a pull request: "test: Use msg_generic in p2p_ping.py"
(https://github.com/bitcoin/bitcoin/pull/32834)

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-3019243840)
Would there be interest in adding a benchmark for the Trim() operation here, or rather in a follow-up?

💬 TheCharlatan commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2175160806)
More accurate error/standardness failure reporting can be done later through #29060.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3019305155)
> The `bench.run` call does not clear `coins`, so it's warm after the first iteration.

Actually the benchmark does populate the cache first through `SetupDummyInputs` (otherwise it couldn't work since it is using a dummy backend). But the main point really is that this micro benchmark is meaningless to demonstrate a slow down in transaction processing. `AreInputsStandard` should represent a fraction of a percent of the total work performed when processing a transaction. Unless we make it 10'0
...

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2175175517)
Apologies if that came across as patronizing, this was not my intention. That said, you should expect to receive pushback if you insistently derail a PR with irrelevant concerns.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2175176935)
I think the code is fine as-is, i don't plan on doing anything else here so will resolve this thread.