💬 fjahr commented on pull request "doc: Assert that assumed-valid blocks are not fully valid in CheckBlockIndex()":
(https://github.com/bitcoin/bitcoin/pull/29355#issuecomment-1953242432)
Seems reasonable, code review ACK fa027e08f7be63c201f42d0e06160d2273b4a6dd

Not sure why this is marked as a "doc" change though.

💬 ariard commented on pull request "Add imbued v3 based on template-matching":
(https://github.com/bitcoin/bitcoin/pull/29427#issuecomment-1953316512)
If the design goal of this imbuance mechanism is to apply "novel" transaction-relay policy in a backward fashion on pre-signed transactions in the context of multi-party applications and contracting protocols, I think there should be a cryptographic opt-in of one of the transaction issuer itself.

I think the template approach is a dead-end as not only in practice each multi-party applications and contracting protocols have inherent malleability affecting the chain of transaction (amounts, scr
...

👍 theStack approved a pull request: "rpc: Do not wait for headers inside loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/29345#pullrequestreview-1889331879)
Code-review ACK faa30a4c566c5b720c7994c55f276352a119129f

💬 delta1 commented on pull request "doc: Assert that assumed-valid blocks are not fully valid in CheckBlockIndex()":
(https://github.com/bitcoin/bitcoin/pull/29355#issuecomment-1953479997)
@fjahr i think drahtbot added that label because the commit starts with “doc:”

💬 maflcko commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1495330198)
```cpp
const CFeeRate max_raw_tx_fee_rate{ParseFeeRate(self.Arg<UniValue>(1))};
```

See https://github.com/bitcoin/bitcoin/pull/29434#event-11853176044

💬 naumenkogs commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1495427464)
>I believe that's impractical due to the limited size of addrman.

Right, probably fine to keep it this way for both cases.

💬 naumenkogs commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#issuecomment-1953718864)
ACK 7edb07ca800991f7c88d582b880a392efe17f31d

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1953738549)
@josibake I'm trying to rebase, but I'm not sure how to change the following:

```cpp
const auto tweak_data = wallet::GetSilentPaymentTweakDataFromTxInputs(tx->vin, coins);

if (!tweak_data) continue;

const uint256 outpoint_hash = tweak_data->first;
CPubKey tweaked_pub_key_sum = tweak_data->second;
if (!tweaked_pub_key_sum.TweakAdd(outpoint_hash.begin())) {
```

It looks like `GetSilentPaymentTweakDataFromTxInputs` is only used by test code. Am I still supposed to us
...

👍 naumenkogs approved a pull request: "consensus: Store transaction nVersion as uint32_t"
(https://github.com/bitcoin/bitcoin/pull/29325#pullrequestreview-1889798865)
ACK 6acfcbfe2487f683e8f62606d195a9974bc2234f

💬 naumenkogs commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#discussion_r1495440147)
ca7090d43fab2013bd396d683f4cf03062666b14

Does this still look meaningful?
If you want to test a large number, then better check against `TX_MAX_STANDARD_VERSION + 1`?

💬 maflcko commented on pull request "ci: Avoid CI failures from temp env file reuse":
(https://github.com/bitcoin/bitcoin/pull/29441#issuecomment-1953753202)
rfm? :)

💬 naumenkogs commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1953785668)
ACK c10f528350152ca9248e8c167b67993fc7321ca3

💬 Sjors commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1495502646)
Assuming I still need this for the index #28241, I still prefer if this wasn't part of the wallet. Afaik nothing in this file has a dependency on the wallet, so the easiest change would to be move this to `src/common`.

💬 BrandonOdiwuor commented on issue "rfc: store PSBTs in wallet ":
(https://github.com/bitcoin/bitcoin/issues/17619#issuecomment-1953810858)
What's the progress with this?

💬 josibake commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1953888046)
> Am I still supposed to use it

Yep, still the correct function. The return value has changed, and we've removed the `pubkey.TweakAdd` method in favor of a dedicated routine in `module/silentpayments`. I need to expose the routine for pubkey tweaking in `src/bip352.h` and then you should be good.

💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1495568944)
Now that we have `src/bip352.h`, it might make more sense to get rid of `src/wallet/silentpayents.h` and put everything in `src/bip352.h`. In the future, anything that is wallet specific can go in `src/wallet/silentpayments.h` and you'd have everything you need for an index in `src/bip352.h`. Thoughts?

🚀 fanquake merged a pull request: "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets"
(https://github.com/bitcoin/bitcoin/pull/26008)

🚀 fanquake merged a pull request: "ci: Avoid CI failures from temp env file reuse"
(https://github.com/bitcoin/bitcoin/pull/29441)

💬 hebasto commented on issue "When selecting a custom data directory on first launch of Bitcoin Core GUI on Mac, where is that setting stored?":
(https://github.com/bitcoin-core/gui/issues/798#issuecomment-1953943599)
> I believe I've located it:
>
> `~/Library/Preferences/org.bitcoin.Bitcoin-Qt.plist`

That's correct. Also see https://github.com/bitcoin-core/gui/blob/master/doc/files.md#gui-settings.

> I spoke too soon. When I delete that file, Bitcoin Core GUI still knows about the custom location I selected at first launch.

You have to invalidate the corresponding cache: https://stackoverflow.com/questions/19742452/qsettings-on-os-x-10-9-unable-to-locate-clear-it.

📝 m3dwards opened a pull request: "docs: ci multi-arch requires qemu"
(https://github.com/bitcoin/bitcoin/pull/29456)
On a fresh Debian system qemu isn't installed and therefore the multi-architecture CI system doesn't run.

This documentation notes that qemu is required and how to install it.