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

docs: update the nonces docs in QGB store #1841

Merged
merged 3 commits into from
May 31, 2023

Conversation

rach-id
Copy link
Member

@rach-id rach-id commented May 26, 2023

Overview

Closes #1805

The latest attestation nonce and earliest attestation nonce are values written to store. In order to get them, we use the getLatestAttestationNonce and the getEarliestAvailableAttestationNonce methods.

These methods panic if these values don't exist in store. However, the only time these values don't exist in store, if no manual changes were done to the store, is during the period between chain startup and block 1.

So, we have the CheckLatestAttestationNonce method that checks if the value exists in store and returns true if so. And we use it in abci.EndBlocker and in queries not to return a stack trace, but a relevant error when querying during the period between chain startup and block 1.

Thus, the only change we can do is think about in this case, is having these methods return an error instead of panicking, but it returns the same thing via using the CheckLatestAttestationNonce, instead of using if errors.Is(...).

This is why this PR removes the misleading docs but makes no changes on the methods.

@rach-id rach-id added the x/qgb label May 26, 2023
@rach-id rach-id self-assigned this May 26, 2023
@rach-id rach-id requested a review from evan-forbes as a code owner May 26, 2023 10:08
@MSevey MSevey requested review from a team and mojtaba-esk and removed request for a team May 26, 2023 10:08
@rach-id rach-id requested a review from rootulp May 26, 2023 10:09
@codecov-commenter
Copy link

codecov-commenter commented May 26, 2023

Codecov Report

Merging #1841 (98c470f) into main (1b33d58) will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1841      +/-   ##
==========================================
+ Coverage   21.79%   21.82%   +0.03%     
==========================================
  Files         116      116              
  Lines       13247    13247              
==========================================
+ Hits         2887     2891       +4     
+ Misses      10068    10065       -3     
+ Partials      292      291       -1     
Impacted Files Coverage Δ
x/qgb/keeper/keeper_attestation.go 66.66% <ø> (ø)

... and 1 file with indirect coverage changes

rootulp
rootulp previously approved these changes May 26, 2023
x/qgb/keeper/keeper_attestation.go Outdated Show resolved Hide resolved
x/qgb/keeper/keeper_attestation.go Outdated Show resolved Hide resolved
Co-authored-by: Rootul P <rootulp@gmail.com>
Co-authored-by: Rootul P <rootulp@gmail.com>
@MSevey MSevey requested a review from a team May 26, 2023 16:10
@rach-id rach-id requested a review from rootulp May 26, 2023 16:10
@rach-id rach-id merged commit 2751a5e into celestiaorg:main May 31, 2023
15 checks passed
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.

Investigate the CheckState methods in QGB state machine
4 participants