💬 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

💬 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" />

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

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

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

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

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

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

💬 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).

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

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

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

🚀 fanquake merged a pull request: "test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work"
(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

💬 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

💬 fanquake commented on issue "depends: fallback server missing Qt downloads":
(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

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

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

👍 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`...