Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Round fixes - Closes #216, #1783 #2169

Merged
merged 12 commits into from
Jun 28, 2018
Merged

Round fixes - Closes #216, #1783 #2169

merged 12 commits into from
Jun 28, 2018

Conversation

4miners
Copy link
Contributor

@4miners 4miners commented Jun 26, 2018

What was the problem?

Inconsistencies in round module:

  • missedBlocks and producedBlocks skipped during round snapshot creation
  • redundant calls during land and backwardLand in round logic
  • missedBlocks and producedBlocks casted to big integer (database type is INT)

How did I fix it?

  • snapshot missedBlocks and producedBlocks during round snapshot and then restoring them when we restore the snapshot
  • simplify logic in land (removed flushRound call)
  • simplify logic in backwardLand (removed updateMissedBlocks, flushRound, updateVotes calls - because they will be overwritten anyway from snapshot)
  • remove BIGINT cast on missedBlocks and producedBlocks
  • adjust expectations in various tests
  • unskip scenario in functional system rounds test (previously failing)

How to test it?

Run test suite.

Review checklist

@@ -40,8 +40,8 @@ const normalFields = [
{ name: 'fees', cast: 'bigint', def: '0', skip: ifNotExists },
{ name: 'rewards', cast: 'bigint', def: '0', skip: ifNotExists },
{ name: 'vote', cast: 'bigint', def: '0', skip: ifNotExists },
{ name: 'producedBlocks', cast: 'bigint', def: '0', skip: ifNotExists },
{ name: 'missedBlocks', cast: 'bigint', def: '0', skip: ifNotExists },
{ name: 'producedBlocks', def: 0, skip: ifNotExists },
Copy link
Contributor

Choose a reason for hiding this comment

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

(Not related to PR but) aren't def: 0 and skip: ifNotExists conflicting? Meaning if default is 0 then it will always exist and never be skipped?

Copy link
Contributor

Choose a reason for hiding this comment

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

@SargeKhan skip is used by update and sets and ignored by inserts and values. So the def covers the use case of inserting records.

@MaciejBaj MaciejBaj removed this from the Version 1.1.0 milestone Jun 26, 2018
account.delegate.producedBlocks
);
account.delegate.missedBlocks = account.delegate.missedBlocks;
account.delegate.producedBlocks = account.delegate.producedBlocks;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are not parsing, then you don't need these lines anymore.

delegate.missedBlocks = parseInt(delegate.missedBlocks);
delegate.producedBlocks = parseInt(delegate.producedBlocks);
delegate.missedBlocks = delegate.missedBlocks;
delegate.producedBlocks = delegate.producedBlocks;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. If you are not parsing, then no need these lines.

@@ -20,6 +20,6 @@
*/

UPDATE mem_accounts m
SET vote = b.vote
SET vote = b.vote, "missedBlocks" = b."missedBlocks", "producedBlocks" = b."producedBlocks"
FROM mem_votes_snapshot b
Copy link
Contributor

Choose a reason for hiding this comment

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

As you are refactoring, I suggest to use FROM mem_votes_snapshot snapshot and UPDATE mem_accounts accounts. That will make the query more readable like SET accounts.vote = snapshot.vote.

@4miners
Copy link
Contributor Author

4miners commented Jun 27, 2018

@nazarhussain Resolved, please check.

@4miners 4miners changed the title Round fixes - Closes #216, #1783 Round fixes - Closes #216, #1783, #2173 Jun 27, 2018
@4miners 4miners changed the title Round fixes - Closes #216, #1783, #2173 Round fixes - Closes #216, #1783 Jun 27, 2018
@MaciejBaj MaciejBaj merged commit 0e8683d into 1.1.0 Jun 28, 2018
@MaciejBaj MaciejBaj deleted the round_fixes branch June 28, 2018 08:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants