💬 MarcoFalke commented on pull request "refactor: serialization simplifications":
(https://github.com/bitcoin/bitcoin/pull/28203#discussion_r1282771203)
review note in 10105d97f412cc54ba4031c56538f7e0c611251f: A simple `is_same_v` work, because I presume `T` can never include `const` in a prevector, similar to how `std::vector<const int>` does not exist.

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1282776719)
No, I did not consider that.

💬 MarcoFalke commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#discussion_r1282779911)
Yes, I am aware, but I think this bug should be reported to IWYU, or is it not a bug?

✅ MarcoFalke closed an issue: "Berkley Database Errors"
(https://github.com/bitcoin/bitcoin/issues/28197)

💬 martinus commented on pull request "refactor: serialization simplifications":
(https://github.com/bitcoin/bitcoin/pull/28203#discussion_r1282836964)
yes, I'll clang-format the changes

💬 martinus commented on pull request "refactor: serialization simplifications":
(https://github.com/bitcoin/bitcoin/pull/28203#discussion_r1282837030)
Ah, actually I didn't think about temporaries at all... I changed it to be more in line with `Serialize` which uses `const Args&... args`, so `Args&... args` felt more appropriate.

I'll change it back to &&

💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1282854035)
This issue has some helpful context: https://github.com/bitcoin/bitcoin/issues/12314#issuecomment-582600899

TLDR; at the time regtest was created, it was assumed that using a separate HRP wouldn't cost anything.

> If each of the four had a different one, I’d get it, but why does regtest have a different one than the other two testing networks?

This actually makes sense to me. Testnet and Signet are "global" networks, so everyone has to agree on the address format. Regtest is always used
...

💬 fanquake commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1663552242)
@hebasto did you open an issue to track this upstream? Can you link it here?

💬 martinus commented on pull request "refactor: serialization simplifications":
(https://github.com/bitcoin/bitcoin/pull/28203#issuecomment-1663556136)
Rebased to f054bd0 to clang-format the changes, and adds back the `&&` in `UnserializeMany`.

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