💬 stickies-v commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1288338213)
That's a good point. To confirm my understanding: the lifecycles are identical for both the (1) hybrid std::optional/pointer and the (2) uniform pointer approach (i.e. in both cases, reference types can only be used during the RPC call, value types can be used whenever), but for (2) the developer (user) would have to dive into the implementation to figure out what the exact lifecycle is, whereas if we use different types it's obvious from just looking at the return type.

I'm going to think ab
...

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1671159616)
re-ACK 9658d0dc17c270138523c41a982425e276b24271 🏂

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 9658d0dc17c270138523
...

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1671166932)
I'll take a stab at rebasing on the bigger thing. It makes sense to do that and then unrebase / debase(?) it later.

💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1288355467)
I could also imagine a third alternative (3) std::shared_ptr/raw pointer, so that everything is a pointer, but there are still different types for each lifetime. (Basically only replacing std::optional in the current approach with std::shared_ptr)

But this also may be more confusing than it helping.

📝 achow101 opened a pull request: "Break up script/standard.{h/cpp}"
(https://github.com/bitcoin/bitcoin/pull/28244)
Some future work needs to touch things in script/standard.{h/cpp}, however it is unclear if it is safe to do so as they are included in several different places that could effect standardness and consensus. It contains a mix of policy parameters, consensus parameters, and utilities only used by the wallet. This PR breaks up the various components and renames the files to clearly separate everything.

* `CTxDestination` is moved to a new file `src/addresstype.{cpp/h}`
* `TaprootSpendData` and
...

💬 MarnixCroes commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1671178937)
Should this be added to the release notes?

💬 MarcoFalke commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1288363762)
But `uint256 b = Txid{}` is still allowed? Obviously here it is obvious. However, is it obvious when not on a single line?

```cpp
struct A{
uint256 a; // wtxid
}
void foo(A&a, Txid&t){
a=t;
}
```

💬 fanquake commented on pull request "ci: Move tidy to persistent worker":
(https://github.com/bitcoin/bitcoin/pull/28214#issuecomment-1671185391)
> I'd prefer run it on GitHub Actions due to the ability to create downloadable artifacts that is used in the QML repo to make designers' life a bit easier.

Why can't this just be done in the QML repo if it's required there?

💬 MarcoFalke commented on pull request "ci: Move tidy to persistent worker":
(https://github.com/bitcoin/bitcoin/pull/28214#issuecomment-1671188144)
I'll create a separate pull/thread for macOS-cross next week, so that it can be discussed.

📝 fanquake opened a pull request: "doc: use llvm-config for bitcoin-tidy example"
(https://github.com/bitcoin/bitcoin/pull/28245)
An LLVM installation will have `llvm-config` available to query for info. Ask it for the `--libdir`, and use that in our bitcoin-tidy example, rather than listing multiple different (potential) paths per distro/OS etc.

💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1288382739)
Done in #28245.

💬 dergoegge commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1288386480)
Yea I think this makes sense while we are converting, otherwise I suspect there'll be more churn (similar to the comparisons).

But how would you suggest to forbid this?

👍 TheCharlatan approved a pull request: "doc: use llvm-config for bitcoin-tidy example"
(https://github.com/bitcoin/bitcoin/pull/28245#pullrequestreview-1569411388)
ACK 8651b998ffc80b2a1b79a47b47f2b54924db0512

💬 achow101 commented on pull request "Silent Payments: receiving":
(https://github.com/bitcoin/bitcoin/pull/28202#discussion_r1288398953)
This looks in the UTXO set but is called on a transaction that has already been verified and so therefore it's inputs are no longer in the UTXO set. This results in the `Coin` being uninitialized which causes later `Solver` calls to fail unexpectedly.

💬 achow101 commented on pull request "Silent Payments: receiving":
(https://github.com/bitcoin/bitcoin/pull/28202#discussion_r1288396529)
Should recurse on `SCRIPTHASH` instead of assuming.

🤔 glozow reviewed a pull request: "net processing: clamp PeerManager::Options user input"
(https://github.com/bitcoin/bitcoin/pull/28149#pullrequestreview-1569427500)
reACK 547fa52443cbb5e8ccfee993486f5ced8cdbb33b

🚀 glozow merged a pull request: "net processing: clamp PeerManager::Options user input"
(https://github.com/bitcoin/bitcoin/pull/28149)

💬 fanquake commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#discussion_r1288431124)
```bash
src/script/solver.h seems to be missing the expected include guard:
#ifndef BITCOIN_SCRIPT_SOLVER_H
#define BITCOIN_SCRIPT_SOLVER_H
...
#endif // BITCOIN_SCRIPT_SOLVER_H
```

💬 achow101 commented on pull request "Silent Payments: receiving":
(https://github.com/bitcoin/bitcoin/pull/28202#discussion_r1288438010)
As discussed offline, we shouldn't use any view of the UTXO set. Rather we should use the undo data when receiving a `blockConnected`, and modify `TransactionAddedToMempool` to include the previous outputs. This lets us avoid any race conditions with the state of the UTXO set and guarantees that the previous output data will always be available for the silent payments calculations.

💬 theuni commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1671279124)
Yep, will do. I was just waiting for the others to settle.