📝 fanquake opened a pull request: "sanitizer: symbolizer improvements"
(https://github.com/bitcoin/bitcoin/pull/28814)
Explicitly specifying a symbolizer path has fixed the two issues outlined in https://github.com/bitcoin/bitcoin/pull/28147#issuecomment-1789703076. It's not completely clear to my why this needs to be explicitly specified in some environments, and not in others, while at the same time `llvm-symbolizer` is already in PATH, however, this has fixed the 2 issues (Fedora & Tumbleweed). My guess is something to do with symlinking, and tooling expecting unversioned/exactly named binaries?

In future,
...

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1385015371)
I think before this was to fix a bug, but I fixed that bug in a different way. Regardless, doesn't it make more sense to run `transactionRemovedFromMempool` before syncing the transaction as confirmed?

💬 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)
+
...

💬 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).

🤔 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.

💬 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
...

💬 maflcko commented on issue "fuzz: Fix timeouts":
(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.

👍 maflcko approved a pull request: "sanitizer: symbolizer improvements"
(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?

💬 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
...

💬 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.

💬 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
```

👍 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.
...

💬 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`

💬 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.

💬 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.

💬 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.

💬 fanquake commented on issue "v26.0 Testing":
(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