Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 laanwj commented on pull request "randomenv: remove some `/proc/` accesses":
(https://github.com/bitcoin/bitcoin/pull/32450#issuecomment-2866302918)
> E.g. simplest implementation: pass a reference to a static flag to every AddFile( and set it to false after a failure. If it's false at start, don't try again.

Alternative using command pattern:
```diff
diff --git a/src/randomenv.cpp b/src/randomenv.cpp
index fbab23afe94315faa65d58d15ee0856875e1e92c..411985c662ea904eeaa983ed87e7597dd1485509 100644
--- a/src/randomenv.cpp
+++ b/src/randomenv.cpp
@@ -94,10 +94,29 @@ void AddSockaddr(CSHA512& hasher, const struct sockaddr *addr)
}
...
💬 hebasto commented on pull request "test: Suppress Windows abort message in CI":
(https://github.com/bitcoin/bitcoin/pull/32409#discussion_r2081584114)
This behaviour is MSVC-specific (I've added a note to the PR description). And I cannot reproduce it when running cross-compiled binaries.
💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2081584805)
It can be, and that was why the 1 second timeout check kept failing. I tried a few implementations of the test and it's just impossible to reliably start the test timer in sync with the HTTP server. So sometimes it starts late and you end up with `duration < expected`.

The point of the test is to ensure that the server disconnects idle clients, I think it does that even though the exact time is a little fudged.
💬 hebasto commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2866400462)
@laanwj

Are you going to upstream the second commit?
💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2866412296)
> Are you going to upstream the second commit?

Yes, but only after cpp-subprocess starts using c++17: https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844382194
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081597743)
Was done on purpose - to retire the type of `Obfuscation` gradually - but I don't mind doing it early either.
Changed in the commit messages as well.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081597874)
Named it as such to minimize diffs - but I also see the logic here - done
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081597972)
Done
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081598130)
Was done in the next commit, indeed - localized it, thanks for finding these rebase inconsistencies (likely happened when I changed the order of commits).
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081598285)
But I *am* Hungarian :p
In the tests I've been using `key_bytes`, applied it here as well.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081598512)
Valid critique - which is a gateway-suggestion forcing me to admit that we need static extend spans/arrays here :/
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081598646)
Hah, indeed, thanks!
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081598779)
Removed, thanks
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081599213)
Good idea, though it doesn't change the final commit
💬 laanwj commented on pull request "bench: replace benchmark block with more representative one (413567 → 784588)":
(https://github.com/bitcoin/bitcoin/pull/32457#issuecomment-2866425607)
Agree with the rationale of this PR, but having 1MB+ binary files in the repo is really meh.
📝 fanquake opened a pull request: "guix: move `*-check.py` scripts under contrib/guix/"
(https://github.com/bitcoin/bitcoin/pull/32458)
These scripts are not meant for general developer usage. They are for use on the release binaries, which have been compiled in an environment that makes various assumptions in regards to c library, compiler options, hardening options, patching etc.

Anyone is free to run these scripts against self-compiled binaries, but this isn't something we want to modifying/generalize the scripts to support.
💬 fanquake commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2866437307)
> But moving both to a guix-specific path would be fine with me.

See #32458.
💬 l0rinc commented on pull request "bench: replace benchmark block with more representative one (413567 → 784588)":
(https://github.com/bitcoin/bitcoin/pull/32457#issuecomment-2866447793)
Agree - do you have a better idea?
🤔 maflcko reviewed a pull request: "bench: replace benchmark block with more representative one (413567 → 784588)"
(https://github.com/bitcoin/bitcoin/pull/32457#pullrequestreview-2827710648)
Is there a benchmark that needs this? If yes, going for synthetic, but representative (and easily adjustable) data may be a better choice for that benchmark.
💬 maflcko commented on pull request "bench: replace benchmark block with more representative one (413567 → 784588)":
(https://github.com/bitcoin/bitcoin/pull/32457#discussion_r2081323234)
Instead of a block, this could just be random bytes from a fast random context?