🤔 josibake reviewed a pull request: "25.0 Final Changes"
(https://github.com/bitcoin/bitcoin/pull/27686#pullrequestreview-1434354654)
ACK https://github.com/bitcoin/bitcoin/pull/27686/commits/9ded442dffb827d8762c0becce9d10fa283d1a3a

looks good, although we probably don't need to credit `merge-script` as a contributor

💬 josibake commented on pull request "25.0 Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27686#discussion_r1198915559)
lol credit where credit is due, i guess?

💬 MarcoFalke commented on pull request "test: Explicitly specify directory where to search tests for":
(https://github.com/bitcoin/bitcoin/pull/27561#issuecomment-1554519947)
Anyway, should be unrelated to this pull request.

💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1554521123)
Ah yes, broken my own #26076.

💬 josibake commented on pull request "25.0 Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27686#issuecomment-1554528629)
reACK https://github.com/bitcoin/bitcoin/pull/27686/commits/6675cf809543866792bf6706372fc9ac253d3e99

📝 josibake opened a pull request: "doc: filter out merge-script from list of authors"
(https://github.com/bitcoin/bitcoin/pull/27703)

💬 furszy commented on issue "Sign PSBT: Can't verify change output":
(https://github.com/bitcoin-core/gui/issues/732#issuecomment-1554580472)
Small note about this.
This would work if the change address was previously recorded by the wallet (used in a previous tx). If you just obtained it by a `getrawchangeaddress` RPC call, the wallet will currently not detect it as change. Only will detect it as yours (reasons are explained in #25979).

So, would say to present it as "own address" in the dialog instead of checking whether is change or not (external addresses should be presented as yours too).

💬 stickies-v commented on pull request "test: Explicitly specify directory where to search tests for":
(https://github.com/bitcoin/bitcoin/pull/27561#issuecomment-1554587245)
> Sure. I've updated the PR description with steps to replicate the failure.

Thanks, this helped a lot with my understanding.

I'm not a huge fan of the build system leaking into the test code, if we can avoid it. Someone reading this without knowledge of the build system wouldn't understand why we're appending to `sys.path`, so at the very least I'd suggest adding some documentation and/or a link to this PR.

An alternative approach could be to add a `bitcoin/test/functional/setup.py` fi
...

💬 furszy commented on pull request "[WIP] wallet: standardize change output detection process":
(https://github.com/bitcoin/bitcoin/pull/25979#issuecomment-1554595205)
Thanks for the ping marko, rebased.
Yeah, this is still relevant. Change detection is still an issue.

The base work is done, just need some extra love.
Will kept it rebased, so anyone can review/test it up until I'm able to get back to it.

📝 furszy converted_to_draft a pull request: "[WIP] wallet: standardize change output detection process"
(https://github.com/bitcoin/bitcoin/pull/25979)
This work aims to define, and implement a base standard mechanism to
detect individual change outputs.

### Context

Currently, the wallet detects whether an output is change or not based
on data stored in the address book.

There is no notion of “change outputs”, the wallet detects change scripts.

Connoting that any address book record modification has implications
on all the historical outputs related to that particular destination. Meaning
that all those outputs can either be cha
...

💬 furszy commented on pull request "[WIP] wallet: standardize change output detection process":
(https://github.com/bitcoin/bitcoin/pull/25979#issuecomment-1554609029)
This now depends on #27601.
Moved to draft until that one gets merged.

💬 fanquake commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#discussion_r1199008155)
```suggestion
$(package)_sha256_hash=18df38247af3fba0e0e2991fb00d7e3cf3560b4d3509233a14af699ef0039e1c
```

💬 vostrnad commented on pull request "25.0 Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27686#discussion_r1199008977)
```suggestion
signed by a _threshold of trusted keys_. For more details and
```

💬 ajtowns commented on pull request "p2p: Stop relaying non-mempool txs":
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1554651019)
> Thanks. Did you want me to push it here (removing the logging), or did you want to open a fresh pull yourself?

Go ahead and clean it up and push it here :)

💬 furszy commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1199013812)
Sure, will change it. I used indexers to try to differentiate them from block indexes but guess that its an odd term in English.

💬 pinheadmz commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1199014871)
Ok good point I see, well maybe just then...


```diff
diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
index 085f938f80..adb0ebc383 100644
--- a/src/policy/fees.cpp
+++ b/src/policy/fees.cpp
@@ -925,7 +925,7 @@ void CBlockPolicyEstimator::FlushFeeEstimates() {
if (est_file.IsNull() || !Write(est_file)) {
LogPrintf("Failed to write fee estimates to %s. Continue anyway.\n", fs::PathToString(m_estimation_filepath));
} else {
- LogPrintf("Flushed fee esti
...

💬 fanquake commented on pull request "25.0 Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27686#discussion_r1199018113)
Thanks. Fixed.

🚀 glozow merged a pull request: "Implement Mini version of BlockAssembler to calculate mining scores"
(https://github.com/bitcoin/bitcoin/pull/27021)

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1554675156)
> As I already have a follow-up PR with https://github.com/bitcoin/bitcoin/pull/26152, since this PR has three ACKs now, and all the open comments are nits, I would like to include those changes as a new commit in the follow-up https://github.com/bitcoin/bitcoin/pull/26152.

Yep, I'll look for these in #26152:
https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1188846506
https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1188865405
https://github.com/bitcoin/bitcoin/pull/27021
...

💬 hebasto commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1199037376)
Why using a reference type here is safe?