💬 denavila commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561725613)
> The API isn't public, so changing it isn't a big concern. Really the only consumer of it is the GUI.
>
> Having it inside of `CWallet` will probably make this feature more likely to be accepted. It will be easier to write tests (adding an RPC allows for functional tests, and implementing in `CWallet` allows unit tests to better access the data) and it can be split up into at least 2 chunks - the `CWallet` and RPC only implementation, and then the GUI and interface changes. This makes it mor
...

💬 D33r-Gee commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1561739681)
Tested up to "Finalizing a PSBT..." (no bonus test yet).

They all executed successfully!

25.0rc2 compiled from source

Hardware Info:

- Memory: 15.5 GiB
- Processor: Intel® Core™ i7-7700HQ CPU @ 2.80GHz × 8
- OS: Ubuntu 20.04.5 LTS 64-bit

💬 achow101 commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561747032)
> I'd need to open a PR against the main repo in that case, right?

Yes

> Should I keep this PR and leave the GUI bits here, and open another PR in the main repo with just the CWallet changes?
> Or is it better to close this PR and open a new one in the main repo with all the changes?

You can leave this open and rebase it onto the main repo PR, just mention it in the description.

💬 ArmchairCryptologist commented on issue "Frequent "Timeout downloading block" with 24.1":
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1561755480)
>The reason why connections/disconnections aren't logged for inbound peers is that these events are remotely triggerable at almost no cost. If we'd log these by default, that would make us susceptible to disk-exhaustion attacks where an attacker tries to fill up our logs over time by triggering the log repeatedly.

I see, that makes sense in general. But when a connection is closed due to a block download timeout, you already generate a log entry `Timeout downloading block x from peer=y, disco
...

💬 davidsfeet7-z commented on issue "Frequent "Timeout downloading block" with 24.1":
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1561760539)
Please stop

On Wed, May 24, 2023, 12:56 PM Martin Zumsande ***@***.***>
wrote:

> I did block the peer right after I posted the latest logs four days ago,
> and after that the block timeouts dropped to 2-3 per day, but like I
> mentioned in the bug report, since Core doesn't normally log the IP of
> disconnected incoming peers you pretty much have to enable debug=net
> logging to find the IP of the misbehaving peer. Which on this particular
> node is ~1GB of logs per 2-3 hours.
>
> The reason w
...

🤔 mzumsande reviewed a pull request: "Unconditionally return when compact block status == READ_STATUS_FAILED"
(https://github.com/bitcoin/bitcoin/pull/27743#pullrequestreview-1442557635)
ACK d97269579769effbe6eec2303ea0cc3e396d3e0d
Agree that it doesn't make sense to continue and send a BLOCKTXN here.

💬 willcl-ark commented on issue "bitcoin core crashes when too many rpc calls are made":
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-1561803409)
> I have opened a draft PR #27731 that uses the proposed solution with the max connection feature that was added there. But currently I still have a hard time reproducing the issue in the same way I was able to back in 2020 described above: [#11368 (comment)](https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-638770701) I have a much faster machine now but there may also be other reasons.

I also do not seem to be able to reproduce this on master with a similar python script.

<det
...

⚠️ Sjors opened an issue: "Log which peer sent us a header (first)"
(https://github.com/bitcoin/bitcoin/issues/27744)
Discussed in https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1478368582

```
Saw new header hash=... height=...
```

It would be useful to add the node id to this message. The validation code doesn't know which peer the header came from, so we'd have to move the log entry to net_processing.

One way is to add a helper function `LogBlockHeader` and call it right after both `ProcessNewBlockHeaders(header, peer, compact_blk)` calls in net_processing, only if they return true. Sin
...

📝 amitiuttarwar opened a pull request: "addrman: select addresses by network follow-up "
(https://github.com/bitcoin/bitcoin/pull/27745)
this PR addresses outstanding review comments from #27214

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1204661115)
done in #27745

💬 mzumsande commented on issue "Frequent "Timeout downloading block" with 24.1":
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1561807894)
> As such it shouldn't add any risk of such attacks if the remote address was simply added to this line, something like peer=x addr=x.x.x.x?

I agree, with `addr=x.x.x.x` being conditional on `-logips`. That could make sense in some more spots as well, I could open a PR to suggest it unless you want to.

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1204661204)
done in #27745

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1204661284)
done in #27745

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1204661358)
done in #27745

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1204661423)
done in #27745

💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#discussion_r1204663303)
Good catch thanks. First draft of this PR did check for `blocks` directory but of course that is unreliable because it could be customized by the user.

💬 ryanofsky commented on pull request "refactor: Replace `std::optional<bilingual_str>` with `util::Result`":
(https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1561821162)
Thanks, added std::move as suggested.

Updated b2bcba068e2c5542ab157e62c82bb868f678beeb -> 8aa8f73adce72e1ae855b43413c1f65504423cb7 ([`pr/bresult-opt.10`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult-opt.10) -> [`pr/bresult-opt.11`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult-opt.11), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/bresult-opt.10..pr/bresult-opt.11)) with workaround for CI errors https://cirrus-ci.com/task/6456067106799616?logs=ci#L1528 https:/
...

💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-1561833344)
> This change ignores non-main network data.

Ah I see, because I'm checking for `Bitcoin\chainstate` specifically but if a user only ever used signet, there would be a nested `signet` directory in there too. Is there anything else I can check for that always gets written to that root? Or maybe just check if `Bitcoin\` is empty at all?

> I'm OK with that. But it seems worth mentioning in the docs.

Sure but I feel like that's not super clean either.

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1204686737)
done

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1204687270)
Sounds good, I am using this now.