✅ MarcoFalke closed an issue: "test: `interface_bitcoin_cli.py --descriptors` failure under `--valgrind`"
(https://github.com/bitcoin/bitcoin/issues/27281)
(https://github.com/bitcoin/bitcoin/issues/27281)
💬 MarcoFalke commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#issuecomment-1479241540)
re-ACK 95ad70ab652ddde7de65f633c36c1378b26a313a 🚥
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 95ad70ab652ddde7de65
...
(https://github.com/bitcoin/bitcoin/pull/26749#issuecomment-1479241540)
re-ACK 95ad70ab652ddde7de65f633c36c1378b26a313a 🚥
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 95ad70ab652ddde7de65
...
🚀 fanquake merged a pull request: "test: Fix TypeError (expected str instance, bytes found) in wait_for_debug_log"
(https://github.com/bitcoin/bitcoin/pull/27280)
(https://github.com/bitcoin/bitcoin/pull/27280)
📝 fanquake opened a pull request: "guix: import/sync python-lief (0.12.3) package definition from upstream"
(https://github.com/bitcoin/bitcoin/pull/27296)
Update to version 0.12.3.
Retain our PPC64 patch.
Mention when we can drop our local definition.
Also switch to using cmake-minimal (see #27172), which fixes atleast one build failure I've seen on aarch64, where cmake dependencies fail to build. Fix that by using the cmake without all the dependencies we don't actually need:
```bash
The following derivations will be built:
/gnu/store/7qqvqq2g7l5ylrjv0gn6zha565a12kar-python-lief-0.12.1.drv
/gnu/store/f9zwh1ldy63ga0i5w6cbbqlj6sfq226j-
...
(https://github.com/bitcoin/bitcoin/pull/27296)
Update to version 0.12.3.
Retain our PPC64 patch.
Mention when we can drop our local definition.
Also switch to using cmake-minimal (see #27172), which fixes atleast one build failure I've seen on aarch64, where cmake dependencies fail to build. Fix that by using the cmake without all the dependencies we don't actually need:
```bash
The following derivations will be built:
/gnu/store/7qqvqq2g7l5ylrjv0gn6zha565a12kar-python-lief-0.12.1.drv
/gnu/store/f9zwh1ldy63ga0i5w6cbbqlj6sfq226j-
...
💬 MarcoFalke commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1144532693)
Same commit: Can remove the now unused constructors for all other structs as well? See diff, plus unrelated cleanup: (Maybe for a follow-up to avoid another round of re-review?)
```diff
diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp
index 012b0eb321..975dace633 100644
--- a/src/test/checkqueue_tests.cpp
+++ b/src/test/checkqueue_tests.cpp
@@ -57,7 +57,6 @@ struct FakeCheckCheckCompletion {
struct FailingCheck {
bool fails;
FailingCheck(bool _fa
...
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1144532693)
Same commit: Can remove the now unused constructors for all other structs as well? See diff, plus unrelated cleanup: (Maybe for a follow-up to avoid another round of re-review?)
```diff
diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp
index 012b0eb321..975dace633 100644
--- a/src/test/checkqueue_tests.cpp
+++ b/src/test/checkqueue_tests.cpp
@@ -57,7 +57,6 @@ struct FakeCheckCheckCompletion {
struct FailingCheck {
bool fails;
FailingCheck(bool _fa
...
💬 hebasto commented on pull request "guix: import/sync python-lief (0.12.3) package definition from upstream":
(https://github.com/bitcoin/bitcoin/pull/27296#issuecomment-1479266165)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/27296#issuecomment-1479266165)
Concept ACK.
💬 hebasto commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1144539820)
> Maybe for a follow-up to avoid another round of re-review?
Yes, please :)
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1144539820)
> Maybe for a follow-up to avoid another round of re-review?
Yes, please :)
💬 darosior commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1479296136)
Concept ACK. Have you looked into a fuzz target for this parser?
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1479296136)
Concept ACK. Have you looked into a fuzz target for this parser?
💬 fanquake commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144580869)
In 7a9d2fb8cf11eef8b6bd87ad1066df374bf3b620: Make these Doxygen comments?
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144580869)
In 7a9d2fb8cf11eef8b6bd87ad1066df374bf3b620: Make these Doxygen comments?
💬 fanquake commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144582632)
In 7a9d2fb8cf11eef8b6bd87ad1066df374bf3b620: These could be
```suggestion
constexpr static size_t RFC8439_KEYLEN{32};
```
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144582632)
In 7a9d2fb8cf11eef8b6bd87ad1066df374bf3b620: These could be
```suggestion
constexpr static size_t RFC8439_KEYLEN{32};
```
💬 fanquake commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144584874)
In 7a9d2fb8cf11eef8b6bd87ad1066df374bf3b620: nit use c++ headers, i.e `<cassert>`
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144584874)
In 7a9d2fb8cf11eef8b6bd87ad1066df374bf3b620: nit use c++ headers, i.e `<cassert>`
💬 fanquake commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144588967)
Please don't add `for xyz` comments. They are unmaintainable, and if someone wants to know why an include is used, they can look at IWYU etc. Note that you could also add newly-added files to `ci/test/06-script_b.sh` to have the includes checked.
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144588967)
Please don't add `for xyz` comments. They are unmaintainable, and if someone wants to know why an include is used, they can look at IWYU etc. Note that you could also add newly-added files to `ci/test/06-script_b.sh` to have the includes checked.
💬 fanquake commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144594272)
```suggestion
std::memcpy(header_and_contents.data(), &flags, BIP324_HEADER_LEN);
```
Same for memset etc.
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144594272)
```suggestion
std::memcpy(header_and_contents.data(), &flags, BIP324_HEADER_LEN);
```
Same for memset etc.
💬 fanquake commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144580119)
In 7a9d2fb8cf11eef8b6bd87ad1066df374bf3b620:
It is fairly awkward to introduce a second copy of `timingsafe_bcmp` (one already in `chacha_poly_aead`), with a different name, and broken #ifdef logic, just to then rename it when you delete the original `timingsafe_bcmp` in a later commit. Although I guess re-using/extracting `timingsafe_bcmp` early from the to-be-deleted file is also a bit awkward.
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144580119)
In 7a9d2fb8cf11eef8b6bd87ad1066df374bf3b620:
It is fairly awkward to introduce a second copy of `timingsafe_bcmp` (one already in `chacha_poly_aead`), with a different name, and broken #ifdef logic, just to then rename it when you delete the original `timingsafe_bcmp` in a later commit. Although I guess re-using/extracting `timingsafe_bcmp` early from the to-be-deleted file is also a bit awkward.
💬 fanquake commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144586412)
nit: additional newline
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1144586412)
nit: additional newline
💬 fanquake commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1144620929)
Ok. We'll push this to a followup.
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1144620929)
Ok. We'll push this to a followup.
💬 darosior commented on issue "Allow for rescan using block filters with pruned node":
(https://github.com/bitcoin/bitcoin/issues/21267#issuecomment-1479349534)
@furszy are you still working on this?
(https://github.com/bitcoin/bitcoin/issues/21267#issuecomment-1479349534)
@furszy are you still working on this?
💬 darosior commented on issue "Manual-pruning cursor rewind":
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1479351248)
Slightly related: https://github.com/bitcoin/bitcoin/issues/21267.
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1479351248)
Slightly related: https://github.com/bitcoin/bitcoin/issues/21267.
🚀 fanquake merged a pull request: "refactor: Use move semantics instead of custom swap functions"
(https://github.com/bitcoin/bitcoin/pull/26749)
(https://github.com/bitcoin/bitcoin/pull/26749)
💬 fanquake commented on pull request "test: add support for all networks in `deserialize_v2`":
(https://github.com/bitcoin/bitcoin/pull/27295#issuecomment-1479382280)
https://cirrus-ci.com/task/6600092300345344
```bash
test 2023-03-21T23:55:59.273000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\test_framework.py", line 132, in main
self.run_test()
...
(https://github.com/bitcoin/bitcoin/pull/27295#issuecomment-1479382280)
https://cirrus-ci.com/task/6600092300345344
```bash
test 2023-03-21T23:55:59.273000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\test_framework.py", line 132, in main
self.run_test()
...