-
Notifications
You must be signed in to change notification settings - Fork 106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Security: Handle gossiped blocks that are a long way ahead of the current tip #1389
Closed
3 of 7 tasks
Labels
A-rust
Area: Updates to Rust code
C-security
Category: Security issues
I-unbounded-growth
Zebra keeps using resources, without any limit
Milestone
Comments
teor2345
added
C-enhancement
Category: This is an improvement
S-needs-triage
Status: A bug report needs triage
labels
Nov 25, 2020
I believe we'll get the behavior we want as a side effect of other work we have to do (#1390). |
hdevalence
added a commit
that referenced
this issue
Nov 25, 2020
The state service API says explicitly that AwaitUTXO requests should be coupled with a timeout layer. I didn't add this when I was testing and fixing the UTXO lookup code (#1348, #1358) because causing zebrad to hang on a failed dependency was useful for identifying cases where the code wasn't useful (and then inspecting execution traces). As a side effect, I believe this closes #1389, because far-future gossiped blocks will have their UTXO lookups time out, though we may wish to do other work as part of debugging the combined sync+gossip logic.
hdevalence
added a commit
that referenced
this issue
Nov 25, 2020
The state service API says explicitly that AwaitUTXO requests should be coupled with a timeout layer. I didn't add this when I was testing and fixing the UTXO lookup code (#1348, #1358) because causing zebrad to hang on a failed dependency was useful for identifying cases where the code wasn't useful (and then inspecting execution traces). As a side effect, I believe this closes #1389, because far-future gossiped blocks will have their UTXO lookups time out, though we may wish to do other work as part of debugging the combined sync+gossip logic.
I think #1390 addresses the UTXO case, but not the other internal queues. See my detailed response in #1391 (comment) |
teor2345
pushed a commit
that referenced
this issue
Nov 26, 2020
The state service API says explicitly that AwaitUTXO requests should be coupled with a timeout layer. I didn't add this when I was testing and fixing the UTXO lookup code (#1348, #1358) because causing zebrad to hang on a failed dependency was useful for identifying cases where the code wasn't useful (and then inspecting execution traces). As a side effect, this ticket resolves most of the hangs in #1389, because far-future gossiped blocks will have their UTXO lookups time out, though we may wish to do other work as part of debugging the combined sync+gossip logic.
This was referenced Jan 13, 2021
teor2345
added
C-bug
Category: This is a bug
and removed
C-enhancement
Category: This is an improvement
labels
Feb 4, 2021
teor2345
changed the title
Handle gossiped blocks that are a long way ahead of the current tip
Security: Handle gossiped blocks that are a long way ahead of the current tip
Dec 6, 2021
teor2345
added
C-security
Category: Security issues
I-unbounded-growth
Zebra keeps using resources, without any limit
and removed
C-bug
Category: This is a bug
labels
Dec 6, 2021
This was referenced Dec 7, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-rust
Area: Updates to Rust code
C-security
Category: Security issues
I-unbounded-growth
Zebra keeps using resources, without any limit
Is your feature request related to a problem? Please describe.
In #1328, Zebra downloads and verifies blocks gossiped via
AdvertiseBlock
requests.But these blocks can be a long way ahead of the local chain tip. If they are, they wait on:
Keeping these blocks around might fill up memory with blocks and tasks that won't complete for a long time.
These issues can also happen if peers send incorrect ObtainTips or ExtendTips responses to the syncer, but that's much less likely - and it should be handled by sync restarts.
Describe the alternatives you've considered
There are a number of different solutions to this issue:
We'll need a solution for each different kind of queue.
Additional context
This change isn't a priority unless we actually see memory usage issues in practice.
But it's a DoS risk, so we might want to fix it before the first stable release.
The text was updated successfully, but these errors were encountered: