💬 MarcoFalke commented on pull request "util: Replace std::filesystem with util/fs.h":
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1661511351)
> Can you elaborate on why this is a problem?

It just makes developer UX worse. The linters are already horrible, with some contributors having quit (at least parts of the code base) because of them. If someone pushes code and then has to wait more than 20 minutes (a full tidy CI run on 4 CPUs, 30+ minutes on a dirty ccache) to figure out they'll have to replace `std::to_string` with `ToString`, it is certainly more annoying than having to wait 30 milliseconds:

```
$ time git grep 'std::t
...

🤔 MarcoFalke reviewed a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-1558219665)
Looks like CI is red?

💬 MarcoFalke commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1281410852)
style nit: I don't think this method is a subset of our streams. Many streams can't seek to the beginning after a read. It may be better to move this up by 3 lines of code (or more if you want)

💬 MarcoFalke commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1281412977)
style nit: If you remove this, it may be possible to feed in a "happy" wallet.dat (created with vanilla Bitcoin Core) as a fuzz input. See https://github.com/bitcoin-core/qa-assets/pull/140#issuecomment-1660088315

Though, I haven't tested this.

💬 MarcoFalke commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1281421341)
style nit: `const DataStream&` seems fine here, but if you want to pass an immutable view of raw bytes, `Span<const std::byte>` may be better. (`Span{ssKey}` should do the conversion, but it will likely happen implicitly by the compiler already).

💬 MarcoFalke commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1281421788)
Also, `CharCast` can be moved to the cpp file in the last commit, if you want

💬 ajtowns commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1281498873)
A `std::array` can be viewed as a specific form of map that uses keys from a range of integers starting at 0 that's known at compile time, with either few or no missing elements. Where that's the case, an array is more efficient (smaller storage, no dynamic allocation needed, faster lookups, better caching behaviour etc) and more convenient (eg, `x = a[3]` works even if `a` is a `const&` which isn't true for a map, requiring a [much more convoluted approach]https://github.com/bitcoin/bitcoin/pul
...

💬 darosior commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1661649303)
Concept ACK.

👋 MarcoFalke's pull request is ready for review: "blockstorage: XOR blocksdir *.dat files"
(https://github.com/bitcoin/bitcoin/pull/28052)

💬 MarcoFalke commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1281548137)
Done

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

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

* Any test improvements or new tests that
...

✅ fanquake closed a pull request: "Create extracoin"
(https://github.com/bitcoin/bitcoin/pull/28198)

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

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

* Any test improvements or new tests that
...

💬 MarcoFalke commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1661738608)
Rebased, and made every commit a refactor, except for the one that has release notes and test changes :)

💬 fanquake commented on pull request "test: dedup file hashing using `sha256sum_file` helper":
(https://github.com/bitcoin/bitcoin/pull/27572#issuecomment-1661842143)
Concept ACK. Did you want to address the review comment, or just mark as resolved.

💬 MarcoFalke commented on pull request "lint: fix custom mypy cache dir setting":
(https://github.com/bitcoin/bitcoin/pull/28184#discussion_r1281660991)
If it is not needed, it may be better to remove it.

💬 theStack commented on pull request "test: dedup file hashing using `sha256sum_file` helper":
(https://github.com/bitcoin/bitcoin/pull/27572#discussion_r1281680165)
Thanks for reviewing! Decided to keep the name as it is, as sha256 falls into the SHA message digests category (see e.g. https://linux.die.net/man/1/shasum) and for the sake of the test concrete type of hash used doesn't matter anyway. All we care is detecting if the wallet file contents have been changed.

💬 hebasto commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1661920835)
> Thus, someone would have to sponsor an amount of roughly 5kUSD/mo for those two tasks.

According to Cirrus CI stats for June / July 2023, the usage of resources was equivalent to compute credits/USD as follows:
- Windows -- 2275 / 1941
- Linux -- 2546 / 3160
- macOS -- 2053 / 3568

Therefore, future of Linux tasks should be considered as well.

💬 MarcoFalke commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1661941813)
> Therefore, future of Linux tasks should be considered as well.

Have you seen https://github.com/bitcoin/bitcoin/pull/28161 ?

👍 willcl-ark approved a pull request: "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py"
(https://github.com/bitcoin/bitcoin/pull/27452#pullrequestreview-1558684193)
ACK ba8ab4fc54

Nice to have this additional coverage for TorV3 addresses 👍🏼

It's not intruduced by this PR, but I notice a mypy error in _feature_anchors.py_ where a tuple is assigned to `onion_conf.addr` which is initialized to `None` in _socks5.py_.


If you do end up re-touching this for some reason, you can fix the warning by specifying the type of `.addr` in _socks5.py_ (which will
...