💬 MarcoFalke commented on pull request "ci: Move tidy to persistent worker":
(https://github.com/bitcoin/bitcoin/pull/28214#issuecomment-1671100821)
> It would be nice if we postpone moving macOS-cross job for a week or two.

Done for now. Should be trivial to re-ACK.

👍 hebasto approved a pull request: "ci: Move tidy to persistent worker"
(https://github.com/bitcoin/bitcoin/pull/28214#pullrequestreview-1569282121)
re-ACK fa1d8955f69d3934f975f42eb04b5a3fc0d8aa35.

Thanks you.

💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1288309113)
Enforcing/Documenting the lifetime behavior seems easier to understand with separate types (at least for me)

💬 dergoegge commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1288311782)
> Alternatively it could be disallowed properly.

converting from `uint256` can now only be done using `{Txid,Wtxid}::FromUint256`. This required less changes then expected so I think that is actually preferable.

📝 willcl-ark opened a pull request: "doc: Use GitHub's "Alert" markdown syntax"
(https://github.com/bitcoin/bitcoin/pull/28243)
GH has introduced new "Alert" syntax for md files (a.k.a "Admonitions" in other languages).

Whilst being wary of changing documentation just for the sake of "shiny" new features, in my opinion this is worthwhile in this case as we can use it to draw user attention to particularly important sections of docs.

The format is relatively backwards-compatible (i.e outside of GH), as the syntax is effectively a multi-block quotation, where the first line includes the alert type. This means the tex
...

💬 glozow commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#issuecomment-1671142331)
Fixed accidental drop of TrimToSize commit

📝 fanquake locked a pull request: "Revert "Remove unused raw-pointer read helper from univalue""
(https://github.com/bitcoin/bitcoin/pull/28242)
This reverts commit fa940f41eaffa4b2a28c465a10a4c12d4b8976b8.

With this commit produce the following compilation error.

```
Making all in src
make[1]: Entering directory '/home/vincent/Github/bitcoin/src'
make[2]: Entering directory '/home/vincent/Github/bitcoin/src'
make[3]: Entering directory '/home/vincent/Github/bitcoin'
make[3]: Leaving directory '/home/vincent/Github/bitcoin'
GEN obj/build.h
CXX libbitcoin_util_a-clientversion.o
AR libbitcoin_util.a
C
...

💬 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