💬 Sjors commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1663560293)
> There's still another crash that I have

Not sure if that was fixed in your later push, but in any case, I pushed another crash to the qa-assets PR.

🤔 MarcoFalke reviewed a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-1560573265)
(nits only)

💬 MarcoFalke commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1282873918)
nit in 167cbf7b1851eeeb28937bd8e241e1f865b813f8:


```cpp
int64_t r{std::ftell(m_file)};

💬 MarcoFalke commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1282875416)
nit in 167cbf7b1851eeeb28937bd8e241e1f865b813f8:

Could add `std::` before `fseek`? Also, the commit message has a typo:

usefule -> useful (remove `e`)

⚠️ feed3r opened an issue: "Master doesn't compile on MacOS X 10.13 High Sierra"
(https://github.com/bitcoin/bitcoin/issues/28206)
Hello guys, I'm trying to compile the current Master branch on a quite old Macbook Pro (early 2011) for which the latest supported os is Mac OS X 10.13 high sierra, and I'm getting the following error:

```
Alessios-MacBook-Pro:src feeder$ make V=1
g++ -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -DHAVE_BUILD_INFO -Xclang -internal-isystem/usr/local/include -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0 -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/inc
...

💬 Sjors commented on pull request "net_processing: re-allow fetching of genesis block via `getblockfrompeer`":
(https://github.com/bitcoin/bitcoin/pull/28205#issuecomment-1663573528)
Or even simpler: have `getblockfrompeer` fail immediately if you ask it for the genesis block.

💬 fanquake commented on issue "Master doesn't compile on MacOS X 10.13 High Sierra":
(https://github.com/bitcoin/bitcoin/issues/28206#issuecomment-1663577646)
> that does not fully support C++17

Master requires a C++17 capable compiler, so you wont be able to compile it with a compiler that doesn't support C++17. I'd suggest installing a modern LLVM/Clang via brew, and using that, if possible.

> Does anybody know if MacOS 10.13 is not supported anymore
> Is it known if MacOS 10.15 Catalina is supported?

On master, the minimum supported macOS version is 11.0. The prior minimum supported macOS version was 10.15.

🚀 fanquake merged a pull request: "qa: Close SQLite connection properly"
(https://github.com/bitcoin/bitcoin/pull/28204)

💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1282885877)
Addressed this.

💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1663582461)
Moving back to draft while we follow up with more suggestions.

📝 fanquake converted_to_draft a pull request: "ci: Integrate `bitcoin-tidy` clang-tidy plugin"
(https://github.com/bitcoin/bitcoin/pull/26296)
Demo of integrating the [bitcoin-tidy](https://github.com/theuni/bitcoin-tidy-plugin), [clang-tidy plugin](https://clang.llvm.org/extra/clang-tidy/) written by theuni into our tidy CI job.

The plugin currently has a single check, `bitcoin-unterminated-logprintf`. This would replace our current Python driven, `git-grep`-based, `.cpp` file only, lint-logs linter.

💬 Sjors commented on pull request "Silent Payments: sending":
(https://github.com/bitcoin/bitcoin/pull/28201#issuecomment-1663589369)
Maybe add a quick summary in the description with the main implementation differences relative to #24897. It seems a big one is that this doesn't require an index!

📝 MarcoFalke opened a pull request: "mempool: Persist with XOR"
(https://github.com/bitcoin/bitcoin/pull/28207)
Currently the `mempool.dat` file stores data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan the file and move it into quarantine, or delete it, or corrupt it.

While the local wallet is expected to re-submit any pending transactions, unrelated transactions may be missing from the mempool after a restart.

Fix this, similar to https://github.com/b
...

💬 Sjors commented on pull request "Silent Payments: send and receive":
(https://github.com/bitcoin/bitcoin/pull/27827#issuecomment-1663592220)
Maybe add a quick summary in the description with the main implementation differences relative to #24897. It seems a big one is that this doesn't require an index!

You should also link from the child PRs back to this one.

💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1663593182)
Currently opened as draft to wait for initial feedback. There is no option to disable this feature, because I am not aware of anyone reading the `mempool.dat`, is there? (The `getrawmempool` RPC is the recommended way to get the mempool, and using the `savemempool` RPC instead seems like an edge-case?)

📝 fanquake opened a pull request: "lint: remove pkg_resources usage"
(https://github.com/bitcoin/bitcoin/pull/28208)
`pkg_resources` is deprecated, and warns with newer Python:
```bash
/bitcoin/test/lint/lint-python.py:12: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
import pkg_resources
```

Switch to using `importlib.metadata`, which has existed since Python 3.8.

See: https://docs.python.org/3/library/importlib.metadata.html#module-importlib.metadata.
See: https://setuptools.pypa.io/en/latest/pkg_resources.html

💬 MarcoFalke commented on issue "Master doesn't compile on MacOS X 10.13 High Sierra":
(https://github.com/bitcoin/bitcoin/issues/28206#issuecomment-1663609935)
For reference, you can read the release notes of the release you are interested in to see which version of macOS is supported. For example:

https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-25.0.md#compatibility

💬 Sjors commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1663611211)
Concept ACK.

It may be difficult to find out if anyone relies on parsing `mempool.dat`. If it's not too hard, we might as well add an option (default `1`) and deprecate it in a few releases. That also makes it easier to toggle between master and the last release (by setting it to `0`).

You may also want to use a bit flag instead of increasing the version. The added complexity of that could be an argument to _not_ make this optional.

💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1663626986)
I don't think bit flags make sense in this context, because the `mempool.dat` frequently changes and is at most expected to be read by the previous version. However, your suggestion to use a setting makes sense. The setting could have several options, like "write version 1 mempool.dat", and "use `0` for all random numbers" (if needed), and "use prng".

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1663631661)
I think it is fine to merge this, and then fixup the two doc nits in the @theuni follow-up?