👍 hodlinator approved a pull request: "common: Make arith_uint256 trivially copyable"
(https://github.com/bitcoin/bitcoin/pull/33332#pullrequestreview-3213497518)
re-ACK 653a9849d5f98ba80e334ddc0ae9a5e367459f59

New push only adds `static_assert` with motivation.

📝 BrandonOdiwuor opened a pull request: "ci: use Mold linker for asan-lsan-ubsan-integer-no-depends-usdt workflow"
(https://github.com/bitcoin/bitcoin/pull/33370)
Follow up to https://github.com/bitcoin/bitcoin/pull/32888#pullrequestreview-2993523631 and https://github.com/bitcoin/bitcoin/pull/32888#issuecomment-3044773485

>>Can we use `mold` as a linker in other Linux based system workflows ? dependencies [we have](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md#compiler) seem to satisfy the deps here https://github.com/rui314/mold?tab=readme-ov-file#how-to-build
>
> Sure, happy to review a follow-up. Only place to avoid it would
...

💬 achow101 commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2342296606)
In 4d07eff13002384ed59a3b7f592be56a25b88c15 "descriptor: ToPrivateString() pass if at least 1 priv key

`has_priv_key` is a misnomer since for public strings, it has a completely different meaning. I would prefer naming this something more generic, like `any_success`, here and elsewhere.

💬 achow101 commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2342299196)
In 4d07eff13002384ed59a3b7f592be56a25b88c15 "descriptor: ToPrivateString() pass if at least 1 priv key exists"

nit: can be simplified to

```suggestion
return type != StringType::PRIVATE
```

💬 achow101 commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2342300386)
In 4d07eff13002384ed59a3b7f592be56a25b88c15 "descriptor: ToPrivateString() pass if at least 1 priv key exists"

`has_priv_key` does not need to be a pointer here.

🚀 achow101 merged a pull request: "kernel: make blockTip index const"
(https://github.com/bitcoin/bitcoin/pull/33321)

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2342348714)
We want to get the private key early so that we know whether we will be able to even do anything here.

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2342368768)
I actually prefer verbosity in type names.

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2342376993)
For a followup or if I need to retouch.

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3282694791)
> It would be helpful to mention why this is done in the commit message

If I need to retouch.

💬 achow101 commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#issuecomment-3282710122)
ACK 653a9849d5f98ba80e334ddc0ae9a5e367459f59

🚀 achow101 merged a pull request: "common: Make arith_uint256 trivially copyable"
(https://github.com/bitcoin/bitcoin/pull/33332)

💬 davidgumberg commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#issuecomment-3282744720)
The members of `base_uint` are all [trivially copyable](https://en.cppreference.com/w/cpp/named_req/TriviallyCopyable.html): `WIDTH` is a scalar, and `pn` is an array of scalars, so the class is trivially copyable and we should use the implicit copy constructors of the class. But, by explicitly declaring the implicitly defined copy constructors the move constructors are suppressed.

<details><summary>

#### (https://en.cppreference.com/w/cpp/language/move_constructor.html)

</summary>


...

💬 Raimo33 commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#issuecomment-3282754152)
if I understand this correctly, move constructors were suppressed before this PR as well.

💬 davidgumberg commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#issuecomment-3282762069)
> if I understand this correctly, move constructors were suppressed before this PR as well.

They were, so this is not a regression, but it would be better for the implicitly defined move constructors to be usable.

💬 Raimo33 commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#issuecomment-3282765661)

> it would be better for the implicitly defined move constructors to be usable.

good catch, totally agree, let's run some benchmarks

💬 achow101 commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3282783147)
ACK 3bdff9a154733f8f9f379448f5595a2e90474bc6

💬 achow101 commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#issuecomment-3282836726)
ACK fa96a4afea2a9bf90c843198e75a00acef02c32d

🚀 achow101 merged a pull request: "test: Fix CLI_MAX_ARG_SIZE issues"
(https://github.com/bitcoin/bitcoin/pull/33243)

💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3282884135)
I see that there were two failing checks -- is there any further suggested action I could take to help with this? I'm beginning to investigate now to try to understand what failed and why.