💬 ryanofsky commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1429983108)
I could be wrong, but the current version of this 41e9a810837b1c1eb497c2c22a6fa9873f379836 appears to change behavior by only creating the `<datadir>/<netdir>/wallets` directory, and no longer creating the `<datadir>/wallets` one level up if `<datadir>` did not exist.

This seems like a potentially bad thing, because if testnet or signet or regtest bitcoin is started mainnet bitcoin, mainnet wallets will go be created in `<datadir>` not `<datadir>/wallets` as intended. I think more ideally thi
...

📝 fanquake opened a pull request: "build: produce a .zip for macOS distribution"
(https://github.com/bitcoin/bitcoin/pull/27099)
Reviving the discussion around using a `.zip` for the distributed macOS binaries, as opposed to a `.dmg`.

The commits can be improved, but currently good enough for (non-guix) testing / discussion.

Given we only had a single report of the "no finder window" issue (#26176), I wonder if that means macOS users were able to figure it out, they gave up/didn't report, or, we just have very few macOS users.

Related to #18128.

💬 fanquake commented on issue "build: Use zip instead of dmg for macOS":
(https://github.com/bitcoin/bitcoin/issues/18128#issuecomment-1429990669)
Opened a new PR to discuss this change in #27099.

💬 achow101 commented on issue "Expose PSBT AddInput/AddOutput via RPC":
(https://github.com/bitcoin/bitcoin/issues/19608#issuecomment-1430001293)
They could be added with after PSBTv2 (once #21283 is merged) since PSBTv2 actually supports this operation.

💬 achow101 commented on pull request "wallet: SecureString to allow null characters":
(https://github.com/bitcoin/bitcoin/pull/27068#issuecomment-1430018623)
> I googled "random password generator" and with the [second result](https://passwordsgenerator.net/) I was able to generate a null-character containing password (\039"`\9%|+']1+5)

But that doesn't actually contain a null character. That contains a backslash followed by a 0, which some things may interpret as a null character, but not always, and not necessarily in a way that will affect us. For example. my terminal will interpret it as a null character if I don't put quotes around it. It wil
...

👍 jonatack approved a pull request: "net: avoid overriding non-virtual ToString() in CService and use better naming"
(https://github.com/bitcoin/bitcoin/pull/25619)

📝 MarcoFalke opened a pull request: "ci: Add CLA bot"
(https://github.com/bitcoin/bitcoin/pull/27100)
Not sure for which files we want to enforce copyright headers, if any at all. However if we do it for any files, it should be enforced by CI. See also the section "Copyright" in https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#copyright

💬 brunoerg commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1106079939)
Got it, thank you!

👍 fanquake approved a pull request: "test: Fix intermittent sync issue in wallet_pruning"
(https://github.com/bitcoin/bitcoin/pull/27093)

✳️ fanquake pushed commits to a branch: bitcoin/bitcoin:master
(https://github.com/bitcoin/bitcoin/compare/d6ef44cccbdf...af49d86dd740)
Merge bitcoin/bitcoin#27093: test: Fix intermittent sync issue in wallet_pruning

fa9ec7b0fecd198d3b659d5197c6032416b1551f test: Fix intermittent sync issue in wallet_pruning (MarcoFalke)

Pull request description:

The `sync_fun=self.no_op` has no motivation or rationale, and seems to be causing issues.

Fix that by removing it.

Actually fixes https://github.com/bitcoin/bitcoin/issues/27065, see https://github.com/bitcoin/bitcoin/pull/27066#issuecomment-1428249997

ACKs for top commit:
fanquake:
ACK fa9ec7b0fecd198d3b659d5197c6032416b1551f

Tree-SHA512: 3c67da6705d6698fcabb29de169a2b4723f74705c979380d1fddce5fe9595b4595445fd7d9790a6b2a89f10ce8ec3c64ce45248f58fd920b72b7b6fba8afb09f

🚀 fanquake merged a pull request: "test: Fix intermittent sync issue in wallet_pruning"
(https://github.com/bitcoin/bitcoin/pull/27093)

✳️ fanquake pushed commits to a branch: bitcoin/bitcoin:master
(https://github.com/bitcoin/bitcoin/compare/af49d86dd740...fb2f0934799a)
Merge bitcoin/bitcoin#27097: descriptors: fix docstring (param [in] vs [out])

588fad868dd49b5baca26170c2adca8544fed04b descriptors: fix docstring (param [in] vs [out]) (SomberNight)

Pull request description:

As in title, these docstrings look incorrect.

ACKs for top commit:
john-moffett:
ACK 588fad868dd49b5baca26170c2adca8544fed04b

Tree-SHA512: 1ab343a1b1fc57a7d6bd8363b84db9d96e8ea11a4cec85bcf79885c9df53da889fe2fb10b1fa92d824ddf0dee800c07353f46f1fea9887d2ad518bed0afebe3d

🚀 fanquake merged a pull request: "descriptors: fix docstring (param [in] vs [out])"
(https://github.com/bitcoin/bitcoin/pull/27097)

👍 john-moffett approved a pull request: "validation: Improve error handling when VerifyDB fails due to insufficient dbcache"
(https://github.com/bitcoin/bitcoin/pull/25574)

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1430090612)
`9b656a5...7a93d7b`: rebase and address suggestions

💬 prusnak commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1430092015)
Concept ACK

Here's a revised version of the passage with improved grammar and natural-sounding language:

At present, the user experience with DMG is terrible (as noted in this issue: https://github.com/bitcoin/bitcoin/issues/26176). If there's no straightforward solution on the horizon, it would be best to switch to using ZIP instead. Using DMG without a Finder window provides no advantages and only leads to a lot of confusion. As evidence, Visual Studio Code has recently started distribut
...

👍 fanquake approved a pull request: "verify-commits: Bump trusted git root to after most recent laanwj merge"
(https://github.com/bitcoin/bitcoin/pull/27076)

💬 adam2k commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1430135758)
This is great! ACK: https://github.com/bitcoin/bitcoin/commit/12758d3f69c4900f0d23df06e1ad746417428f3a

I see from the `NACK` comments that the reviewers seem to mostly be pushing back on this portion:

> ## Roadmap
> There are two goals:
>
> * merge all CMake code by v25.0 and test it
> * switch to the CMake-based build system by v26.0

In the spirit of having a complete git working history in the `master` branch, I understand the viewpoint.

Are there any other pieces (without ov
...

📝 hebasto opened a pull request: "refactor: Disable unused special members functions in `UnlockContext`"
(https://github.com/bitcoin-core/gui/pull/711)
Also `UnlockContext::valid` and `UnlockContext::relock` are `const` now.

💬 pinheadmz commented on issue "Manual-pruning cursor rewind":
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1430157210)
Downloading arbitrary blocks is certainly possible (this is how neutrino clients work) however, downloading old blocks would not include their UNDO data. This means you might be able to download old blocks to (for example) rescan a wallet but you would not be able to handle a deeper chain reorg than the original prune depth.

What might be possible is to increase the prune depth by 1 with every new incoming block, until a new desired prune depth is reached. The mechanism there would be to simp
...