💬 glozow commented on pull request "refactor: Miniminer package linearization followups":
(https://github.com/bitcoin/bitcoin/pull/28808#issuecomment-1798706638)
This diff appears to have added new lines with trailing whitespace.
The following changes were suspected:
diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp
@@ -668,7 +667,7 @@ BOOST_FIXTURE_TEST_CASE(manual_ctor, TestChain100Setup)
+ miniminer_info.emplace_back(grandparent_zero_fee, /*vsize_self=*/tx_vsize,/*vsize_ancestor=*/tx_vsize, /*fee_self=*/0,/*fee_ancestor=*/0);
@@ -696 +695 @@ BOOST_FIXTURE_TEST_CASE(manual_ctor, TestChain100Setup)
+
...
(https://github.com/bitcoin/bitcoin/pull/28808#issuecomment-1798706638)
This diff appears to have added new lines with trailing whitespace.
The following changes were suspected:
diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp
@@ -668,7 +667,7 @@ BOOST_FIXTURE_TEST_CASE(manual_ctor, TestChain100Setup)
+ miniminer_info.emplace_back(grandparent_zero_fee, /*vsize_self=*/tx_vsize,/*vsize_ancestor=*/tx_vsize, /*fee_self=*/0,/*fee_ancestor=*/0);
@@ -696 +695 @@ BOOST_FIXTURE_TEST_CASE(manual_ctor, TestChain100Setup)
+
...
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1798760647)
Updated per @andrewtoth feedback.
Improved a test comment for better clarity. Tiny [diff](https://github.com/bitcoin/bitcoin/compare/5628ac55be42c5450c6817ba8dcfe463ceda32a9..60a487dd2430145115fcd0215472a5a2ef9bb090).
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1798760647)
Updated per @andrewtoth feedback.
Improved a test comment for better clarity. Tiny [diff](https://github.com/bitcoin/bitcoin/compare/5628ac55be42c5450c6817ba8dcfe463ceda32a9..60a487dd2430145115fcd0215472a5a2ef9bb090).
🤔 furszy reviewed a pull request: "wallet: prevent bugs from invalid transaction heights with asserts, comments, and refactoring"
(https://github.com/bitcoin/bitcoin/pull/28546#pullrequestreview-1717909906)
Code review ACK f06016d
Have to admit that I'm still not sure about the first commit, but it isn't blocking if others are happy with it.
(https://github.com/bitcoin/bitcoin/pull/28546#pullrequestreview-1717909906)
Code review ACK f06016d
Have to admit that I'm still not sure about the first commit, but it isn't blocking if others are happy with it.
💬 kevkevinpal commented on pull request "refactor: Miniminer package linearization followups":
(https://github.com/bitcoin/bitcoin/pull/28808#issuecomment-1798778274)
> This diff appears to have added new lines with trailing whitespace. The following changes were suspected:
>
> diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp @@ -668,7 +667,7 @@ BOOST_FIXTURE_TEST_CASE(manual_ctor, TestChain100Setup)
>
> * ```
> miniminer_info.emplace_back(grandparent_zero_fee, /*vsize_self=*/tx_vsize,/*vsize_ancestor=*/tx_vsize, /*fee_self=*/0,/*fee_ancestor=*/0);
> ```
>
>
> @@ -696 +695 @@ BOOST_FIXTURE
...
(https://github.com/bitcoin/bitcoin/pull/28808#issuecomment-1798778274)
> This diff appears to have added new lines with trailing whitespace. The following changes were suspected:
>
> diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp @@ -668,7 +667,7 @@ BOOST_FIXTURE_TEST_CASE(manual_ctor, TestChain100Setup)
>
> * ```
> miniminer_info.emplace_back(grandparent_zero_fee, /*vsize_self=*/tx_vsize,/*vsize_ancestor=*/tx_vsize, /*fee_self=*/0,/*fee_ancestor=*/0);
> ```
>
>
> @@ -696 +695 @@ BOOST_FIXTURE
...
💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1798781365)
Bloom filter flame graph:

(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1798781365)
Bloom filter flame graph:

💬 mzumsande commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1798836337)
> I presume this will fix itself, once and if v2 is enabled by default?
After enabling v2 by default for the functional tests, we'd probably still want to run some or all of the tests with `-v2transport=0` to avoid regressions - so I don't think that would change much.
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1798836337)
> I presume this will fix itself, once and if v2 is enabled by default?
After enabling v2 by default for the functional tests, we'd probably still want to run some or all of the tests with `-v2transport=0` to avoid regressions - so I don't think that would change much.
👍 maflcko approved a pull request: "sanitizer: symbolizer improvements"
(https://github.com/bitcoin/bitcoin/pull/28814#pullrequestreview-1717966778)
nice, lgtm
(https://github.com/bitcoin/bitcoin/pull/28814#pullrequestreview-1717966778)
nice, lgtm
💬 maflcko commented on pull request "sanitizer: symbolizer improvements":
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385087260)
Is this true on all platforms? If not, what would happen?
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385087260)
Is this true on all platforms? If not, what would happen?
💬 maflcko commented on pull request "sanitizer: symbolizer improvements":
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385090297)
Looks like the CI provides the answer already:
```
==18180==WARNING: invalid path to external symbolizer!
==18180==WARNING: Failed to use and restart external symbolizer!
test/fuzz/FuzzedDataProvider.h:212:47: runtime error: unsigned integer overflow: 9223372036854775807 - 9223372036854775808 cannot be represented in type 'uint64_t' (aka 'unsigned long')
#0 0x556fc34a1b1b (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1a12b1b) (BuildId: 3ec865ec
...
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385090297)
Looks like the CI provides the answer already:
```
==18180==WARNING: invalid path to external symbolizer!
==18180==WARNING: Failed to use and restart external symbolizer!
test/fuzz/FuzzedDataProvider.h:212:47: runtime error: unsigned integer overflow: 9223372036854775807 - 9223372036854775808 cannot be represented in type 'uint64_t' (aka 'unsigned long')
#0 0x556fc34a1b1b (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1a12b1b) (BuildId: 3ec865ec
...
💬 fanquake commented on pull request "sanitizer: symbolizer improvements":
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385093144)
Yea, the worst that can happen is the problem we already have. Let's see if we can make this more generic.
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385093144)
Yea, the worst that can happen is the problem we already have. Let's see if we can make this more generic.
💬 fanquake commented on pull request "sanitizer: symbolizer improvements":
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385106168)
I guess there are also other issues here, as the ASAN job does find the symbolizer, but not at that path:
```bash
AddressSanitizer: reading suppressions file at /ci_container_base/test/sanitizer_suppressions/ubsan
==28779==Using llvm-symbolizer found at: /usr/bin/llvm-symbolizer-17
==28779==AddressSanitizer Init done
Error parsing command line arguments: ==28779==__tls_get_addr: DTLS_Find 0x7f7962720770 2
```
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385106168)
I guess there are also other issues here, as the ASAN job does find the symbolizer, but not at that path:
```bash
AddressSanitizer: reading suppressions file at /ci_container_base/test/sanitizer_suppressions/ubsan
==28779==Using llvm-symbolizer found at: /usr/bin/llvm-symbolizer-17
==28779==AddressSanitizer Init done
Error parsing command line arguments: ==28779==__tls_get_addr: DTLS_Find 0x7f7962720770 2
```
👍 laanwj approved a pull request: "Add support for RNDR/RNDRRS for AArch64 on Linux"
(https://github.com/bitcoin/bitcoin/pull/26839#pullrequestreview-1718023531)
Tested ACK aee5404e02e203a256c1a97b629b9b107cc8bb07
I have verified this PR on the only available hardware (Amazon Graviton 3, m7g.medium instance). I was hoping for real hardware to become available but it looks like the recent crop of SoCs (such as RPI5) still doesn't have support for this extension.
In any case I've checked that the capability is detected correctly, and that RNDRRS is called four times at startup and RNDR frequently during runtime, and that the returned values look random.
...
(https://github.com/bitcoin/bitcoin/pull/26839#pullrequestreview-1718023531)
Tested ACK aee5404e02e203a256c1a97b629b9b107cc8bb07
I have verified this PR on the only available hardware (Amazon Graviton 3, m7g.medium instance). I was hoping for real hardware to become available but it looks like the recent crop of SoCs (such as RPI5) still doesn't have support for this extension.
In any case I've checked that the capability is detected correctly, and that RNDRRS is called four times at startup and RNDR frequently during runtime, and that the returned values look random.
...
💬 maflcko commented on pull request "sanitizer: symbolizer improvements":
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385139952)
Yes, the task does not have llvm installed, but only `llvm-17`
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385139952)
Yes, the task does not have llvm installed, but only `llvm-17`
💬 fanquake commented on pull request "sanitizer: symbolizer improvements":
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385146001)
Yea, my question is why is it now failing, if it's successfully finding a symbolizer.
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385146001)
Yea, my question is why is it now failing, if it's successfully finding a symbolizer.
💬 fanquake commented on pull request "sanitizer: symbolizer improvements":
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385151899)
Ah, it looks like the `verbosity=2` output is breaking things that are parsing output:
```bash
/usr/bin/python3.11 ../test/util/test_runner.py
2023-11-07 15:13:31,154 - ERROR - Error mismatch:
Expected: Error parsing command line arguments: Invalid command 'foo'
```
and the ASAN job here would otherwise be working as expected.
(https://github.com/bitcoin/bitcoin/pull/28814#discussion_r1385151899)
Ah, it looks like the `verbosity=2` output is breaking things that are parsing output:
```bash
/usr/bin/python3.11 ../test/util/test_runner.py
2023-11-07 15:13:31,154 - ERROR - Error mismatch:
Expected: Error parsing command line arguments: Invalid command 'foo'
```
and the ASAN job here would otherwise be working as expected.
💬 mzumsande commented on issue "v26.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1799048645)
The link to the release notes draft doesn't work for me.
(https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1799048645)
The link to the release notes draft doesn't work for me.
💬 fanquake commented on issue "v26.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1799053374)
Should be fixed now.
(https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1799053374)
Should be fixed now.
💬 achow101 commented on pull request "wallet: prevent bugs from invalid transaction heights with asserts, comments, and refactoring":
(https://github.com/bitcoin/bitcoin/pull/28546#issuecomment-1799093337)
ACK f06016d77d848133fc6568f287bb86b644c9fa69
(https://github.com/bitcoin/bitcoin/pull/28546#issuecomment-1799093337)
ACK f06016d77d848133fc6568f287bb86b644c9fa69
🚀 achow101 merged a pull request: "wallet: prevent bugs from invalid transaction heights with asserts, comments, and refactoring"
(https://github.com/bitcoin/bitcoin/pull/28546)
(https://github.com/bitcoin/bitcoin/pull/28546)
💬 TheCharlatan commented on pull request "guix: switch to 6.1 kernel headers over 5.15":
(https://github.com/bitcoin/bitcoin/pull/28786#issuecomment-1799123554)
Guix builds (aarch64)
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
9aba5e0ea27cbf609baac52e65f0eb63bc42aa7e1865108422d30b0bad01a7da guix-build-380e3655631b/output/aarch64-linux-gnu/SHA256SUMS.part
67644dd783c781ba6ae6bffa91204d3add423fe856b3aac09cb18c5f6db18599 guix-build-380e3655631b/output/aarch64-linux-gnu/bitcoin-380e3655631b-aarch64-linux-gnu-debug.tar.gz
0130236c032fbdcb5b791510d2ecf61239b434a1996b4b35ad7e7
...
(https://github.com/bitcoin/bitcoin/pull/28786#issuecomment-1799123554)
Guix builds (aarch64)
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
9aba5e0ea27cbf609baac52e65f0eb63bc42aa7e1865108422d30b0bad01a7da guix-build-380e3655631b/output/aarch64-linux-gnu/SHA256SUMS.part
67644dd783c781ba6ae6bffa91204d3add423fe856b3aac09cb18c5f6db18599 guix-build-380e3655631b/output/aarch64-linux-gnu/bitcoin-380e3655631b-aarch64-linux-gnu-debug.tar.gz
0130236c032fbdcb5b791510d2ecf61239b434a1996b4b35ad7e7
...