💬 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
...
💬 maflcko commented on pull request "test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#issuecomment-3551316361)
review ACK 7f318e1dd0496384e7bc6d8754c5a2a618a14b2a 🏀
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 7f318e1dd049
...
(https://github.com/bitcoin/bitcoin/pull/33026#issuecomment-3551316361)
review ACK 7f318e1dd0496384e7bc6d8754c5a2a618a14b2a 🏀
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 7f318e1dd049
...
💬 maflcko commented on pull request "rpc: Add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#issuecomment-3551325875)
the commit message is wrong?
(https://github.com/bitcoin/bitcoin/pull/32741#issuecomment-3551325875)
the commit message is wrong?
💬 hodlinator commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3551348815)
Latest push rebased to resolve conflict with #31734. + Added explicit test coverage of `operator=(Node&&)` (https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3490078075).
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3551348815)
Latest push rebased to resolve conflict with #31734. + Added explicit test coverage of `operator=(Node&&)` (https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3490078075).
💬 Sjors commented on issue "rfc: virtio-vsock for RPC and IPC":
(https://github.com/bitcoin/bitcoin/issues/33897#issuecomment-3551516882)
> it would not really be safe without authentication
And encryption, since we can't use SSL.
(https://github.com/bitcoin/bitcoin/issues/33897#issuecomment-3551516882)
> it would not really be safe without authentication
And encryption, since we can't use SSL.
💬 Sjors commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3551580243)
@plebhash indeed I'm not worried about client-side memory, it's node that could get into memory trouble if there's a long block interval with lots of mempool churn.
> it's extremely unlikely that mempool would sustain sub-second changes for over 2h
In the context of templates we only care about about at the top of the mempool. For the mempool as a whole there can be dozens of transactions per second, see e.g. #28592.
@ryanofsky how would the node go about tracking the memory footprint of clie
...
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3551580243)
@plebhash indeed I'm not worried about client-side memory, it's node that could get into memory trouble if there's a long block interval with lots of mempool churn.
> it's extremely unlikely that mempool would sustain sub-second changes for over 2h
In the context of templates we only care about about at the top of the mempool. For the mempool as a whole there can be dozens of transactions per second, see e.g. #28592.
@ryanofsky how would the node go about tracking the memory footprint of clie
...
💬 Sjors commented on pull request "mining: add requestedOutputs field, e.g. for merged mining":
(https://github.com/bitcoin/bitcoin/pull/33890#issuecomment-3551630141)
> is this aimed at Sv2?
Any consumer of the interface, but I mostly have Stratum v2 in mind.
> if it requires a spec change so this can be integrated
As @ajtowns points out in the https://github.com/stratum-mining/sv2-spec/issues/167 thread it's a bit odd that @TheBlueMatt states that merged-mining is "a key goal of Sv2" while the spec doesn't even mention it.
In principle the change here doesn't need a spec change. Miners who want merged-mining will patch _something_, and this just
...
(https://github.com/bitcoin/bitcoin/pull/33890#issuecomment-3551630141)
> is this aimed at Sv2?
Any consumer of the interface, but I mostly have Stratum v2 in mind.
> if it requires a spec change so this can be integrated
As @ajtowns points out in the https://github.com/stratum-mining/sv2-spec/issues/167 thread it's a bit odd that @TheBlueMatt states that merged-mining is "a key goal of Sv2" while the spec doesn't even mention it.
In principle the change here doesn't need a spec change. Miners who want merged-mining will patch _something_, and this just
...
🚀 fanquake merged a pull request: "test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work"
(https://github.com/bitcoin/bitcoin/pull/33026)
(https://github.com/bitcoin/bitcoin/pull/33026)
💬 fanquake commented on pull request "ci: Consistenly only cache on the default branch":
(https://github.com/bitcoin/bitcoin/pull/33905#issuecomment-3551729334)
cc @willcl-ark @m3dwards
(https://github.com/bitcoin/bitcoin/pull/33905#issuecomment-3551729334)
cc @willcl-ark @m3dwards
💬 fanquake commented on pull request "ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG":
(https://github.com/bitcoin/bitcoin/pull/33888#issuecomment-3551737960)
cc @m3dwards @willcl-ark
(https://github.com/bitcoin/bitcoin/pull/33888#issuecomment-3551737960)
cc @m3dwards @willcl-ark
💬 fanquake commented on issue "depends: fallback server missing Qt downloads":
(https://github.com/bitcoin/bitcoin/issues/33898#issuecomment-3551739190)
cc @hebasto
(https://github.com/bitcoin/bitcoin/issues/33898#issuecomment-3551739190)
cc @hebasto
🤔 maflcko reviewed a pull request: "test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work"
(https://github.com/bitcoin/bitcoin/pull/33026#pullrequestreview-3481321005)
forgot to send the nits, basically just a test nit to check the returned size, but just a nit
(https://github.com/bitcoin/bitcoin/pull/33026#pullrequestreview-3481321005)
forgot to send the nits, basically just a test nit to check the returned size, but just a nit
💬 maflcko commented on pull request "test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2540945544)
Maybe just de-duplicate with `FromBytes` in the other test file?
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2540945544)
Maybe just de-duplicate with `FromBytes` in the other test file?
💬 maflcko commented on pull request "test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2540940425)
nit: Here and above in the last commit: Why not check the size is equal to 10 each time?
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2540940425)
nit: Here and above in the last commit: Why not check the size is equal to 10 each time?
👍 willcl-ark approved a pull request: "ci: Consistenly only cache on the default branch"
(https://github.com/bitcoin/bitcoin/pull/33905#pullrequestreview-3481757071)
ACK fa411f938e4796cf3806f234c9787b7c79492cc2
We could probably drop `github.event_name != 'pull_request' &&`? Feels like it might be redundant with `github.ref_name == github.event.repository.default_branch`...
(https://github.com/bitcoin/bitcoin/pull/33905#pullrequestreview-3481757071)
ACK fa411f938e4796cf3806f234c9787b7c79492cc2
We could probably drop `github.event_name != 'pull_request' &&`? Feels like it might be redundant with `github.ref_name == github.event.repository.default_branch`...
📝 hodlinator opened a pull request: "ci: Make the max number of commits tested explicit"
(https://github.com/bitcoin/bitcoin/pull/33909)
Gives less of a false sense of security.
(https://github.com/bitcoin/bitcoin/pull/33909)
Gives less of a false sense of security.
💬 maflcko commented on pull request "ci: Consistenly only cache on the default branch":
(https://github.com/bitcoin/bitcoin/pull/33905#issuecomment-3551842381)
Yeah, I guess even if a pull is opened from a branched named equally to the default branch, the ref_name is rewritten to `<pr_number>/merge`, according to https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#github-context
However, it can't hurt to check twice explicitly, so I'll leave as-is for now :sweat_smile:
(https://github.com/bitcoin/bitcoin/pull/33905#issuecomment-3551842381)
Yeah, I guess even if a pull is opened from a branched named equally to the default branch, the ref_name is rewritten to `<pr_number>/merge`, according to https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#github-context
However, it can't hurt to check twice explicitly, so I'll leave as-is for now :sweat_smile:
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2541346873)
Hmm, looks like this happens because there are still connections between the bitcoin node and the SOCKS5 proxy and then the bitcoin node is restarted, closing the connections. Or rather, the bitcoin node is just establishing a new connection and is interrupted in the middle of the SOCKS5 handshake because of the `self.restart_node()` call.
This is expected and the test succeeds rightfully, however that printout looks odd and scary. Should be handled more gracefully. One way would be to conver
...
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2541346873)
Hmm, looks like this happens because there are still connections between the bitcoin node and the SOCKS5 proxy and then the bitcoin node is restarted, closing the connections. Or rather, the bitcoin node is just establishing a new connection and is interrupted in the middle of the SOCKS5 handshake because of the `self.restart_node()` call.
This is expected and the test succeeds rightfully, however that printout looks odd and scary. Should be handled more gracefully. One way would be to conver
...