π¬ BinaryIgor commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2334298532)
As this already is a rather too long test, it would be nice to move these assertions into a separate method, similarly as it done with check_tx_counts
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2334298532)
As this already is a rather too long test, it would be nice to move these assertions into a separate method, similarly as it done with check_tx_counts
β οΈ yancyribbens opened an issue: "CoinGrinder and SRD disagree on small change"
(https://github.com/bitcoin/bitcoin/issues/33352)
Both CoinGrinder and SRD create a change output, however they disagree on what change output is too small.
Consider SRD selection, which adds `CHANGE_LOWER` to the target value and in so doing, prevents a selection that is less than `CHANGE_LOWER` [ref](https://github.com/bitcoin/bitcoin/blob/c0894a0a2be032cd9a5d5945643689230ab10255/src/wallet/coinselection.cpp#L546C5-L546C47)
```
target_value += CHANGE_LOWER + change_fee;
```
However, CoinGrinder does not add `CHANGE_LOWER` [ref](https://gith
...
(https://github.com/bitcoin/bitcoin/issues/33352)
Both CoinGrinder and SRD create a change output, however they disagree on what change output is too small.
Consider SRD selection, which adds `CHANGE_LOWER` to the target value and in so doing, prevents a selection that is less than `CHANGE_LOWER` [ref](https://github.com/bitcoin/bitcoin/blob/c0894a0a2be032cd9a5d5945643689230ab10255/src/wallet/coinselection.cpp#L546C5-L546C47)
```
target_value += CHANGE_LOWER + change_fee;
```
However, CoinGrinder does not add `CHANGE_LOWER` [ref](https://gith
...
π¬ yancyribbens commented on issue "CoinGrinder and SRD disagree on small change":
(https://github.com/bitcoin/bitcoin/issues/33352#issuecomment-3271758012)
cc @murchandamus
(https://github.com/bitcoin/bitcoin/issues/33352#issuecomment-3271758012)
cc @murchandamus
π€ w0xlt reviewed a pull request: "test/refactor: use test deque to avoid quadratic iteration"
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3202749415)
ACK https://github.com/bitcoin/bitcoin/pull/33313/commits/8c1f10233dc9a843147772c40973031f5bfdbb7c
While the impact of this PR on a few hundred items is negligible, itβs still a good practice and is likely to scale more effectively as test coverage increases.
For reference, the script below demonstrates the performance difference with 1,000,000 items on my machine:
```
List pop(0): 62.790 seconds
Deque popleft(): 0.022 seconds
```
```python
import time
from collections import d
...
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3202749415)
ACK https://github.com/bitcoin/bitcoin/pull/33313/commits/8c1f10233dc9a843147772c40973031f5bfdbb7c
While the impact of this PR on a few hundred items is negligible, itβs still a good practice and is likely to scale more effectively as test coverage increases.
For reference, the script below demonstrates the performance difference with 1,000,000 items on my machine:
```
List pop(0): 62.790 seconds
Deque popleft(): 0.022 seconds
```
```python
import time
from collections import d
...
π¬ murchandamus commented on issue "CoinGrinder and SRD disagree on small change":
(https://github.com/bitcoin/bitcoin/issues/33352#issuecomment-3271804098)
This is intentional.
Knapsack and CoinGrinder use [`GenerateChangeTarget`](https://github.com/bitcoin/bitcoin/blob/c0894a0a2be032cd9a5d5945643689230ab10255/src/wallet/coinselection.cpp#L809) to make a [randomized minimum change](https://github.com/bitcoin/bitcoin/pull/24494), because these two algorithms minimize the overshoot over the minimum change which led to a fingerprint and a clustering of change outputs of the same value. SRD randomly selects inputs, so it already overshoots the minimum
...
(https://github.com/bitcoin/bitcoin/issues/33352#issuecomment-3271804098)
This is intentional.
Knapsack and CoinGrinder use [`GenerateChangeTarget`](https://github.com/bitcoin/bitcoin/blob/c0894a0a2be032cd9a5d5945643689230ab10255/src/wallet/coinselection.cpp#L809) to make a [randomized minimum change](https://github.com/bitcoin/bitcoin/pull/24494), because these two algorithms minimize the overshoot over the minimum change which led to a fingerprint and a clustering of change outputs of the same value. SRD randomly selects inputs, so it already overshoots the minimum
...
π¬ davidgumberg commented on pull request "Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3271811950)
> The patch indeed fixes the pathological memory usage. However, it modifies the clipboard content. The copy-pasted prompt appears as follow:
>
> [...]
Thanks for catching this, I should have checked that clipboard contents actually were reasonable with my change, my mistake.
I've updated the branch to fix both the newline issue, and the issues that existed with copying icon's and some of the other rich text stuff in the console. I use the same [function](https://github.com/qt/qtbase/bl
...
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3271811950)
> The patch indeed fixes the pathological memory usage. However, it modifies the clipboard content. The copy-pasted prompt appears as follow:
>
> [...]
Thanks for catching this, I should have checked that clipboard contents actually were reasonable with my change, my mistake.
I've updated the branch to fix both the newline issue, and the issues that existed with copying icon's and some of the other rich text stuff in the console. I use the same [function](https://github.com/qt/qtbase/bl
...
β
achow101 closed an issue: "CoinGrinder and SRD disagree on small change"
(https://github.com/bitcoin/bitcoin/issues/33352)
(https://github.com/bitcoin/bitcoin/issues/33352)
π¬ Roasbeef commented on issue "Difficulty in reliably mapping errors from Bitcoin Core due to unstable error codes and messages":
(https://github.com/bitcoin/bitcoin/issues/33350#issuecomment-3271824803)
> proactively avoid violating it
Avoiding violating the rules is itself a moving target. Consider that a user can set config options in `bitciond` to: affect the min relay fee, reduce the mempool size, etc (min mempool fee is itself a dynamic value). Or the user picks a fee that's below the min relay fee, and the transaction is rejected. Another example is if someone sends you some sats, and you go to spend that transaction zero conf, but get rejected as the ancestor chain is too long. Or if ne
...
(https://github.com/bitcoin/bitcoin/issues/33350#issuecomment-3271824803)
> proactively avoid violating it
Avoiding violating the rules is itself a moving target. Consider that a user can set config options in `bitciond` to: affect the min relay fee, reduce the mempool size, etc (min mempool fee is itself a dynamic value). Or the user picks a fee that's below the min relay fee, and the transaction is rejected. Another example is if someone sends you some sats, and you go to spend that transaction zero conf, but get rejected as the ancestor chain is too long. Or if ne
...
π¬ instagibbs commented on issue "Difficulty in reliably mapping errors from Bitcoin Core due to unstable error codes and messages":
(https://github.com/bitcoin/bitcoin/issues/33350#issuecomment-3271843015)
> RPC calls like submitpackage (or testmempoolaccept) help, but as there isn't an atomic transaction submission API, something can show success with submitpacakge, but then rejected once it actually hits the mempool.
Sorry can you elaborate? `submitpackage` *should* return an error string if it doesn't enter the local mempool
(https://github.com/bitcoin/bitcoin/issues/33350#issuecomment-3271843015)
> RPC calls like submitpackage (or testmempoolaccept) help, but as there isn't an atomic transaction submission API, something can show success with submitpacakge, but then rejected once it actually hits the mempool.
Sorry can you elaborate? `submitpackage` *should* return an error string if it doesn't enter the local mempool
π¬ davidgumberg commented on issue "GUI (?): Copying output from console causes large mem usage/OOM":
(https://github.com/bitcoin-core/gui/issues/887#issuecomment-3271861101)
I experience this issue linking against system Qt 6.9.1, so I do not think this has been fixed upstream.
But in our `depends` build we set `-no-feature-textmarkdownwriter`:
https://github.com/bitcoin-core/gui/blob/689a32197638e92995dd8eb071425715f5fdc3a4/depends/packages/qt.mk#L103
and when that feature is disabled, `QTextEditMimeData` won't try to convert the selection to markdown, avoiding the problematic function:
https://github.com/qt/qtbase/blob/b617d1176593963a2a9ed21dd5d9a63e84a09400/
...
(https://github.com/bitcoin-core/gui/issues/887#issuecomment-3271861101)
I experience this issue linking against system Qt 6.9.1, so I do not think this has been fixed upstream.
But in our `depends` build we set `-no-feature-textmarkdownwriter`:
https://github.com/bitcoin-core/gui/blob/689a32197638e92995dd8eb071425715f5fdc3a4/depends/packages/qt.mk#L103
and when that feature is disabled, `QTextEditMimeData` won't try to convert the selection to markdown, avoiding the problematic function:
https://github.com/qt/qtbase/blob/b617d1176593963a2a9ed21dd5d9a63e84a09400/
...
π¬ w0xlt commented on pull request "Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3271874773)
I wasnβt able to detect any memory increase on macOS when running getblocktemplate '{"rules": ["segwit"]}' in the GUI.
Is this issue specific to Linux?
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3271874773)
I wasnβt able to detect any memory increase on macOS when running getblocktemplate '{"rules": ["segwit"]}' in the GUI.
Is this issue specific to Linux?
π¬ davidgumberg commented on pull request "Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3271898148)
> I wasnβt able to detect any memory increase on macOS when running getblocktemplate '{"rules": ["segwit"]}' in the GUI. Is this issue specific to Linux?
Are you running from a release or a depends build? This will only occur when linking against a QT with the `textmarkdownwriter` feature enabled, which we disable in our depends build of QT. See discussion here: https://github.com/bitcoin-core/gui/issues/887#issuecomment-3271861101
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3271898148)
> I wasnβt able to detect any memory increase on macOS when running getblocktemplate '{"rules": ["segwit"]}' in the GUI. Is this issue specific to Linux?
Are you running from a release or a depends build? This will only occur when linking against a QT with the `textmarkdownwriter` feature enabled, which we disable in our depends build of QT. See discussion here: https://github.com/bitcoin-core/gui/issues/887#issuecomment-3271861101
π¬ achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334523023)
Used now for `has_internal`.
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334523023)
Used now for `has_internal`.
π¬ achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334523216)
Done
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334523216)
Done
π l0rinc opened a pull request: "logs: show reindex progress in `ImportBlocks`"
(https://github.com/bitcoin/bitcoin/pull/33353)
### Summary
When triggering a reindex, users had no indication of how many files remained or how far along the process was.
### Fix
This patch prefetches the target file block file count to be able to show progress information. Instead of just displaying which block file is being processed, it now indicates how many files remain.
### Reproducer + expected results
Running
```bash
cmake -B build && make -C build -DCMAKE_BUILD_TYPE=Release && ./build/bin/bitcoind -datadir=demo -rei
...
(https://github.com/bitcoin/bitcoin/pull/33353)
### Summary
When triggering a reindex, users had no indication of how many files remained or how far along the process was.
### Fix
This patch prefetches the target file block file count to be able to show progress information. Instead of just displaying which block file is being processed, it now indicates how many files remain.
### Reproducer + expected results
Running
```bash
cmake -B build && make -C build -DCMAKE_BUILD_TYPE=Release && ./build/bin/bitcoind -datadir=demo -rei
...
π¬ achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334524197)
I think it is useful to to exercise the change address of `send`.
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334524197)
I think it is useful to to exercise the change address of `send`.
π¬ achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334524489)
Done
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334524489)
Done
π¬ achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334525262)
Fixing that is orthogonal to this PR
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334525262)
Fixing that is orthogonal to this PR
π¬ achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334526025)
Updated the name and changed this to use a regex that matches only on a 2 index multipath.
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334526025)
Updated the name and changed this to use a regex that matches only on a 2 index multipath.
π¬ achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334526200)
Done
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2334526200)
Done