💬 fanquake commented on issue "Permission to comment on closed PRs":
(https://github.com/bitcoin/bitcoin/issues/27234#issuecomment-1500359805)
> Could a maintainers unlock an old PR of mine https://github.com/bitcoin/bitcoin/pull/21353?

Done.

💬 ryanofsky commented on pull request "interfaces: Stop exposing wallet destdata to gui":
(https://github.com/bitcoin/bitcoin/pull/21353#discussion_r1160766690)
In commit "wallet: Add IsAddressUsed / SetAddressUsed methods" (f5ba424cd44619d9b9be88b8593d69a7ba96db26)

Note: behavior accidentally changed here. Used addresses previously were written with "p" values, now they are written with "1" values. The change should not be significant because the values are currently ignored, but future code should continue to treat "1" and "p" the same

🤔 ryanofsky reviewed a pull request: "wallet: Replace use of purpose strings with an enum"
(https://github.com/bitcoin/bitcoin/pull/27217#pullrequestreview-1376251729)
~This PR has 3 acks so maybe it is ready to be merged.~ I think this PR basically looks good as is but it would be a little better if it avoided changing behavior for refund addresses in translateTransactionType (see comment below)

💬 ryanofsky commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1160747569)
In commit "wallet: Replace use of purpose strings with an enum" (ccddce7582a51fd2857ce4d142076167631e9cfe)

It would be good to make it more explicit that value is null for change addresses. Could say:

```c++
/**
* Address label which is always nullopt for change addresses. For sending
* and receiving addresses, it will be set to an arbitrary label string
* provided by the user, or to "", which is the default label. The presence
* or absence of a label is used to distinguish change
...

💬 ryanofsky commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1160759069)
In commit "wallet: Replace use of purpose strings with an enum" (ccddce7582a51fd2857ce4d142076167631e9cfe)

To be more accurate, comment should say `with values set to "1" or "p" if present`. Value was inadvertently changed from `"p"` to `"1"` in f5ba424cd44619d9b9be88b8593d69a7ba96db26 from #21353

💬 ryanofsky commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1160736280)
In commit "wallet: Replace use of purpose strings with an enum" (ccddce7582a51fd2857ce4d142076167631e9cfe)

re: https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1149457810

I think I agree with the suggestion to use a switch statement here. Also:

- It looks like behavior of this code has been changed unintentionally to no longer return hidden in the "refund" case.
- The "if purpose not set, guess" logic is no longer here needed because it is implemented at the lower layer

So
...

📝 fanquake opened a pull request: "test: LLVM/Clang 16 for MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/27436)
Similar to other CI infra changes we've made recently. Move to LLVM/Clang 16 for the MSAN jobs (which is currently using LLVM 12).

💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1500389847)
@Sjors the filter works almost like a grep, so it depends on the release. Starting with v23 the apple binaries were renamed.

⚠️ Bandarfaqih opened an issue: "Enhancing Privacy in the Bitcoin Protocol."
(https://github.com/bitcoin/bitcoin/issues/27437)
This BIP proposes the direct implementation of several privacy and anonymity-enhancing features in the Bitcoin protocol, including CoinJoin, zk-SNARKs, ring signatures, and stealth addresses directly into the Bitcoin protocol. These features were envisioned by Satoshi Nakamoto and other contributors as essential tools for preserving user privacy on the network.

Motivation

Bitcoin's current level of privacy and anonymity is limited due to its public ledger, which allows anyone to view all trans
...

💬 fanquake commented on pull request "test: LLVM/Clang 16 for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160783338)
Could also bump this to 16.0.1 now https://github.com/llvm/llvm-project/compare/llvmorg-16.0.0...llvmorg-16.0.1.

💬 fanquake commented on issue "Enhancing Privacy in the Bitcoin Protocol BIP Proposal":
(https://github.com/bitcoin/bitcoin/issues/27437#issuecomment-1500396999)
Thanks. However BIPs (and related discussion) should be sent to the bitcoin-dev mailing list. Opening an issue here is not the right place. See https://github.com/bitcoin/bips#readme.

✅ fanquake closed an issue: "Enhancing Privacy in the Bitcoin Protocol BIP Proposal"
(https://github.com/bitcoin/bitcoin/issues/27437)

💬 vasild commented on pull request "doc: FreeBSD DataDirectoryGroupReadable Setting":
(https://github.com/bitcoin/bitcoin/pull/26741#issuecomment-1500398188)
I usually resort to `git rebase -i`. With the current history (a877011f64):

```sh
git rebase -i HEAD~3
```
an editor will open that contains this:
```
pick 0a6f9b4440 doc: Updating tor.md doc to include mention of FreeBSD requiring the DataDirectoryGroupReadable setting
pick 3c531ed814 doc: update DataDirectoryGroupReadable 1 in tor.md
pick a877011f64 doc: update DataDirectoryGroupReadable 1 in tor.md
```
change the last two lines to begin with `f` and `f -C`, like this:
```
pick 0
...

👍 MarcoFalke approved a pull request: "test: LLVM/Clang 16 for MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/27436#pullrequestreview-1376328538)
lgtm

💬 MarcoFalke commented on pull request "test: LLVM/Clang 16 for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160787798)
any reason to change this here and elsewhere? Either you use `update-alternatives`, or hardcode the version, but doing both seems not needed?

💬 fanquake commented on pull request "test: LLVM/Clang 16 for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160790846)
Right, no need to change these with update-alternatives. Dropped.

💬 fanquake commented on pull request "test: LLVM/Clang 16 for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160790999)
Moved to 16.0.1.

🤔 Sjors reviewed a pull request: "[Draft / POC] Silent Payments"
(https://github.com/bitcoin/bitcoin/pull/24897#pullrequestreview-1376326898)
Don't forget to rebase. The bech32 hrp stuff moved to `kernel/chainparams.{h,cpp}`.

> The silentpayment index has been removed from the codebase. … A new PR, with the index, will be added soon.

That's nice. It's more important to prove functionality, can improve performance later.

The `rawtr()` functionality has been merged, so the dependency can be removed from the PR description.

Enabling the ECDH module in `libsecp256k1` might be a reason to put silent payments behind a configure
...

💬 Sjors commented on pull request "[Draft / POC] Silent Payments":
(https://github.com/bitcoin/bitcoin/pull/24897#discussion_r1160786689)
3742cc75571a60bda52a41923c8244535af9ff23: maybe move that above `UNKNOWN`?

💬 Sjors commented on pull request "[Draft / POC] Silent Payments":
(https://github.com/bitcoin/bitcoin/pull/24897#discussion_r1160804856)
9143c7e1f1710f123072cc272b66ab2cad0eb57a: this commit won't compile, probably needs something from a later commit