💬 pablomartin4btc commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#discussion_r1569924233)
Thanks for reviewing and spotting that. I tried on Ubuntu to call **only** `dlg->raise()` but doesn't seem to work (also try other things like `dlg->setFocus()`, `dlg->setFocusPolicy(Qt::StrongFocus)`, `dlg->focusWidget()` and some combinations), until I found this in the QT documentation about [activateWindow()](https://doc.qt.io/qt-5/qwidget.html#activateWindow) (which I should have checked first):

_This function performs the same operation as clicking the mouse on the title bar of a top-le
...

💬 stratospher commented on pull request "test: Fix intermittent issue in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/pull/29898#discussion_r1569994243)
makes sense, less diff too. but `peer_id` is bounded to [0, 11]/`p2p_idx = peer_id + 1` is bounded to [1, 12] [here](https://github.com/bitcoin/bitcoin/blob/dbd2000b34903b87ae2e02eb2fc6c4a2f7d11451/test/functional/test_framework/p2p.py#L745). so we'd cross that limit if we do global counters.

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-2063026719)
> Are you still working on this?

Sorry I got a little occupied with some other things and would want to continue working on this. I've the approach in mind and will force push the update by this end of this month.

- Will create a new file for `CoinsResult ` and remove global Singleton from the `Spend` file.

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2063029281)
Can someone tell me why is the `previous releases, qt5 dev package and depends packages, DEBUG` test is failing?

I've ran this on my machine and it works fine.

💬 maflcko commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2063032378)
It is using an old CI config. You can rebase on current master to get a fresh CI run.

💬 maflcko commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1570017035)
Could also check if source_location fixes the annoyance on Windows to print the full path when logging:

```
2024-04-15T13:27:36.5198997Z node0 2024-04-15T13:27:36.001614Z [D:\a\bitcoin\bitcoin\src\validation.cpp:2549] [ConnectBlock] [bench] - Verify 0 txins: 0.06ms (0.000ms/txin) [0.00s (0.03ms/blk)]

💬 maflcko commented on issue "Intermittent issue in p2p_unrequested_blocks.py", line 98, in run_test test_node.send_and_ping(msg_block(blocks_h2[0])) assert self.is_connected AssertionError / AcceptBlock FAILED (time-too-old, block's timestamp is too early) ":
(https://github.com/bitcoin/bitcoin/issues/29897#issuecomment-2063072877)
This is the known bug on Windows Python, where the `time.time()` returned is just wrong. See https://github.com/bitcoin/bitcoin/issues/25482#issuecomment-1220756898

💬 maflcko commented on issue "Intermittent issue in p2p_unrequested_blocks.py", line 98, in run_test test_node.send_and_ping(msg_block(blocks_h2[0])) assert self.is_connected AssertionError / AcceptBlock FAILED (time-too-old, block's timestamp is too early) ":
(https://github.com/bitcoin/bitcoin/issues/29897#issuecomment-2063076877)
<details><summary>CI log excerpt</summary>

```
2024-04-15T13:27:36.5164046Z test 2024-04-15T13:27:35.999000Z TestFramework.p2p (DEBUG): Send message to 127.0.0.1:11540: msg_block(block=CBlock(nVersion=4 hashPrevBlock=4ea0ebae5e05d36fd76d9e8b74df5858d92619c07e759207b4599172704a26fe hashMerkleRoot=894d347901f746841d50a120c07cf7ae0dc1d8afed389e63ad1032d8d4dfbcf1 nTime=Mon Apr 15 13:27:36 2024 nBits=207fffff nNonce=00000000 vtx=[CTransaction(nVersion=2 vin=[CTxIn(prevout=COutPoint(hash=0000000
...

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2063096725)
@setavenger here's one for height 727505:

```
src/bitcoin-cli getsilentpaymentblockdata 00000000000000000002d8cc04ec765eeeaaf4684f43c2ec28d81ad34436b1bc
```
```json
{
"tweaks": [
"03031f9711550ee688c52fe8635a1b122eb2744c362e19e6b2284dd69ab8d277b4",
"02be4494f8a62126483634f45192b189cd121bc037ba0b8f8f1ac4bd4e15135c94",
"03c22ca4a3d3d0e0fa8073fdc7d252f599726445a56fd61523c6f5150ed151a992",
"02adf5951aa06107d18f35e20d902b8b54c4e8ff82ddb91e7a265f7146c018ded6",
"03b95
...

👍 maflcko approved a pull request: "rpc: return warnings as an array instead of just a single one"
(https://github.com/bitcoin/bitcoin/pull/29845#pullrequestreview-2007948174)
Left some stupid style nits. Leave a comment if you want to ignore them or address them.

very nice. Thanks for fixing this. ACK 4b821915bf92082043d6cf60efb1a5faea0151db 🛏

<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}"
RUTRmVTMeKV5npGrKx1nqX
...

💬 maflcko commented on pull request "rpc: return warnings as an array instead of just a single one":
(https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1570060650)
```suggestion
obj.pushKV("warnings", GetNodeWarnings());
```

style nit: Could remove the excessive whitespace to match the previous line, and clang-format?

💬 maflcko commented on pull request "rpc: return warnings as an array instead of just a single one":
(https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1570060847)
style nit: Could remove the excessive whitespace to match the previous line, and clang-format?

💬 maflcko commented on pull request "rpc: return warnings as an array instead of just a single one":
(https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1570069541)
unrelated follow-up idea: It seems wrong to have the warnings module in the `common` library, when it is using globals to keep track of the warnings.

I guess it would be nice to actually move it to the `node` library, and `node` namespace?

💬 maflcko commented on pull request "rpc: return warnings as an array instead of just a single one":
(https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1570062725)
```suggestion
for (auto warning : GetWarnings()) {
warnings.push_back(std::move(warning.original));
}
```

style nit: Not that it matters here, also not sure if it compiles, but could move instead of copy?

style nit: Also, can drop the newline after the for-loop?

💬 maflcko commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1570094430)
```suggestion
tx_to_be_evicted_id = miniwallet.send_self_transfer(from_node=node, utxo_to_spend=confirmed_utxos.pop(0), fee_rate=relayfee)["txid"]

```

nit in the first commit: In python, the `pop` method can be used to pop

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2063145818)
Got the same results. As mentioned above I had to sort them in order to compare them.

How should we best go about comparing more blocks?
I have linked the entire tweak index [here](https://github.com/setavenger/BIP0352-light-client-specification?tab=readme-ov-file#the-files-can-be-found-here). Maybe you could do something like that for a couple of blocks and we could compare the tweaks?

```json
[
"0207a9a7ce03287d67984117a2301e4c6f3faf2e7559d1151fc3a22ea8ee897727",
"021c31eaebf539c
...

⚠️ Alfgb opened an issue: "Afoito.com"
(https://github.com/bitcoin/bitcoin/issues/29902)

💬 Sjors commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2063151580)
I use both the default signet and at least one custom one, so having separate directories would be quite useful. Shorter would be nice.

Ideally we would also support different config file sections and startup flags, using the same convention:

```
src/bitcoind -chain=signet_xxxx
```

```ini
[signet]
prune=1000

[signet_xxx]
signetchallenge=51
signetseednode=...
```

💬 maflcko commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#issuecomment-2063158835)
ACK 60ca5d55081275a011ccfc9546e0c4a8c4030493 🍳

<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: ACK 60ca5d55081275a011ccfc9546
...

✅ maflcko closed an issue: "Afoito.com"
(https://github.com/bitcoin/bitcoin/issues/29902)