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 a potential problem of committing non-uniform consensus versions #3453

Merged

Conversation

algorandskiy
Copy link
Contributor

Summary

If accountdb accumulates a large backlog, it is possible catchpoint tracker would attempt to commit a wider range than account updates tracker expects.

Test Plan

Added a test simulating said behavior.

Copy link
Contributor

@tsachiherman tsachiherman left a comment

Choose a reason for hiding this comment

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

The change looks good to me.

@codecov-commenter
Copy link

Codecov Report

Merging #3453 (a5fbfcb) into master (4006ce2) will increase coverage by 0.00%.
The diff coverage is 30.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3453   +/-   ##
=======================================
  Coverage   47.64%   47.64%           
=======================================
  Files         370      370           
  Lines       59826    59834    +8     
=======================================
+ Hits        28502    28507    +5     
+ Misses      28013    28011    -2     
- Partials     3311     3316    +5     
Impacted Files Coverage Δ
ledger/accountdb.go 65.26% <0.00%> (ø)
ledger/catchpointtracker.go 56.30% <33.33%> (+1.37%) ⬆️
ledger/tracker.go 75.75% <33.33%> (-1.14%) ⬇️
crypto/merkletrie/trie.go 66.42% <0.00%> (-2.19%) ⬇️
crypto/merkletrie/node.go 91.62% <0.00%> (-1.87%) ⬇️
catchup/service.go 69.90% <0.00%> (+0.74%) ⬆️

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 4006ce2...a5fbfcb. Read the comment docs.

@tsachiherman tsachiherman merged commit 82eabff into algorand:master Jan 20, 2022
jannotti pushed a commit that referenced this pull request Jan 24, 2022
* ledger: fix `NextRewardsState()` (#3403)

## Summary

A modification of #3336. Added a new test where the rewards pool overspends and proposed a fix in `NextRewardsState()` requiring a consensus upgrade.

## Test Plan

This is mostly tests.

* Fix a potential problem of committing non-uniform consensus versions (#3453)

If accountdb accumulates a large backlog, it is possible catchpoint tracker would attempt to commit a wider range than account updates tracker expects.

* avoid generating log error on EnsureValidatedBlock / EnsureBlock (#3424)

In EnsureBlock,, do not log as error message if the error is ledgercore.ErrNonSequentialBlockEval and the block round is in the past (i.e. already in the ledger).

* Fix typo Fulll to Full (#3456)

Fix typo

* Fix worng message on restore crash db. (#3455)

When crash state is found but could not be restored, noCrashState variable is used to report a warning.
However, this variable was set to false in a case where there was no crash state, and the wrong warning was reported.

* Adding new scenario for feature networks (#3451)

Co-authored-by: Tolik Zinovyev <[email protected]>
Co-authored-by: Pavel Zbitskiy <[email protected]>
Co-authored-by: Shant Karakashian <[email protected]>
jannotti pushed a commit that referenced this pull request Jan 25, 2022
* ledger: fix `NextRewardsState()` (#3403)

## Summary

A modification of #3336. Added a new test where the rewards pool overspends and proposed a fix in `NextRewardsState()` requiring a consensus upgrade.

## Test Plan

This is mostly tests.

* Fix a potential problem of committing non-uniform consensus versions (#3453)

If accountdb accumulates a large backlog, it is possible catchpoint tracker would attempt to commit a wider range than account updates tracker expects.

* avoid generating log error on EnsureValidatedBlock / EnsureBlock (#3424)

In EnsureBlock,, do not log as error message if the error is ledgercore.ErrNonSequentialBlockEval and the block round is in the past (i.e. already in the ledger).

* Fix typo Fulll to Full (#3456)

Fix typo

* Fix worng message on restore crash db. (#3455)

When crash state is found but could not be restored, noCrashState variable is used to report a warning.
However, this variable was set to false in a case where there was no crash state, and the wrong warning was reported.

* Adding new scenario for feature networks (#3451)

* Fixing telemetry ports (#3497)

Co-authored-by: Tolik Zinovyev <[email protected]>
Co-authored-by: Pavel Zbitskiy <[email protected]>
Co-authored-by: Shant Karakashian <[email protected]>
Co-authored-by: Jack <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants