💬 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
...
(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.
(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
...
(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
...
(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
(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
(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.
(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
(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
(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
(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
(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.
(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:/
...
(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.
(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
(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.
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1204687270)
Sounds good, I am using this now.
👋 fjahr's pull request is ready for review: "net: Continuous ASMap health check"
(https://github.com/bitcoin/bitcoin/pull/27581)
(https://github.com/bitcoin/bitcoin/pull/27581)
💬 amitiuttarwar commented on pull request "fuzz: addrman, add coverage for `network` field in `Select()`, `Size()` and `GetAddr()`":
(https://github.com/bitcoin/bitcoin/pull/27549#issuecomment-1561847276)
it looks like this PR provides the same coverage as 8f91e5898b3571e0802f2197981c54dda9ee71fa in #27213. I'll remove the commit there so 27213 can focus on discussing the changes to node behavior
(https://github.com/bitcoin/bitcoin/pull/27549#issuecomment-1561847276)
it looks like this PR provides the same coverage as 8f91e5898b3571e0802f2197981c54dda9ee71fa in #27213. I'll remove the commit there so 27213 can focus on discussing the changes to node behavior
💬 amitiuttarwar commented on pull request "fuzz: addrman, add coverage for `network` field in `Select()`, `Size()` and `GetAddr()`":
(https://github.com/bitcoin/bitcoin/pull/27549#issuecomment-1561848392)
for the record, ACK 35a2175ad8bec92b409ae2202c124e39b2f3f838 - only small changes from the version (previously) proposed in 27213
(https://github.com/bitcoin/bitcoin/pull/27549#issuecomment-1561848392)
for the record, ACK 35a2175ad8bec92b409ae2202c124e39b2f3f838 - only small changes from the version (previously) proposed in 27213
📝 sdaftuar opened a pull request: "Draft: rework validation logic for assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/27746)
This PR proposes a clean up of the relationship between block storage and the chainstate objects, by moving the decision of whether to store a block on disk to something that is not chainstate-specific. Philosophically, the decision of whether to store a block on disk is related to validation rules that do not require any UTXO state; for anti-DoS reasons we were using some chainstate-specific heuristics, and those have been reworked here to achieve the proposed separation.
This PR also fixes
...
(https://github.com/bitcoin/bitcoin/pull/27746)
This PR proposes a clean up of the relationship between block storage and the chainstate objects, by moving the decision of whether to store a block on disk to something that is not chainstate-specific. Philosophically, the decision of whether to store a block on disk is related to validation rules that do not require any UTXO state; for anti-DoS reasons we were using some chainstate-specific heuristics, and those have been reworked here to achieve the proposed separation.
This PR also fixes
...