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

Handel evaluator returning usize::MAX for full contributions is problematic #2786

Closed
nibhar opened this issue Jul 29, 2024 · 1 comment
Closed
Assignees

Comments

@nibhar
Copy link
Member

nibhar commented Jul 29, 2024

As only the top scoring contribution gets verified it is possible to send top level contributions with every participant present, which do not verify. By the nature of the top level (one above the tree depth) they are accepted as long as they are full by every node. That does not protect the network sufficiently.

By contrast one the highest level in the actual tree only at most half the network is on that level for any given validator. Furthermore the evaluator prioritizes completing lower levels over higher ones. On the lowest level any node is only on that level for one other peer.

While the goal of stopping an aggregation and sending the full aggregate to parties who send a non full aggregate is good it needs adjustment to not introduce surface for an denial of service attack. Generally nodes should stop sending messages unsolicited once they have a final aggregate. If presented with an incomplete aggregate on the appropriate level they should answer with the complete, or if applicable improved aggregate on the same level. If they cannot improve the presented aggregate they should not answer with any aggregate of their own.

This means keeping more data around after an aggregation finishes. It also means aggregations will complete a bit less fast after a node in the network has a final aggregate. It should however be effective protection against denial of service from that avenue, as well as keep network traffic low once a final aggregate is reached.

@nibhar nibhar self-assigned this Jul 29, 2024
@hrxi
Copy link
Member

hrxi commented Jul 29, 2024

An easy solution to this DoS is keeping track of who sent you the aggregations and downranking aggregations from people who sent you invalid aggregations in the past.

styppo added a commit that referenced this issue Sep 25, 2024
styppo added a commit that referenced this issue Oct 4, 2024
nibhar pushed a commit that referenced this issue Oct 8, 2024
nibhar pushed a commit that referenced this issue Oct 9, 2024
nibhar pushed a commit that referenced this issue Oct 9, 2024
nibhar pushed a commit that referenced this issue Oct 11, 2024
nibhar pushed a commit that referenced this issue Oct 15, 2024
@jsdanielh jsdanielh added this to the Nimiq PoS Mainnet milestone Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants