search

dtr-org / unit-e

A digital currency for a new era of decentralized trust
https://unit-e.io
MIT License
45 stars 15 forks source link

Cleanup old checkpoints from finalization state #1083

Closed frolosofsky closed 5 years ago

frolosofsky commented 5 years ago

This fix significantly reduces individual finalization state size.

In a test, I run 1 proposer with 4 finalizers with configuration epoch_length=2. After processing ~9k blocks and ~4.5k epochs, finalization state size was 767599. After applying this PR, it has been reduced to 196002. More finalizers we have, more memory we save, because checkpoints map holds votes from every finalizer.

Relates to #1073.

scravy commented 5 years ago

The lint step failed in a weird way:

Screenshot 2019-05-08 14 31 50

I have a feeling this has something to do with the scripted diff merged in #1072 as the messages refers to https://github.com/dtr-org/unit-e/commit/a52d88e8f1caa064c754f86f4eabaf79cac17811

@cornelius Can you have a look?

scravy commented 5 years ago

Concept ACK 0bc5e57231477ce906f11ea3e858425f7d475e28

Gnappuraz commented 5 years ago

I'm looking at finalizationstate.cpp:924 GetCheckpoint(vote.m_target_epoch).m_vote_set.insert(vote.m_validator_address); and checking if actually we always pass by the ValidateVote function, if not, is an easy way to crash the node with an old vote.

As a bare minimum I think we should change anyway the log messages in finalizationstate.cpp:322 "%s: target_epoch=%d is in the future.\n", __func__, and following since is not necessarily an epoch in the future anymore.

cornelius commented 5 years ago

The weird lint failure should be fixed by #1084 now.

Ruteri commented 5 years ago

Tested 0bc5e57231477ce906f11ea3e858425f7d475e28 and while it does improve overall memory consumption, it does not resolve the syncing issue https://github.com/dtr-org/unit-e/issues/1073.

frolosofsky commented 5 years ago

I'm looking at finalizationstate.cpp:924 GetCheckpoint(vote.m_target_epoch).m_vote_set.insert(vote.m_validator_address); and check if actually we always pass by the ValidateVote function, if not, is an easy way to crash the node with an old vote.

As a bare minimum I think we should change anyway the log messages in finalizationstate.cpp:322 "%s: target_epoch=%d is in the future.\n", __func__, and following since is not necessarily an epoch in the future anymore.

Fixed/checked in 8fe4a2479b96ab80c2a95a5c7295e34b2bcad7a2.