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

💬 maflcko commented on pull request "refactor: Misc int sign change fixes":
(https://github.com/bitcoin-core/gui/pull/806#issuecomment-2063201720)
rfm, or is anything left to be done here?

💬 Sjors commented on pull request "net: Decrease nMaxIPs when learning from DNS seeds":
(https://github.com/bitcoin/bitcoin/pull/29850#issuecomment-2063203156)
utACK f2e3662e57eca1330962faf38ff428a564d50a11

💬 maflcko commented on pull request "Change example address from legacy (P2PKH) to bech32m (P2TR)":
(https://github.com/bitcoin-core/gui/pull/808#issuecomment-2063203191)
rfm or is anything left to be done here?