✅ hebasto closed a pull request: "Dialog for allowing the user to choose the change output when bumping a tx"
(https://github.com/bitcoin-core/gui/pull/700)
(https://github.com/bitcoin-core/gui/pull/700)
📝 hebasto reopened a pull request: "Dialog for allowing the user to choose the change output when bumping a tx"
(https://github.com/bitcoin-core/gui/pull/700)
Based on https://github.com/bitcoin/bitcoin/pull/26467
Implements a GUI dialog for allowing the user to choose the output to reduce when bumping a transaction. This adds the functionality that was added to the RPC.
(https://github.com/bitcoin-core/gui/pull/700)
Based on https://github.com/bitcoin/bitcoin/pull/26467
Implements a GUI dialog for allowing the user to choose the output to reduce when bumping a transaction. This adds the functionality that was added to the RPC.
💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2539959279)
The last part fails for me intermittently when I run it locally:
```
2025-11-18T23:32:22.558603Z TestFramework (INFO): Trying to send a transaction when none of Tor or I2P is reachable
2025-11-18T23:32:22.559372Z TestFramework.socks5 (ERROR): socks5 request handling failed.
Traceback (most recent call last):
File "XXX/29415_tor_tx/test/functional/test_framework/socks5.py", line 139, in handle
ver = recvall(self.conn, 1)[0]
^^^^^^^^^^^^^^^^^^^^^
File "XXX/29415_tor_tx/t
...
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2539959279)
The last part fails for me intermittently when I run it locally:
```
2025-11-18T23:32:22.558603Z TestFramework (INFO): Trying to send a transaction when none of Tor or I2P is reachable
2025-11-18T23:32:22.559372Z TestFramework.socks5 (ERROR): socks5 request handling failed.
Traceback (most recent call last):
File "XXX/29415_tor_tx/test/functional/test_framework/socks5.py", line 139, in handle
ver = recvall(self.conn, 1)[0]
^^^^^^^^^^^^^^^^^^^^^
File "XXX/29415_tor_tx/t
...
💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2539928309)
I wonder if this logic could be made more modular/generic, with parts of it being in the test framework instead of just this file.
It seems very useful in general to have functionality that will let the node make actual automatic connections with the possibility to redirect them to `P2PInterface()` or `P2PDataStore()` objects depending on the test, so that we don't have to actively call `add_outbound_p2p_connection` but let the node handle it itself.
For example I am currently working on a
...
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2539928309)
I wonder if this logic could be made more modular/generic, with parts of it being in the test framework instead of just this file.
It seems very useful in general to have functionality that will let the node make actual automatic connections with the possibility to redirect them to `P2PInterface()` or `P2PDataStore()` objects depending on the test, so that we don't have to actively call `add_outbound_p2p_connection` but let the node handle it itself.
For example I am currently working on a
...
💬 fjahr commented on pull request "test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#issuecomment-3549949721)
> I think the `AutoFile::size` implementation and the subsequent refactoring would fit better in their own dedicated refactoring PR, or in a PR where they're actually required. They don't seem to be needed by either of the other two commits (the `LogWarning` change in `asmap.cpp` or the newly added test vectors in `netbase_tests.cpp`).
Most of our refactoring commits are not "actually required", strictly speaking. We still do them for a variety of reasons. In this case, I was working on the a
...
(https://github.com/bitcoin/bitcoin/pull/33026#issuecomment-3549949721)
> I think the `AutoFile::size` implementation and the subsequent refactoring would fit better in their own dedicated refactoring PR, or in a PR where they're actually required. They don't seem to be needed by either of the other two commits (the `LogWarning` change in `asmap.cpp` or the newly added test vectors in `netbase_tests.cpp`).
Most of our refactoring commits are not "actually required", strictly speaking. We still do them for a variety of reasons. In this case, I was working on the a
...
💬 hebasto commented on issue "Bitcoin Core-[test] : Text overflows during the sync process.":
(https://github.com/bitcoin-core/gui/issues/847#issuecomment-3549973100)
> The text showing the status of progress during the inital sync process overflows on Bitcoin Core
Does increasing the Bitcoin Core window size help?
> Bitcoin Core version v27.0.0
Did you try the latest [release](https://bitcoincore.org/en/download/)?
(https://github.com/bitcoin-core/gui/issues/847#issuecomment-3549973100)
> The text showing the status of progress during the inital sync process overflows on Bitcoin Core
Does increasing the Bitcoin Core window size help?
> Bitcoin Core version v27.0.0
Did you try the latest [release](https://bitcoincore.org/en/download/)?
💬 fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#issuecomment-3549979018)
> > The first three PRs were already part of #28792, the others are new.
>
> i'm sure you mean the first three _commits_? 😄
Yepp, fixed :)
(https://github.com/bitcoin/bitcoin/pull/33878#issuecomment-3549979018)
> > The first three PRs were already part of #28792, the others are new.
>
> i'm sure you mean the first three _commits_? 😄
Yepp, fixed :)
💬 fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2540013317)
The commit is here because this PR depends on #33026 and the commit is coming from there, so the discussion on these commits should be held there. I just responded there, so marking this as resolved here.
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2540013317)
The commit is here because this PR depends on #33026 and the commit is coming from there, so the discussion on these commits should be held there. I just responded there, so marking this as resolved here.
🤔 w0xlt reviewed a pull request: "kernel: Expose reusable `PrecomputedTransactionData` in script validation"
(https://github.com/bitcoin/bitcoin/pull/33891#pullrequestreview-3480249411)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33891#pullrequestreview-3480249411)
Concept ACK
💬 ajtowns commented on pull request "[wip] wallet: Add separate balance info for non-mempool wallet txs":
(https://github.com/bitcoin/bitcoin/pull/33671#issuecomment-3550142278)
> This extends the `getbalances` RPC. Would be good to also update the GUI (not necessary in this PR).
Okay, I have a patch for that at https://github.com/ajtowns/bitcoin/commits/202511-wallet-unconf-bal-gui/
> The new type of transactions would have somehow to be fed to the wallet so that they would be returned by `wallet.GetTXOs()`, called by `GetBalance()`. How would that be done?
I don't think there's a way to do this for arbitrary txs that originate outside of the wallet, even if t
...
(https://github.com/bitcoin/bitcoin/pull/33671#issuecomment-3550142278)
> This extends the `getbalances` RPC. Would be good to also update the GUI (not necessary in this PR).
Okay, I have a patch for that at https://github.com/ajtowns/bitcoin/commits/202511-wallet-unconf-bal-gui/
> The new type of transactions would have somehow to be fed to the wallet so that they would be returned by `wallet.GetTXOs()`, called by `GetBalance()`. How would that be done?
I don't think there's a way to do this for arbitrary txs that originate outside of the wallet, even if t
...
👋 ajtowns's pull request is ready for review: "[wip] wallet: Add separate balance info for non-mempool wallet txs"
(https://github.com/bitcoin/bitcoin/pull/33671)
(https://github.com/bitcoin/bitcoin/pull/33671)
📝 ajtowns opened a pull request: "Adds non-mempool wallet balance to overview"
(https://github.com/bitcoin-core/gui/pull/911)
The wallet can contain transactions that are not accepted into the node's mempool (eg due to containing a too large OP_RETURN output, due to too low a feerate, or due to too many unconfirmed ancestors). In the event you end up in this situation, it can appear as if funds have gone missing from your wallet due to the non-mempool balance not being reported. Correct this by reporting the non-mempool balance.
Depends on bitcoin/bitcoin#33671
(https://github.com/bitcoin-core/gui/pull/911)
The wallet can contain transactions that are not accepted into the node's mempool (eg due to containing a too large OP_RETURN output, due to too low a feerate, or due to too many unconfirmed ancestors). In the event you end up in this situation, it can appear as if funds have gone missing from your wallet due to the non-mempool balance not being reported. Correct this by reporting the non-mempool balance.
Depends on bitcoin/bitcoin#33671
💬 ajtowns commented on pull request "ArgsManager: support subcommand-specific options":
(https://github.com/bitcoin/bitcoin/pull/28802#issuecomment-3550159408)
> I do think the PR could use a more detailed description.
Should I cut and paste what you wrote below into the PR or commit description? Sorry, I need an ELI5 here :)
(https://github.com/bitcoin/bitcoin/pull/28802#issuecomment-3550159408)
> I do think the PR could use a more detailed description.
Should I cut and paste what you wrote below into the PR or commit description? Sorry, I need an ELI5 here :)
💬 ajtowns commented on issue "Standardness policy rules for legacy Multisig script is incoherent":
(https://github.com/bitcoin/bitcoin/issues/33882#issuecomment-3550406321)
> This checking m ≤ n behaviour in the Solver seems to have been added in [#13194](https://github.com/bitcoin/bitcoin/pull/13194). However this wouldn't count as soft-confiscations because multisigs with m > n are presumably unspendable anways.
Yeah, see https://github.com/bitcoin/bitcoin/blob/4405b78d6059e536c36974088a8ed4d9f0f29898/script.cpp#L745 from the start of git history.
(https://github.com/bitcoin/bitcoin/issues/33882#issuecomment-3550406321)
> This checking m ≤ n behaviour in the Solver seems to have been added in [#13194](https://github.com/bitcoin/bitcoin/pull/13194). However this wouldn't count as soft-confiscations because multisigs with m > n are presumably unspendable anways.
Yeah, see https://github.com/bitcoin/bitcoin/blob/4405b78d6059e536c36974088a8ed4d9f0f29898/script.cpp#L745 from the start of git history.
💬 waketraindev commented on pull request "[30.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3550668156)
Please add https://github.com/bitcoin-core/gui/pull/910 , it's test coverage for https://github.com/bitcoin-core/gui/pull/901
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3550668156)
Please add https://github.com/bitcoin-core/gui/pull/910 , it's test coverage for https://github.com/bitcoin-core/gui/pull/901
💬 waketraindev commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3550854849)
ACK 8558902e576e2c2d66f6083b66953dd6cc464de4
Tested on Windows 11 (Dark Theme) (wsl/depends cross build) x86_64-w64-mingw32 [Release config]
Tested on Windows 11 (Dark Theme) (guix build) x86_64-w64-mingw32
<img width="781" height="161" alt="image" src="https://github.com/user-attachments/assets/9400b604-86a5-4635-9fa5-07e21b80e32e" />
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3550854849)
ACK 8558902e576e2c2d66f6083b66953dd6cc464de4
Tested on Windows 11 (Dark Theme) (wsl/depends cross build) x86_64-w64-mingw32 [Release config]
Tested on Windows 11 (Dark Theme) (guix build) x86_64-w64-mingw32
<img width="781" height="161" alt="image" src="https://github.com/user-attachments/assets/9400b604-86a5-4635-9fa5-07e21b80e32e" />
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2540645348)
Yes. Maybe some stuff from here can be moved to `Socks5Configuration` and/or `Socks5Server`. Maybe the SOCKS5 server can have a default "destinations factory" function which does what is in the `else` branch here. Then there could be callback which either tells where to redirect the Nth connection or tells to use the default
```
default destinations factory:
# let i be the number of the connection
if override callback is set:
whereto = override(i)
if whereto is no
...
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2540645348)
Yes. Maybe some stuff from here can be moved to `Socks5Configuration` and/or `Socks5Server`. Maybe the SOCKS5 server can have a default "destinations factory" function which does what is in the `else` branch here. Then there could be callback which either tells where to redirect the Nth connection or tells to use the default
```
default destinations factory:
# let i be the number of the connection
if override callback is set:
whereto = override(i)
if whereto is no
...
💬 ajtowns commented on pull request "Adds non-mempool wallet balance to overview":
(https://github.com/bitcoin-core/gui/pull/911#issuecomment-3551114643)
To test, start with `-datacarriersize=10`, and run
```
send '{"data": "4368616e63656c6c6f72206f6e206272696e6b206f66207365636f6e64206261696c6f757420666f722062616e6b73"}' null "unset" 1
```
from the console. "Non-mempool: <change amount>" should appear in the Balances pane on the Overview window with the change from the transaction. Restarting with `-datacarriersize=100000` should allow the tx to enter the mempool, and move the amount into the Available balance. (Presumably, do this on -re
...
(https://github.com/bitcoin-core/gui/pull/911#issuecomment-3551114643)
To test, start with `-datacarriersize=10`, and run
```
send '{"data": "4368616e63656c6c6f72206f6e206272696e6b206f66207365636f6e64206261696c6f757420666f722062616e6b73"}' null "unset" 1
```
from the console. "Non-mempool: <change amount>" should appear in the Balances pane on the Overview window with the change from the transaction. Restarting with `-datacarriersize=100000` should allow the tx to enter the mempool, and move the amount into the Available balance. (Presumably, do this on -re
...
💬 hebasto commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3551271373)
My Guix build:
```
aarch64
15ff16a4958d217927fb68c99c0dcd19bf51595517c59aae8b442b3e1b4529e2 guix-build-8558902e576e/output/aarch64-linux-gnu/SHA256SUMS.part
db8080232c5216f0f9cb06ce7052fe81156f9dab6a15964be187f1186f24e47e guix-build-8558902e576e/output/aarch64-linux-gnu/bitcoin-8558902e576e-aarch64-linux-gnu-debug.tar.gz
e3c053d21dcab9234a1943af0e61a2c7c9270429238e0eb11fb0378378bcf87e guix-build-8558902e576e/output/aarch64-linux-gnu/bitcoin-8558902e576e-aarch64-linux-gnu.tar.gz
1194b7e6
...
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3551271373)
My Guix build:
```
aarch64
15ff16a4958d217927fb68c99c0dcd19bf51595517c59aae8b442b3e1b4529e2 guix-build-8558902e576e/output/aarch64-linux-gnu/SHA256SUMS.part
db8080232c5216f0f9cb06ce7052fe81156f9dab6a15964be187f1186f24e47e guix-build-8558902e576e/output/aarch64-linux-gnu/bitcoin-8558902e576e-aarch64-linux-gnu-debug.tar.gz
e3c053d21dcab9234a1943af0e61a2c7c9270429238e0eb11fb0378378bcf87e guix-build-8558902e576e/output/aarch64-linux-gnu/bitcoin-8558902e576e-aarch64-linux-gnu.tar.gz
1194b7e6
...
💬 vasild commented on issue "Load PSBT error: Unable to decode PSBT":
(https://github.com/bitcoin/bitcoin/issues/30070#issuecomment-3551296046)
`prepareTransaction()` is called before "Create Unsigned" is clicked. It seems a bit confusing, there are two "Send" buttons.
The user selects inputs, fills destination address and amount and clicks "Send". This is when `SendCoinsDialog::sendButtonClicked()` is called. Then it calls `SendCoinsDialog::PrepareSendText() -> WalletModel::prepareTransaction()`. At this point the transaction is created and signed. I think it must be in order to calculate the fee exactly? Then a dialog is presented "D
...
(https://github.com/bitcoin/bitcoin/issues/30070#issuecomment-3551296046)
`prepareTransaction()` is called before "Create Unsigned" is clicked. It seems a bit confusing, there are two "Send" buttons.
The user selects inputs, fills destination address and amount and clicks "Send". This is when `SendCoinsDialog::sendButtonClicked()` is called. Then it calls `SendCoinsDialog::PrepareSendText() -> WalletModel::prepareTransaction()`. At this point the transaction is created and signed. I think it must be in order to calculate the fee exactly? Then a dialog is presented "D
...