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

Avoid whitelisting out of range block data #2096

Merged
merged 4 commits into from
Jul 12, 2020

Conversation

SebastianMarian
Copy link
Contributor

  • Rejected whitelisting miniblocks included in headers which are out of range (this will fix the situation when a node is syncing and it will receive headers with higher nonces compared with the last one processed. Because of these headers which are received, it will whitelist its miniblocks and afterwards its transactions. More than this it will also add these transactions into immunized cache. This will fill up these caches (whitelist cache, immunized cache) with a lot of information which is not needed right now and which will also be evicted until its time will come)

…of range (this will fix the situation when a node is syncing and it will receive headers with higher nonces compared with the last one processed. Because of these headers which are received, it will whitelist its miniblocks and afterwards its transactions. More than this it will also add these transactions into immunized cache. This will fill up these caches (whitelist cache, immunized cache) with a lot of information which is not needed right now and which will also be evicted until its time will come)
@SebastianMarian SebastianMarian added type:bug Something isn't working type:feature New feature or request labels Jul 12, 2020
@SebastianMarian SebastianMarian self-assigned this Jul 12, 2020
@sasurobert sasurobert self-requested a review July 12, 2020 15:06
sasurobert
sasurobert previously approved these changes Jul 12, 2020
@LucianMincu LucianMincu self-requested a review July 12, 2020 15:12
raduchis
raduchis previously approved these changes Jul 12, 2020
@@ -2495,3 +2557,34 @@ func TestBaseBlockTrack_DoWhitelistWithShardHeaderIfNeededMetaShouldWhitelistCro

assert.True(t, ok)
}

func TestBaseBlockTrack_IsHeaderOutOfRangeShouldWork(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests run under the assumption that lastCrossNotarizedHeader has round 0, nonce 0. Did I understood corectly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, what I wanted to test is that if the difference between last cross notarized header, no matter if it has nonce and round 0 (genesis), and the current header received is out of range ( <= last notarized or > last notarized + window ) the method works correctly. So, if you are syncing block with nonce 1 or 2, last cross notarized is for sure still 0 as you can include cross nonce 1 not early than in your block with nonce 3 (because of the finality), you don't care about received cross headers with nonces greater than 11. So you will not extract from these out of range cross headers, mini blocks to whitelist them and afterward because they were whitelisted you will also accept cross transactions which will be whitelisted too and then added into the immunized cache.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, I have refactored the test to be more clear now

@SebastianMarian SebastianMarian dismissed stale reviews from raduchis and sasurobert via 1d74e38 July 12, 2020 17:26
Copy link
Contributor

@LucianMincu LucianMincu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System tests passed.

@LucianMincu LucianMincu merged commit 034eb98 into development Jul 12, 2020
@LucianMincu LucianMincu deleted the Avoid-whitelisting-out-of-range-block-data branch July 12, 2020 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working type:feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants