Bitcoin Core Github
44 subscribers
119K links
Download Telegram
πŸ’¬ Khanwafa232426 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579531)
TTBp1cVNxDHUxetKwXof2cLfeRFWMqejhV
πŸ’¬ Khanwafa232426 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/6e21dedbf2b3029c729108f225469b321a1b3d39#r148579539)
TTBp1cVNxDHUxetKwXof2cLfeRFWMqejhVTTBp1cVNxDHUxetKwXof2cLfeRFWMqejhVhttps://www.binance.com/fan-token/BWS/FCShakhtarFanverse?ref=CG5V70EE&registerChannel=fcshakhtar&utm_source=fan-token&utm_campaign=fcshakhtar-fanverse-invite
πŸ’¬ laanwj commented on issue "Gracefully handle dropped UPnP support ":
(https://github.com/bitcoin-core/gui/issues/843#issuecomment-2449234882)
Even ignoring the spurious setting would be better in the GUI case. No need to rub this in the users face. i didn't expect this consequence when i suggested adding a message πŸ˜…

Though it's still better than creashing with generic "-upnp: Unknown setting". i guess...

But yes this needs to be informational level at most, it should just continue.

Even worse: i guess it even happens if the settings json specifies upnp=0 to explicitly disable it?
πŸ“ maflcko converted_to_draft a pull request: "ci: Place datadirs for tests under tmpfs "
(https://github.com/bitcoin/bitcoin/pull/31182)
This should speed up the tests a bit (see comment below for results).
πŸ’¬ maflcko commented on pull request "ci: Place datadirs for tests under tmpfs ":
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2449255282)
> > (see comment below for results).
>
> What is this referring to?

I was still running the commit (and master) twice each for all CI tasks, but I couldn't find any difference at all.

I checked that the folder is correctly picked up and used by the tests, so I guess modern storage is fast enough to not make a difference? Alternatively, there is an issue I was missing.

It would be good if someone else double checked the code and whether there is a performance difference at all.

For
...
πŸ’¬ maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2449259598)
> > ramdisk
>
> Benchmarking this is a bit more involved and I think requires changes to the CI scripts, so that the ramdisk is (1) mounted into the CI container and (2) picked up as the temp dir. I can take a look in the future, unless someone beats me to it.

Done in https://github.com/bitcoin/bitcoin/pull/31182, but I couldn't find any difference at all so far, which is a bit surprising.
πŸ’¬ l0rinc commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2449260674)
Could we obviate in the [DrahtBot's response](https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2413637472) that the link doesn't just provide a `test coverage report` - since the regression was already visible there: https://corecheck.dev/bitcoin/bitcoin/pulls/31093
πŸ’¬ naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1824060041)
I was looking for something like "As a general rule, we keep the transaction that appeared first".

Currently, the `AddToSet` documentation seems outdatedβ€”it basically claims that the only way to fail is to reach a set limit.

How about the following replacement:

```
/**
* Step 1. Add a to-be-announced transaction to the local reconciliation set of the target peer.
* Returns false if the set is at capacity, or if the set contains a colliding transaction.
* Returns t
...
πŸ’¬ willcl-ark commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2449385872)
> > In my main machine, is ryzen with 32gb ddr4 with 2 sticks of memory. The other machine is Xeon, with 32gb memory, in quad channel ddr3. So i dont think its memory, because the 2 computers are stable. I can run prime95 in both for 2 hours with no problem.
>
> it's true, I just ran disk diagnostics, no issues. this is a new 4tb ssd. going to run a memory diagnostic... but my system has been stable so I would be very surprised if it was that. I'm also running on ZFS. I keep hitting this, eve
...
πŸ’¬ maflcko commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2449435667)
Sure, it is open source and pull requests for re-wording are welcome: https://github.com/maflcko/DrahtBot/blob/main/webhook_features/src/features/summary_comment.rs#L194
πŸ’¬ hodlinator commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1824164226)
@dergoegge any thoughts on my suggestion of altering `FuzzedDataProvider` to trigger asserts when calls like `ConsumeIntegralInRange<int>(0, decoded.size() - 1)` are made after calls to `ConsumeRemainingBytes*()`? Maybe such an improvement could be upstreamed even (possibly with a flag to turn it off for sloppy tests).
πŸ’¬ maflcko commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2449446364)
I wonder if an `[[unlikely]]` annotation can take care of this, so that the compiler can treat the Assume as an (almost) no-op in the happy path. Before and after the `g_fuzzing` change, `val` would have to always be evaluated. However, it seems the `!val` evaluation is the overhead in this case?
πŸ’¬ maflcko commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1824176187)
I'd say it would be easier to sell to upstream if it was checked at compile-time at no runtime cost/overhead. Is there a benefit of doing it at runtime?
πŸ’¬ hodlinator commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1824185306)
It might add extra friction to agree on preprocessor logic to make the code still compile on other compilers. Also, doing it at compile-time means it's an all/nothing thing that can't be turned off for individual sloppy tests. But I agree it would be great to see if upstream would take a compile-time version first.
πŸ’¬ maflcko commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1824197377)
> agree on preprocessor logic

Maybe I am missing something, but it should be pretty standard, similar to

https://github.com/bitcoin/bitcoin/blob/f07a533dfcb172321972e5afb3b38a4bd24edb87/src/span.h#L22-L30

?
πŸ’¬ hodlinator commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1824202809)
Yeah, hopefully shouldn't be much of an obstacle, maybe I'm too pessimistic about that aspect.