💬 Sjors commented on pull request "Silent Payments: sending":
(https://github.com/bitcoin/bitcoin/pull/28201#issuecomment-1663589369)
Maybe add a quick summary in the description with the main implementation differences relative to #24897. It seems a big one is that this doesn't require an index!

📝 MarcoFalke opened a pull request: "mempool: Persist with XOR"
(https://github.com/bitcoin/bitcoin/pull/28207)
Currently the `mempool.dat` file stores data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan the file and move it into quarantine, or delete it, or corrupt it.

While the local wallet is expected to re-submit any pending transactions, unrelated transactions may be missing from the mempool after a restart.

Fix this, similar to https://github.com/b
...

💬 Sjors commented on pull request "Silent Payments: send and receive":
(https://github.com/bitcoin/bitcoin/pull/27827#issuecomment-1663592220)
Maybe add a quick summary in the description with the main implementation differences relative to #24897. It seems a big one is that this doesn't require an index!

You should also link from the child PRs back to this one.

💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1663593182)
Currently opened as draft to wait for initial feedback. There is no option to disable this feature, because I am not aware of anyone reading the `mempool.dat`, is there? (The `getrawmempool` RPC is the recommended way to get the mempool, and using the `savemempool` RPC instead seems like an edge-case?)

📝 fanquake opened a pull request: "lint: remove pkg_resources usage"
(https://github.com/bitcoin/bitcoin/pull/28208)
`pkg_resources` is deprecated, and warns with newer Python:
```bash
/bitcoin/test/lint/lint-python.py:12: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
import pkg_resources
```

Switch to using `importlib.metadata`, which has existed since Python 3.8.

See: https://docs.python.org/3/library/importlib.metadata.html#module-importlib.metadata.
See: https://setuptools.pypa.io/en/latest/pkg_resources.html

💬 MarcoFalke commented on issue "Master doesn't compile on MacOS X 10.13 High Sierra":
(https://github.com/bitcoin/bitcoin/issues/28206#issuecomment-1663609935)
For reference, you can read the release notes of the release you are interested in to see which version of macOS is supported. For example:

https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-25.0.md#compatibility

💬 Sjors commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1663611211)
Concept ACK.

It may be difficult to find out if anyone relies on parsing `mempool.dat`. If it's not too hard, we might as well add an option (default `1`) and deprecate it in a few releases. That also makes it easier to toggle between master and the last release (by setting it to `0`).

You may also want to use a bit flag instead of increasing the version. The added complexity of that could be an argument to _not_ make this optional.

💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1663626986)
I don't think bit flags make sense in this context, because the `mempool.dat` frequently changes and is at most expected to be read by the previous version. However, your suggestion to use a setting makes sense. The setting could have several options, like "write version 1 mempool.dat", and "use `0` for all random numbers" (if needed), and "use prng".

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1663631661)
I think it is fine to merge this, and then fixup the two doc nits in the @theuni follow-up?

💬 fanquake commented on issue "Master doesn't compile on MacOS X 10.13 High Sierra":
(https://github.com/bitcoin/bitcoin/issues/28206#issuecomment-1663634124)
Think we can close this for now, as it's not a bug, and I don't think there's anything we can do. @feed3r feel free to keep commenting if you have further questions etc.

✅ fanquake closed an issue: "Master doesn't compile on MacOS X 10.13 High Sierra"
(https://github.com/bitcoin/bitcoin/issues/28206)

💬 MarcoFalke commented on pull request "ci: Move ASan USDT to persistent_worker":
(https://github.com/bitcoin/bitcoin/pull/28161#issuecomment-1663639215)
There may be a language barrier here, so I am wondering if anything is left to be done here or if there are any outstanding suggestions or questions. Just to clarify, that this pull does not aim to change any behavior of the CI system. The second commit is a mandatory "refactor" needed for the persistent worker on CIRRUS_CI to work.

💬 hebasto commented on pull request "ci: Move ASan USDT to persistent_worker":
(https://github.com/bitcoin/bitcoin/pull/28161#issuecomment-1663643891)
> There may be a language barrier here, so I am wondering if anything is left to be done here or if there are any outstanding suggestions or questions.

I'm [OK](https://github.com/bitcoin/bitcoin/pull/28161#pullrequestreview-1558764134) with this PR in its current state.

💬 ismaelsadeeq commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1282947793)
Will previous versions be able to read the mempool dump version 2? `mempool.dat` suppose to be both backward and forward-compatible between versions

💬 josibake commented on pull request "Silent Payments: sending":
(https://github.com/bitcoin/bitcoin/pull/28201#issuecomment-1663670668)
> Maybe add a quick summary in the description with the main implementation differences relative to #24897. It seems a big one is that this doesn't require an index!

updated! I added the summary in #27827 and added links back to the parent PR in each of the child PRs.

💬 fanquake commented on pull request "lint: remove pkg_resources usage":
(https://github.com/bitcoin/bitcoin/pull/28208#issuecomment-1663679742)
I guess `packages_distributions` isn't actually available until 3.10. Will ignore and come back to this later.

✅ fanquake closed a pull request: "lint: remove pkg_resources usage"
(https://github.com/bitcoin/bitcoin/pull/28208)

📝 fanquake converted_to_draft a pull request: "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)"
(https://github.com/bitcoin/bitcoin/pull/28175)
There's been at least a few instances where someone tried to contribute LLM-generated content, but such content has a dubious copyright status.

Our contributing policy already implicitly rules out such contributions, but being more explicit here might help.

💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1282961735)
I've updated the BIP to recommend upping the character limit to 1023, as having a limit of 117 would cause issues with forward compatibility with future silent payment addresses. I've also removed regtest from the BIP, as I don't think it's correct to define Bitcoin Core specific conventions in BIPs. Regarding "tsp" vs "sprt", I don't really have a strong opinion here, but would prefer to follow the convention in Bitcoin Core. If we end up moving Bitcoin Core to use the same HRP for all networks
...

💬 fanquake commented on pull request "CONTRIBUTING: Caution against using AI/LLMs (ChatGPT, Copilot, etc)":
(https://github.com/bitcoin/bitcoin/pull/28175#issuecomment-1663685355)
Moved to draft for now, as there's not consensus to merge as-is, and in any case, this is waiting on further legal opinions.