Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 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.
💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2175129044)
nit:

log strings currently look like:

> Restarting logging from ../../src/rpc/request.cpp:140 (GenerateAuthCookieResult GenerateAuthCookie(const std::optional<fs::perms> &, std::string &, std::string &)): **(0 MiB)** were dropped during the last hour.

Would suggest printing this as bytes and removing the parentheses.

```suggestion
"%d bytes were dropped during the last hour.\n",
source_loc.file_name(), source_l
...
👍 stickies-v approved a pull request: "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError"
(https://github.com/bitcoin/bitcoin/pull/32604#pullrequestreview-2971317445)
ACK 2ac8696b53e455dd27c8341828404a23b5cb68a9

I still would prefer the approach I suggested [here](https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2172385738) (and am happy to re-review), but I think the PR is fine as is now too.
💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2175159834)
nit
```suggestion
std::string str threadname;
```
💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2175213743)
nit: the `-logsourcelocation` behaviour change is unrelated to the rate limiting, so I think this phrasing is a bit confusing. Would put it in a separate paragraph:

```
Unconditional logging to disk via LogPrintf, LogInfo, LogWarning, LogError, and
LogPrintLevel is now rate limited by giving each source location a logging quota of
1MiB per hour. (#32604)

When `-logsourcelocation` is enabled, the log output now contains the entire function signature
instead of just the function name. (#
...