π¬ 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.
(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
(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)
(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).
(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
...
(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.
(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
...
(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.
(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
```
(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
```
(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 :)
(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.
(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
...
(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.
(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)
(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
...
(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?
(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.
(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.
...
(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
(https://github.com/bitcoin/bitcoin/pull/27460#pullrequestreview-1434610510)
ACK fa83be1c30d5b181c024a40bfb9d5ea15ce64aec