💬 mzumsande commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#issuecomment-1429968540)
> which you've probably already caught

I hadn't, thanks for noticing! Fixed.

💬 hebasto commented on pull request "refactor: don't avoid sys/types.h when building for Windows":
(https://github.com/bitcoin/bitcoin/pull/27098#issuecomment-1429969983)
> Aside from ultimately removing pointless per-platform conditionals,

But this change does not remove per-platform conditionals.

> See #26832.

Yes, I agree with cc16ab13f42b83672030fdd03c65fe27699854ee from #26832.

💬 jonatack commented on pull request "Modernize rpcauth.py":
(https://github.com/bitcoin/bitcoin/pull/27081#issuecomment-1429971743)
> tACK Tested as follows: generated new userpw using an explicit password. Used it on signet through an RPC request (checked I got 'incorrect password attempt' if I changed the userpw a bit).

@codo1 If this comment can be helpful, your review is listed as ignored in https://github.com/bitcoin/bitcoin/pull/27081#issuecomment-1426430503 and is not part of the merge commit because your ACK isn't followed by the commit hash. See these links for more information:

- https://jonatack.github.io/a
...

💬 fanquake commented on pull request "refactor: don't avoid sys/types.h when building for Windows":
(https://github.com/bitcoin/bitcoin/pull/27098#issuecomment-1429971818)
> But this change does not remove per-platform conditionals.

Yea, that's because this is a step towards doing so.

💬 fanquake commented on pull request "contrib: Check symbols in the `bitcoinconsensus` shared library":
(https://github.com/bitcoin/bitcoin/pull/25020#discussion_r1106015544)
NACK. This is accomodating a bug (in the DLL), and doing so in such a way, that it could leak into the actual binaries.

💬 hebasto commented on pull request "contrib: Check symbols in the `bitcoinconsensus` shared library":
(https://github.com/bitcoin/bitcoin/pull/25020#discussion_r1106019516)
> This is accomodating a bug...

Which one?

💬 fanquake commented on pull request "contrib: Check symbols in the `bitcoinconsensus` shared library":
(https://github.com/bitcoin/bitcoin/pull/25020#discussion_r1106020657)
Non-static libssp.

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