Skip to content
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

Check Transaction cardinality at parse time, to limit memory usage #1917

Closed
1 of 2 tasks
teor2345 opened this issue Mar 17, 2021 · 4 comments
Closed
1 of 2 tasks

Check Transaction cardinality at parse time, to limit memory usage #1917

teor2345 opened this issue Mar 17, 2021 · 4 comments
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data I-slow Problems with performance or responsiveness

Comments

@teor2345
Copy link
Contributor

teor2345 commented Mar 17, 2021

Motivation

Zebra can use a lot of memory when parsing malicious transactions, but we can use existing consensus rules to limit memory usage at parse time.

Screen Shot 2021-03-18 at 08 41 45

We should also Box some of the large Optional data structures, so that memory isn't allocated unless they are actually used. Then we can statically assert that the Transaction, Output, and Spend types aren't too large.

We should make these fixes after the Orchard designs and code (#1886 and #1860) and SafeAllocate (#1920) are implemented.

Solution

Alternative Designs

We could Box data structures that are inside Vecs, but that increases memory fragmentation, and doesn't help if the incoming message or block has enough data to fill the Vec.

@teor2345 teor2345 added C-bug Category: This is a bug A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code S-needs-triage Status: A bug report needs triage NU-5 Network Upgrade: NU5 specific tasks P-High C-security Category: Security issues I-slow Problems with performance or responsiveness I-unbounded-growth Zebra keeps using resources, without any limit I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data labels Mar 17, 2021
@teor2345 teor2345 added this to the 2021 Sprint 5 milestone Mar 17, 2021
@teor2345 teor2345 self-assigned this Mar 17, 2021
@teor2345 teor2345 changed the title Implement Transaction count constraints at parse time, to limit memory usage Check Transaction cardinality at parse time, to limit memory usage Mar 17, 2021
@teor2345
Copy link
Contributor Author

Actually Orchard is more important than this local node security fix.

@teor2345 teor2345 removed this from the 2021 Sprint 5 milestone Mar 18, 2021
@teor2345 teor2345 removed their assignment Mar 18, 2021
@teor2345 teor2345 mentioned this issue Mar 18, 2021
53 tasks
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Mar 23, 2021
@mpguerra mpguerra added this to the 2021 Sprint 24 milestone Nov 24, 2021
@mpguerra
Copy link
Contributor

Adding this one to Sprint 24 for discussion

@teor2345
Copy link
Contributor Author

Adding this one to Sprint 24 for discussion

We just did the most important part of this ticket in PRs #3069 and #3076.

But I'm happy to discuss the rest.

@teor2345 teor2345 added P-Medium and removed C-bug Category: This is a bug NU-5 Network Upgrade: NU5 specific tasks P-High I-unbounded-growth Zebra keeps using resources, without any limit labels Nov 24, 2021
@mpguerra
Copy link
Contributor

This is done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data I-slow Problems with performance or responsiveness
Projects
None yet
Development

No branches or pull requests

2 participants