Fix snapshots manager

[iulianpascalau]

Reasoning behind the pull request

• during a sync from genesis on testnet, the metachain node ends up having the PeerAccountsTrie DB at ~13GB even if the config.toml specified that SnapshotsEnabled = false and PeerStatePruningEnabled = true
  The snapshotting process waited for epoch change & snapshot markup even if the trie storage manager did not perform snapshots. The result was that the memoryEvictionWaitingList started dropping stored keys so the correct state pruning was not performed.

Proposed changes

• added an earlier exit point on the snapshotsManager handler

Testing procedure

• standard system test
• start from genesis on testnet on all shards and measure the space occupied by the DBs.

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

• was the PR targeted to the correct branch?
• if this is a larger feature that probably needs more than one PR, is there a feat branch created?
• if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

[ssd04]

i'm thinking if ShouldTakeSnapshot can be used instead of adding IsSnapshotSuppoted method. for trie storage manager without snapshot ShouldTakeSnapshot already returns false.

[iulianpascalau]

The reason I have added the new function is that the original trieStorageManager that supports snapshotting has logic inside the ShouldTakeSnapshot. And that logic we want to execute after the node steps into the new epoch. So it's kind of a chicken-and-egg problem without the new function.

[BeniaminDrasovean]

It won't work as it is right now. There are a few things that need to be set before ShouldTakeSnapshot is called if the snapshots are enabled. Will refactor in a future PR to merge these 2 funcs.

[BeniaminDrasovean]

It won't work as it is right now. There are a few things that need to be set before ShouldTakeSnapshot is called if the snapshots are enabled. Will refactor in a future PR to merge these 2 funcs.

[codecov]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (1c2116b) 80.14% compared to head (741ced4) 80.14%.

Files	Patch %	Lines
...r/evictionWaitingList/memoryEvictionWaitingList.go	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           rc/v1.6.0    #5764   +/-   ##
==========================================
  Coverage      80.14%   80.14%           
==========================================
  Files            708      708           
  Lines          94017    94027   +10     
==========================================
+ Hits           75347    75359   +12     
+ Misses         13326    13325    -1     
+ Partials        5344     5343    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[valentin-lup]

Tested on Public Testnet Stack

[gabi-vuls]

Normal allin test: master-e134e5314d -> fix-snapshots-manager-81f69cfdee

--- Specific errors ---

block hash does not match 12439
wrong nonce in block 4188
miniblocks does not match 0
num miniblocks does not match 0
miniblock hash does not match 0
block bodies does not match 0
receipts hash missmatch 0

/------/

--- Statistics ---

Nr. of all ERRORS: 22
Nr. of all WARNS: 9673
Nr. of new ERRORS: 21
Nr. of new WARNS: 132
Nr. of PANICS: 0

/------/