💬 ajtowns commented on pull request "RPC: Accept options as named-only parameters":
(https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1515021149)
They're both experimental RPCs, so the fact that dropping support for them as a positional param (1) is a backwards incompatibility doesn't seem terribly important? That we can do positional+named params easily now also seems like it doesn't make things very inconvenient?

Could add an `RPCArgOptions{.also_positional=true}` to mark the `options` members that are intentionally duplicating positional arguments.

🚀 fanquake merged a pull request: "ci: build libc++ in DEBUG mode in MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/27448)

👋 fanquake's pull request is ready for review: "ci: standardize custom libc++ usage in MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/27485)

💬 fanquake commented on pull request "ci: standardize custom libc++ usage in MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27485#issuecomment-1515100837)
Rebased past #27448, and taking out of draft.

📝 fanquake opened a pull request: "[WIP] ci: Use DEBUG=1 in depends for MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/27495)
Followup to #27448, which was deffered, as it produces #27448 and another similar issue in sqlite, see comment here: https://github.com/bitcoin/bitcoin/pull/27448#issuecomment-1514902450.

💬 fanquake commented on pull request "ci: build libc++ in DEBUG mode in MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27448#discussion_r1171669217)
Followed up in #27495.

💬 fanquake commented on issue "test: use-of-uninitialized-value in sqlite3Strlen30":
(https://github.com/bitcoin/bitcoin/issues/27222#issuecomment-1515129711)
See #27495, which should reproduce this, and a similar issue on aarch64:
```bash
2023-04-19T14:57:40.859244Z [test] [validation.cpp:2834] [ConnectTip] [bench] - Load block from disk: 0.33ms [0.00s (infms/blk)]
2023-04-19T14:57:40.859773Z [test] [validationinterface.cpp:25Uninitialized bytes in __interceptor_strcmp at offset 0 inside [0xe01000000e0c, 1)
==37748==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0xaaaab8a82008 in sqlite3BtreeOpen /home/fedora/ci_scratch/depends/wo
...

💬 ryanofsky commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1171681447)
In commit "refactor: Replace string chain name constants with ChainTypes" (5589a4b0dd5b25017299f666127b7537f5f7a4c1)

It seems messy and potentially error prone for these functions convert from enum values to strings and back to enums again. Would suggest avoiding unneeded conversions with something like:

```c++
std::variant<ChainType, std::string> ArgsManager::GetChainArg() const
{
...
const auto chain_arg = GetArg("-chain");
const bool fRegTest = get_net("-regtest");

...

👍 ryanofsky approved a pull request: "refactor: Move chain constants to the util library"
(https://github.com/bitcoin/bitcoin/pull/27491#pullrequestreview-1392475293)
Code review ACK bbe05915adad45560907b5a9b58fc55052838c08. Left a few suggestions, but this looks good in its current form

💬 ryanofsky commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1171598387)
In commit "refactor: Create chaintype files" (bd6493bf37554b1fc5848d81bce2bc5487a952dd)

Might be better to move the `throw` after the `switch` statement and drop the `default:` case so the compiler would warn about unhandled enum values

Same suggestion applies to `CreateChainParams` and `CreateBaseChainParams` in the next commit

💬 ryanofsky commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1171619487)
In commit "refactor: Replace string chain name constants with ChainTypes" (5589a4b0dd5b25017299f666127b7537f5f7a4c1)

This comment is a little confusing, would suggest `// Remove this special case when testnet CBaseChainParams::DataDir() is incremented to "testnet4"`

💬 ryanofsky commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1171633479)
In commit "refactor: Replace string chain name constants with ChainTypes" (5589a4b0dd5b25017299f666127b7537f5f7a4c1)

I noticed there are still mentions some of the GetChainName functions in comments that could be replaced (`git grep -n GetChainName`)

💬 achow101 commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1171716388)
In 1a9a349a026af2865e5446091f1e072b7a845819 "wallet: skip block scan if block was created before wallet birthday"

`CBlock` already has a `GetBlockTime()`, so I don't think it's necessary to include the time as a separate field here.

However I'm not sure that we should be using the block's timestamp for this time rather than MTP. With the block timestamp, we could ignore a more recent block that has a timestamp older than its parent which we wouldn't ignore. That would be kinda bad.

💬 achow101 commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1171720322)
In 1a9a349a026af2865e5446091f1e072b7a845819 "wallet: skip block scan if block was created before wallet birthday"

`m_birth_time` could be initialized with `std::numeric_limits<in64_t>::max()` which would remove the need to compare against `BLANK_BIRTH_TIME`.

💬 jamesob commented on pull request "assumeutxo: keep cache when flushing snapshot (#17487 followup)":
(https://github.com/bitcoin/bitcoin/pull/27008#issuecomment-1515221746)
@Sjors how did you detect that RAM *wasn't* being freed? Did you verify that the flush did not happen based on an absence of logs?

Measures of memory can be deceptive; I remember bitcoinperf spuriously detecting an increase in memory usage because once claimed, the process' resident set size (RSS) doesn't necessarily go down upon free.

💬 pinheadmz commented on issue "Improve transaction privacy / fungibility in Bitcoin Core and the Bitcoin system [meta tracking issues]":
(https://github.com/bitcoin/bitcoin/issues/6568#issuecomment-1515251239)
Privacy and fungibility are crucial, ongoing goals of the project. This meta tracking issue has gotten stale and I think it is safe to close. Almost all the recommendations in the OP have been developed or are in-progress such as silent payments, address groupings, and Taproot. 🚀

✅ pinheadmz closed an issue: "Improve transaction privacy / fungibility in Bitcoin Core and the Bitcoin system [meta tracking issues]"
(https://github.com/bitcoin/bitcoin/issues/6568)

💬 achow101 commented on pull request "net, contrib: Bugfix for checking bad dns seeds without casting in `makeseeds.py`":
(https://github.com/bitcoin/bitcoin/pull/26681#issuecomment-1515270617)
ACK 3cc989da5c750e740705131bed05bbf93bfdf169

💬 ryanofsky commented on pull request "RPC: Accept options as named-only parameters":
(https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1515315562)
Thanks, I added the check and flag you suggested in a new commit.

---

Added 1 commits 927a910cbe6c96d60c6a8b05baad179ef3f61f1c -> 284173937d156e1c0f291abe0b82274468d242e8 ([`pr/nonly.12`](https://github.com/ryanofsky/bitcoin/commits/pr/nonly.12) -> [`pr/nonly.13`](https://github.com/ryanofsky/bitcoin/commits/pr/nonly.13), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/nonly.12...pr/nonly.13))

💬 hebasto commented on pull request "build: Add CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/25797#issuecomment-1515397369)
Updated up to [pr25797.59](https://github.com/hebasto/bitcoin/commits/pr25797.59):

Rebased on the current [cmake-staging](https://github.com/hebasto/bitcoin/tree/cmake-staging) branch (first 25 out of a total of 75 commits).

All CI tasks are :green_circle:

Guix builds:
```
25a62ef8d51c07799b2fe3f687f2340b9f5cf2523079ed12cdc1bbac07b38188 guix-build-33b2db8e6f7c/output/aarch64-linux-gnu/SHA256SUMS.part
af60d9a558ed74492cdf2697d54c3a25aa236a29721269a0eaa35a37997724bd guix-build-33b2d
...