💬 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?

💬 instagibbs commented on issue "Proposal for a new mempool design":
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1554708843)
Either sounds good to me. N direct conflicts or N clusters has the same result: Someone trying to avoid "rule#5" type pinning will only try to CPFP up to N transactions, I believe.

💬 kallerosenbaum commented on issue "Sign PSBT: Can't verify change output":
(https://github.com/bitcoin-core/gui/issues/732#issuecomment-1554719677)
I'll explain better what I do to trigger this, to make sure we're talking about the same issue.

1. Create a new wallet A on regtest, on an offline machine.
2. `listdescriptors` on A and save the result
3. Create a new blank wallet B on another machine
4. `importdescriptors` on B using the result from step 2
5. Get a new address from B and send coins to that address from another wallet
6. In the GUI of B make a transaction to an external address, and make sure to have a change output
7.
...

🤔 glozow reviewed a pull request: "rpc: Add importmempool RPC"
(https://github.com/bitcoin/bitcoin/pull/27460#pullrequestreview-1434610510)
ACK fa83be1c30d5b181c024a40bfb9d5ea15ce64aec

💬 furszy commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1199080037)
> Do you think the the main benefit of this commit is cleaner code, and the worse error detection a side effect? Or are there other benefits to this commit?

Aside from the cleaner code (which I think that it's salable on its own), it also have maintainability and test coverage benefits with the global flag removal. Also, the indexes threads active-wait isn't that good in terms of processors context switching (the index threads are waking up every 0.5s and the reindex process could take a whil
...

💬 kallerosenbaum commented on issue "Sign PSBT: Can't verify change output":
(https://github.com/bitcoin-core/gui/issues/732#issuecomment-1554739241)
I don't consider this as a mere inconvenience, but as a security issue. I can't sign this transaction because I don't know which of the outputs, if any, belongs to me. If people do sign these transactions, they're going to be vulnerable to scammers.

If the fix is hard to do, I think the "sign PSBT" feature should be removed from the GUI, as a security measure, until the issue is fixed.

🤔 glozow reviewed a pull request: "Fee estimation: avoid serving stale fee estimate"
(https://github.com/bitcoin/bitcoin/pull/27622#pullrequestreview-1434616046)
Approach ACK

💬 glozow commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1199085331)
What is the use case for being able to configure the maximum file age?

💬 glozow commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1199078486)
in accf995cf095d7ff3d10c0403e70bdbc1273e852

Seems like the error message here should be something like "there is no fee estimates file" instead of "failed to read" ?

💬 furszy commented on issue "Sign PSBT: Can't verify change output":
(https://github.com/bitcoin-core/gui/issues/732#issuecomment-1554758441)
My note was for other devs. Based on your comments, you want to get a visual distinction on addresses owned by the signer wallet in the dialog. Which is doable and not hard to do.
My comment was just referring to the `IsMine(addr)` usage instead of the `IsChange(addr)`. Which fulfills the required purposes in the same way (and also accepts external addresses which is more accurate).

💬 kallerosenbaum commented on issue "Sign PSBT: Can't verify change output":
(https://github.com/bitcoin-core/gui/issues/732#issuecomment-1554767293)
Got it, thanks!

💬 hebasto commented on pull request "test: Explicitly specify directory where to search tests for":
(https://github.com/bitcoin/bitcoin/pull/27561#issuecomment-1554771916)
> Install the package in editable mode:

The drawback of such an approach is that it creates additional files in the source tree.

💬 hebasto commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1199115073)
> Would suggest `ValidationNotifications& notifications` or `ValdiataionNotifications* notifcations{nullptr}` here,

I'd prefer the pointer type to do not bother about lifetime of variables that were used for assigning.