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

fix(core-database): fix round order #3816

Merged
merged 3 commits into from
Jun 19, 2020
Merged

Conversation

rainydio
Copy link
Contributor

@rainydio rainydio commented Jun 19, 2020

Summary

While checking block generator block processor loads round from database, but rows aren't guaranteed to be ordered. There is no built-in guarantee in db on which order rows will come out. Usually they are in order they were added, but it might be mixed.

New RoundRepository.getRound method returns rows the same way they were sorted when list of round delegates was built ordered by balance desc, public_key asc.

Equivalent to:

this.activeDelegates.sort((a, b) => {
    const voteBalanceA = a.getAttribute("delegate.voteBalance");
    const voteBalanceB = b.getAttribute("delegate.voteBalance");
    const diff = voteBalanceB.comparedTo(voteBalanceA);
    if (diff === 0) {
        return a.publicKey.localeCompare(b.publicKey, "en");
    }
    return diff
})

Checklist

  • Tests
  • Ready to be merged

@ghost ghost added Complexity: Low labels Jun 19, 2020
@codecov
Copy link

codecov bot commented Jun 19, 2020

Codecov Report

Merging #3816 into develop will decrease coverage by 0.12%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3816      +/-   ##
===========================================
- Coverage    97.28%   97.15%   -0.13%     
===========================================
  Files          617      617              
  Lines        13921    13922       +1     
  Branches      1636     1636              
===========================================
- Hits         13543    13526      -17     
- Misses         117      135      +18     
  Partials       261      261              
Flag Coverage Δ
#functional 6.35% <50.00%> (-0.13%) ⬇️
#integration 10.18% <0.00%> (-0.01%) ⬇️
#unit 95.21% <50.00%> (-0.01%) ⬇️
Impacted Files Coverage Δ
packages/core-database/src/database-service.ts 97.59% <100.00%> (ø)
...core-database/src/repositories/round-repository.ts 87.50% <100.00%> (-12.50%) ⬇️
...e-test-framework/src/matchers/functional/entity.ts 38.70% <0.00%> (-54.84%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2fee48...f26fe78. Read the comment docs.

@rainydio rainydio force-pushed the fix/core-database/round-order branch from f3fadab to fb712b4 Compare June 19, 2020 15:15
@rainydio rainydio marked this pull request as ready for review June 19, 2020 15:16
@rainydio rainydio changed the title fix(core-database): round order fix(core-database): fix round order Jun 19, 2020
@air1one air1one merged commit 9921a0d into develop Jun 19, 2020
@ghost ghost deleted the fix/core-database/round-order branch June 19, 2020 15:49
@ghost ghost removed the Status: Needs Review label Jun 19, 2020
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

Successfully merging this pull request may close these issues.

2 participants