✅ fanquake closed a pull request: "build: let CMake determine the year"
(https://github.com/bitcoin/bitcoin/pull/32445)
(https://github.com/bitcoin/bitcoin/pull/32445)
💬 laanwj commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2866165846)
> The symbol / security scripts are not for general developer usage, and there's no expectation that the scripts should pass, outside of a Guix build. We could move them out of contrib to make this more clear.
i think this makes sense, especially for the symbol check. The security check may have some value as a more general "binary security check". But moving both to a guix-specific path would be fine with me.
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2866165846)
> The symbol / security scripts are not for general developer usage, and there's no expectation that the scripts should pass, outside of a Guix build. We could move them out of contrib to make this more clear.
i think this makes sense, especially for the symbol check. The security check may have some value as a more general "binary security check". But moving both to a guix-specific path would be fine with me.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081476373)
I admit defeat, added:
```C++
/** Generate random bytes on the stack. */
template <BasicByte B = unsigned char, size_t N>
std::array<B, N> randbytes() noexcept
{
std::array<B, N> ret;
Impl().fillrand(MakeWritableByteSpan(ret));
return ret;
}
```
See: a66c1841bd5708eb446c805d6612e2448d6022f3
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081476373)
I admit defeat, added:
```C++
/** Generate random bytes on the stack. */
template <BasicByte B = unsigned char, size_t N>
std::array<B, N> randbytes() noexcept
{
std::array<B, N> ret;
Impl().fillrand(MakeWritableByteSpan(ret));
return ret;
}
```
See: a66c1841bd5708eb446c805d6612e2448d6022f3
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081476663)
Some compilers had problem with string `constexpr`, it's why I inlined it instead.
I don't mind reverting to `OBFUSCATION_KEY_KEY` (note `OBFUSCATE` -> `OBFUSCATION`), if you think it's better_better.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081476663)
Some compilers had problem with string `constexpr`, it's why I inlined it instead.
I don't mind reverting to `OBFUSCATION_KEY_KEY` (note `OBFUSCATE` -> `OBFUSCATION`), if you think it's better_better.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081477824)
Thanks
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081477824)
Thanks
💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2866204024)
There was a seemingly intermittent CI failure in wallet_basic.py on the commit merging this into master. Tracked as #32456. The following push to master succeeded.
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2866204024)
There was a seemingly intermittent CI failure in wallet_basic.py on the commit merging this into master. Tracked as #32456. The following push to master succeeded.
💬 ryanofsky commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2081485161)
Thanks for clarifying. I got the impression that you did think it was useful from "It may be a good idea to actually improve the check here, since it's relied on by `getblocktemplate` in proposal mode and by #31981" (https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2053597806). But I guess you were saying it *could* be useful if it were improved, not that it is useful currently.
Anyway I was mostly asking about this because I was confused. Assuming you are happy with PR in current fo
...
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2081485161)
Thanks for clarifying. I got the impression that you did think it was useful from "It may be a good idea to actually improve the check here, since it's relied on by `getblocktemplate` in proposal mode and by #31981" (https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2053597806). But I guess you were saying it *could* be useful if it were improved, not that it is useful currently.
Anyway I was mostly asking about this because I was confused. Assuming you are happy with PR in current fo
...
💬 carnhofdaki commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2866215127)
Concept ACK, the patches look fine to me. Compiling 92b05c17b1 right now. Thanks for doing it!
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2866215127)
Concept ACK, the patches look fine to me. Compiling 92b05c17b1 right now. Thanks for doing it!
💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2081493334)
Good catch! I do think daemon could be ok but I agree node is better. I will update this (and I also want to look into @theStack's suggestion to use subprocess.h, which I was unaware of and could simplify the PR)
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2081493334)
Good catch! I do think daemon could be ok but I agree node is better. I will update this (and I also want to look into @theStack's suggestion to use subprocess.h, which I was unaware of and could simplify the PR)
💬 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)
}
...
(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.
(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.
(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?
(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
(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.
(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
(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
(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).
(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.
(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 :/
(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 :/