💬 glozow commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1288170497)
Thanks for catching, should be fixed now

💬 glozow commented on pull request "mempool / miner: regularly flush <=0-fee entries and mine everything in the mempool":
(https://github.com/bitcoin/bitcoin/pull/27018#issuecomment-1670959695)
Closing in favor of #27677 which would solve the more general issue of selection score != eviction score.

Noting that #25038 adds trimming of things below min relay feerate, but we only expect those transactions as a result of reorgs or replacement of v3 children. v3 doesn't have the the issue of selection score != eviction score due to topological restrictions.

✅ glozow closed a pull request: "mempool / miner: regularly flush <=0-fee entries and mine everything in the mempool"
(https://github.com/bitcoin/bitcoin/pull/27018)

📝 Sjors opened a pull request: "Silent payment index (for light wallets and consistency check)"
(https://github.com/bitcoin/bitcoin/pull/28241)
This PR adds an index with the silent payment tweak for every transaction. It builds on top of #28122.

Based on the index that was originally part of #24897.

_Not ready for review._

TODO:

- [ ] replace `SilentPaymentIndex::ExtractPubkeyFromInput` with a call to existing helper functions like `SumInputPubKeys`
- [ ] provide a way to dump the index, e.g. using a `getsilentpaymentblockdata` RPC like in 782d7eb215ee37049b636aa3dce82f16615312bd (but getting data from the index)

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1670965334)
I'd like to keep this based on #28122 but that requires some refactoring there. See https://github.com/bitcoin/bitcoin/pull/28122#issuecomment-1670943979.

💬 josibake commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1670999533)
> I'd like to keep this based on #28122 but that requires some refactoring there. See [#28122 (comment)](https://github.com/bitcoin/bitcoin/pull/28122#issuecomment-1670943979).

you can also rebase on https://github.com/bitcoin/bitcoin/pull/27827 , which has everything, or just the receiving PR. Might be easier then to reason about which functions belong in which PRs if we can see it all together. I think the functions you need are introduced in the receiving PR (#28202)

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1288228607)
Removed mention of "modified," as it's not really relevant whether it's real fees or not

💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1288237266)
> That seems to be the case when opening this against my fork: [jarolrod#2](https://github.com/jarolrod/bitcoin/pull/2)

That's correct. But is this really the case we want to care about?

👍 dergoegge approved a pull request: "refactor: Remove unused boost signals2 from torcontrol"
(https://github.com/bitcoin/bitcoin/pull/28240#pullrequestreview-1569180518)
utACK faaba770e11352ddf6414b9855f4baa46a967580

📝 vincenzopalazzo opened 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
...

💬 dergoegge commented on pull request "Revert "Remove unused raw-pointer read helper from univalue"":
(https://github.com/bitcoin/bitcoin/pull/28242#issuecomment-1671025800)
I had this too and iirc `make clean && make` worked for me.

💬 vincenzopalazzo commented on pull request "Revert "Remove unused raw-pointer read helper from univalue"":
(https://github.com/bitcoin/bitcoin/pull/28242#issuecomment-1671028960)
>I had this too and iirc make clean && make worked for me.

Mh, let me try

💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1671034496)
Updated fe34609476b11cdd907c298f7300d424bf556e98 -> 9658d0dc17c270138523c41a982425e276b24271 ([pr28187.05](https://github.com/hebasto/bitcoin/commits/pr28187.05) -> [pr28187.06](https://github.com/hebasto/bitcoin/commits/pr28187.06), [diff](https://github.com/hebasto/bitcoin/compare/pr28187.05..pr28187.06)):

- addressed @MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1288122926)
- the workflow run name is set as the [commit message](https://docs.github.com/e
...

👍 fanquake approved a pull request: "ci: Use qemu-user through container engine"
(https://github.com/bitcoin/bitcoin/pull/28087#pullrequestreview-1569205742)
ACK fad0b67c212dcb8a16fcbda5a74acc959ed4e284 - this seems ok to me, and removes complexity from our CI system.

I've locally tested the ENVs changed here, and seen the associated slowdown / resource consumption. Had GCC segfault more than once when cross-compiling s390x.

🚀 fanquake merged a pull request: "ci: Use qemu-user through container engine"
(https://github.com/bitcoin/bitcoin/pull/28087)

💬 vincenzopalazzo commented on pull request "Revert "Remove unused raw-pointer read helper from univalue"":
(https://github.com/bitcoin/bitcoin/pull/28242#issuecomment-1671049248)
It worked, thanks! sorry for the confusion here

✅ vincenzopalazzo closed a pull request: "Revert "Remove unused raw-pointer read helper from univalue""
(https://github.com/bitcoin/bitcoin/pull/28242)

💬 stickies-v commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1288266548)
> It is not possible to "move" a const reference into a shared_ptr. It will be a copy again.

Sorry, I messed up my git diff a bit, posting a slightly older version, `auto& ref = result->get_type();` has to be `auto&& ref = result->get_type();` (updated in the above diff now). With that, I think `std::move(ref)` is only called in case of an rvalue, and we use the (from what I understand without copy) `std::shared_ptr` constructor in case of an lvalue ref. I think that avoids unnecessary copie
...

💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1288269938)
> Sorry, I messed up my git diff a bit, posting a slightly older version, `auto& ref = result->get_type();` has to be `auto&& ref = result->get_type();` (updated in the above diff now). With that, I think `std::move(ref)` is only called in case of an rvalue, and we use the (from what I understand without copy) `std::shared_ptr` constructor in case of an lvalue ref. I think that avoids unnecessary copies?

Pretty sure it doesn't, when `get_type()` returns a `const std::string&`, both `auto&` an
...

💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1288270672)
> But if `get_type()` returns by value, how would we use a raw pointer? Who owns the underlying? That's why I went for `shared_ptr`, but I'm still easily confused by memory management so apologies if I'm being dim.

`std::optional` can be used to own optional memory, see my previous reply to the thread and the current version of the code :)