🚀 fanquake merged a pull request: "contrib: drop GCC MAX_VERSION to 4.3.0 in symbol-check"
(https://github.com/bitcoin/bitcoin/pull/28844)
(https://github.com/bitcoin/bitcoin/pull/28844)
🚀 fanquake merged a pull request: "fuzz: Limit p2p fuzz targets to MAX_PROTOCOL_MESSAGE_LENGTH"
(https://github.com/bitcoin/bitcoin/pull/29079)
(https://github.com/bitcoin/bitcoin/pull/29079)
🚀 fanquake merged a pull request: "build: Bump guix time-machine to unlock riscv64 metal"
(https://github.com/bitcoin/bitcoin/pull/29078)
(https://github.com/bitcoin/bitcoin/pull/29078)
✅ fanquake closed an issue: "./contrib/guix/guix-build does not work on riscv64"
(https://github.com/bitcoin/bitcoin/issues/29020)
(https://github.com/bitcoin/bitcoin/issues/29020)
📝 SatoshiNT0 opened a pull request: "bitcoin"
(https://github.com/bitcoin/bitcoin/pull/29106)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/29106)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ fanquake closed a pull request: "bitcoin"
(https://github.com/bitcoin/bitcoin/pull/29106)
(https://github.com/bitcoin/bitcoin/pull/29106)
📝 fanquake locked a pull request: "bitcoin"
(https://github.com/bitcoin/bitcoin/pull/29106)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/29106)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 fanquake commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1860456382)
If we are going to change it here, should also switch in `hash.cpp`:
https://github.com/bitcoin/bitcoin/blob/c840dea27edfcfc67beb756ca6eda27b319f93d5/src/hash.cpp#L12-L15
Also cc @theuni.
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1860456382)
If we are going to change it here, should also switch in `hash.cpp`:
https://github.com/bitcoin/bitcoin/blob/c840dea27edfcfc67beb756ca6eda27b319f93d5/src/hash.cpp#L12-L15
Also cc @theuni.
📝 kristapsk opened a pull request: "Fix spelling errors"
(https://github.com/bitcoin/bitcoin/pull/29107)
Found these when running lint tests locally.
```
src/rpc/util.h:405: falsy ==> falsely, false
src/rpc/util.h:408: falsy ==> falsely, false
src/test/fuzz/package_eval.cpp:214: non-existant ==> non-existent
src/test/span_tests.cpp:56: memeber ==> member
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
Guess it's because I'm having different version of codespell?
In any case,
...
(https://github.com/bitcoin/bitcoin/pull/29107)
Found these when running lint tests locally.
```
src/rpc/util.h:405: falsy ==> falsely, false
src/rpc/util.h:408: falsy ==> falsely, false
src/test/fuzz/package_eval.cpp:214: non-existant ==> non-existent
src/test/span_tests.cpp:56: memeber ==> member
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
Guess it's because I'm having different version of codespell?
In any case,
...
💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430148600)
> No, `m_offsets` is not uninitialized
The array is initialized, but the items are not, and afaik `int64_t` doesn't have a default constructor?
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430148600)
> No, `m_offsets` is not uninitialized
The array is initialized, but the items are not, and afaik `int64_t` doesn't have a default constructor?
💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430149707)
Care to elaborate why? Since this is newly introduced code, seems sensible to do it right away?
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430149707)
Care to elaborate why? Since this is newly introduced code, seems sensible to do it right away?
🤔 ismaelsadeeq reviewed a pull request: "fuzz: make FuzzedDataProvider usage deterministic"
(https://github.com/bitcoin/bitcoin/pull/29043#pullrequestreview-1786902962)
>Good point, I guess since the spec is unspecified they have the right to change it but I would still be surprised if they do. I'm not sure if there is a trivial way to check if they ever have.
The order of evaluation issue is a characteristic of the C++ language standard. While the LLVM compiler might have a consistent order of evaluation, but generally compiler behavior could change, maybe we should not use the current LLVM behavior as validation for this but rather refer to the overall lan
...
(https://github.com/bitcoin/bitcoin/pull/29043#pullrequestreview-1786902962)
>Good point, I guess since the spec is unspecified they have the right to change it but I would still be surprised if they do. I'm not sure if there is a trivial way to check if they ever have.
The order of evaluation issue is a characteristic of the C++ language standard. While the LLVM compiler might have a consistent order of evaluation, but generally compiler behavior could change, maybe we should not use the current LLVM behavior as validation for this but rather refer to the overall lan
...
💬 ismaelsadeeq commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1430148489)
Nit: use descriptive variable names for more clarity instead of single letters
<details>
<summary>see diff</summary>
```diff
diff --git a/src/test/fuzz/crypto.cpp b/src/test/fuzz/crypto.cpp
index aa478277e35..2fa2b109e55 100644
--- a/src/test/fuzz/crypto.cpp
+++ b/src/test/fuzz/crypto.cpp
@@ -23,8 +23,8 @@ FUZZ_TARGET(crypto)
std::vector<uint8_t> data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
if (data.empty()) {
auto new_size = fuzzed_data_prov
...
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1430148489)
Nit: use descriptive variable names for more clarity instead of single letters
<details>
<summary>see diff</summary>
```diff
diff --git a/src/test/fuzz/crypto.cpp b/src/test/fuzz/crypto.cpp
index aa478277e35..2fa2b109e55 100644
--- a/src/test/fuzz/crypto.cpp
+++ b/src/test/fuzz/crypto.cpp
@@ -23,8 +23,8 @@ FUZZ_TARGET(crypto)
std::vector<uint8_t> data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
if (data.empty()) {
auto new_size = fuzzed_data_prov
...
💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430151965)
https://godbolt.org/z/5PKET7E39
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430151965)
https://godbolt.org/z/5PKET7E39
💬 ismaelsadeeq commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1430154424)
This is okay since the standard is left-to-right evaluation order for initializer-clauses within an initializer-list?
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1430154424)
This is okay since the standard is left-to-right evaluation order for initializer-clauses within an initializer-list?
💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430166505)
Ah cool, didn't know, thanks. Still, having default (0-)values in sorted_copy seems like it would lead to an incorrect median calculation?
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430166505)
Ah cool, didn't know, thanks. Still, having default (0-)values in sorted_copy seems like it would lead to an incorrect median calculation?
🚀 fanquake merged a pull request: "fuzz: Improve fuzzing stability for minisketch harness"
(https://github.com/bitcoin/bitcoin/pull/29064)
(https://github.com/bitcoin/bitcoin/pull/29064)
💬 sipa commented on issue "Performance decrease after tapscript miniscript":
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-1860568169)
That sounds very plausible, @darosior.
If so, I think there is an approach that doesn't lose optimality: keep track of the optimal satisfaction size, and once reached, don't bother evaluation further keys?
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-1860568169)
That sounds very plausible, @darosior.
If so, I think there is an approach that doesn't lose optimality: keep track of the optimal satisfaction size, and once reached, don't bother evaluation further keys?
📝 maflcko opened a pull request: "refactor: Replace ALWAYS_FALSE with false"
(https://github.com/bitcoin/bitcoin/pull/29108)
Now that `static_assert(false)` when evaluated in a non-instantiated template is treated as a no-op, use it over the workaround, and remove the workaround.
(https://github.com/bitcoin/bitcoin/pull/29108)
Now that `static_assert(false)` when evaluated in a non-instantiated template is treated as a no-op, use it over the workaround, and remove the workaround.
💬 darosior commented on issue "Performance decrease after tapscript miniscript":
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-1860578924)
Neat. This would give us the same performance in the large majority of cases while still being optimal in the rare non-SIGHASH_DEFAULT cases. In any case this would fix the slowdown you noticed @eriknylund.
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-1860578924)
Neat. This would give us the same performance in the large majority of cases while still being optimal in the rare non-SIGHASH_DEFAULT cases. In any case this would fix the slowdown you noticed @eriknylund.