✅ MarcoFalke closed a pull request: "test: blockstore test with chattr instead of chmod"
(https://github.com/bitcoin/bitcoin/pull/28171)

💬 MarcoFalke commented on pull request "test: blockstore test with chattr instead of chmod":
(https://github.com/bitcoin/bitcoin/pull/28171#discussion_r1277145643)
This has nothing to do with asan, you will have to add it to all created containers.

💬 MarcoFalke commented on pull request "rpc, util: deduplicate AmountFromValue() using util::Result":
(https://github.com/bitcoin/bitcoin/pull/28134#discussion_r1277155809)
Not sure. There were previously two functions and there still are two functions now, with more code at the call sites. So I am not sure if this is an improvement. The only difference seems to be in the throw function, so in theory you could pass in a lambda that takes care of the throw, but I am not sure if this is worth it.

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1277157308)
> Pushed a commit here which tests this: [theuni/bitcoin-tidy-plugin@cdc0070](https://github.com/theuni/bitcoin-tidy-plugin/commit/cdc007022b6cd40320b7e74ab4cbb8e67a6d6e17)
>
> It works as expected.

Nice. Maybe include this here?

💬 russeree commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1277194218)
I have changed this to read ```Invalid or unsupported prefix```... testing my own knowledge; The reason you can have an unsupported but still technically valid address would be in the event of a softfork to a new SegWit version? e.x. _a segwit client interpreting a taproot address would say "invalid"_ but that is not completely true because the address is valid. Just not supported by the client?

This has been resolved in b66e974ac8f21971602051c08df830d14cf3df7e

💬 MarcoFalke commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#discussion_r1277247979)
Just to leave my thought (which I withheld prior to merge): Could just use the raw (single) byte here, but that would interfere with libFuzzer `-only_ascii=1`, which makes me wonder if this is the first ascii fuzz target and whether we should set the option somewhere somehow during input generation?

💬 ismaelsadeeq commented on pull request "test doc: tests `acceptstalefeeestimates` option is only supported on regtest chain":
(https://github.com/bitcoin/bitcoin/pull/28157#discussion_r1277267221)
Thank you, I updated with your suggestions

💬 ismaelsadeeq commented on pull request "test doc: tests `acceptstalefeeestimates` option is only supported on regtest chain":
(https://github.com/bitcoin/bitcoin/pull/28157#discussion_r1277267649)
Done, thanks

👍 TheCharlatan approved a pull request: "util: Remove DirIsWritable, GetUniquePath"
(https://github.com/bitcoin/bitcoin/pull/28075#pullrequestreview-1551626209)
Nice, ACK fa5bcb6dafe376f81a669bf9324eedecf3feeb8b

💬 TheCharlatan commented on pull request "util: Remove DirIsWritable, GetUniquePath":
(https://github.com/bitcoin/bitcoin/pull/28075#discussion_r1277268812)
Could probably improve the logging a bit here now that there is a diagnostic difference between a locking error and a lock write error.

💬 TheCharlatan commented on pull request "util: Remove DirIsWritable, GetUniquePath":
(https://github.com/bitcoin/bitcoin/pull/28075#discussion_r1277267067)
While testing I noticed that the settings file in the datadir is already interacted with before we do this check. So I think the only way to trigger this is if the lock file has bad permissions.

📝 supernormand opened a pull request: "Create codeql.yml.extracoin"
(https://github.com/bitcoin/bitcoin/pull/28177)
<!--
*** 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/extracoin/
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
extracoin user experience or extracoin developer experience
significantly:

* Any test improvements or new tests that improve coverage a
...

✅ hebasto closed a pull request: "Create codeql.yml.extracoin"
(https://github.com/bitcoin/bitcoin/pull/28177)

📝 MarcoFalke opened a pull request: "fuzz: Generate with random libFuzzer settings"
(https://github.com/bitcoin/bitcoin/pull/28178)
Sometimes a libFuzzer setting like `-use_value_profile=1` helps [0], sometimes it hurts [1].

[0] https://github.com/bitcoin/bitcoin/pull/20789#issuecomment-752961937
[1] https://github.com/bitcoin/bitcoin/pull/27888#issuecomment-1645976254

By picking a random value, it is ensured that at least some of the runs will have the beneficial configuration set.

Also, randomize `-max_len=` to possibly get some runs with faster iterations, or to produce smaller reduced fuzz inputs over time.


...

📝 hebasto locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/28177)
<!--
*** 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/extracoin/
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
extracoin user experience or extracoin developer experience
significantly:

* Any test improvements or new tests that improve coverage a
...

💬 MarcoFalke commented on pull request "util: Remove DirIsWritable, GetUniquePath":
(https://github.com/bitcoin/bitcoin/pull/28075#discussion_r1277284113)
Correct. Or you can disable the settings feature.

💬 MarcoFalke commented on pull request "util: Remove DirIsWritable, GetUniquePath":
(https://github.com/bitcoin/bitcoin/pull/28075#discussion_r1277286108)
I don't think there is a difference, at least when the wallets are placed inside the datadir. Maybe for wallets outside the datadir this is different? In any case, it seems unrelated, as this pull is not changing any behavior.

🤔 aureleoules reviewed a pull request: "Silent Payments: Implement BIP352"
(https://github.com/bitcoin/bitcoin/pull/28122#pullrequestreview-1550940798)
Left some comments. I will review further later.

💬 aureleoules commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1276888369)
65b2dec1e39ea4efb327457563d2b18d49b4bc4c: Could use structured binding

💬 aureleoules commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1276883646)
56882622faf469b6f948f79a69c3c8ddbde92ff8
```suggestion
static CPubKey Combine(const std::vector<CPubKey> &pubkeys);
```