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

Add RegisterInvariants to Configuration #273

Merged
merged 105 commits into from
Apr 16, 2021
Merged

Add RegisterInvariants to Configuration #273

merged 105 commits into from
Apr 16, 2021

Conversation

likhita-809
Copy link
Contributor

@likhita-809 likhita-809 commented Feb 22, 2021

Closes: #223

An Invariant is a function that checks for a particular invariant within a module. These functions are useful to detect bugs early on and act upon them to limit their potential consequences (e.g. by halting the chain). If the invariant has been broken, it should return an error containing a descriptive message about what happened and a boolean indicating whether the invariant has been broken.

@codecov
Copy link

codecov bot commented Feb 22, 2021

Codecov Report

Merging #273 (d706766) into master (1b7c80e) will decrease coverage by 0.42%.
The diff coverage is 48.45%.

@@            Coverage Diff             @@
##           master     #273      +/-   ##
==========================================
- Coverage   62.17%   61.74%   -0.43%     
==========================================
  Files          47       48       +1     
  Lines        3003     3100      +97     
==========================================
+ Hits         1867     1914      +47     
- Misses        914      947      +33     
- Partials      222      239      +17     

x/group/server/invariant.go Outdated Show resolved Hide resolved
x/group/server/invariant.go Outdated Show resolved Hide resolved
x/group/server/invariant.go Outdated Show resolved Hide resolved
x/group/server/invariant.go Outdated Show resolved Hide resolved
x/group/server/invariant.go Outdated Show resolved Hide resolved
x/group/server/invariant.go Outdated Show resolved Hide resolved
x/group/server/invariant.go Outdated Show resolved Hide resolved
x/group/server/query_server.go Outdated Show resolved Hide resolved
@likhita-809 likhita-809 marked this pull request as ready for review March 8, 2021 08:16
Copy link
Member

@blushi blushi left a comment

Choose a reason for hiding this comment

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

groupTotalWeightInvariant implementation is wrong

x/group/server/invariants_test.go Outdated Show resolved Hide resolved
x/group/server/invariants_test.go Show resolved Hide resolved
x/group/server/invariants_test.go Show resolved Hide resolved
x/group/server/invariants_test.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
@likhita-809 likhita-809 requested a review from blushi April 9, 2021 06:08
x/group/server/invariants.go Outdated Show resolved Hide resolved
@likhita-809 likhita-809 requested a review from blushi April 12, 2021 07:52
Copy link
Member

@blushi blushi left a comment

Choose a reason for hiding this comment

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

almost there! just a small fix and nit

x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
@likhita-809 likhita-809 requested a review from blushi April 14, 2021 09:30
Copy link
Contributor

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

I had a look at both invariants, and their tests, it lgtm. Thanks @likhita-809 and @blushi!

@amaury1093
Copy link
Contributor

amaury1093 commented Apr 14, 2021

One invariant I'm thinking is: the Tally on a proposal should be equal to the sum of all weigthed votes. @blushi Do you think that's useful? (If yes, we can definitely add that in another PR).

@blushi
Copy link
Member

blushi commented Apr 15, 2021

One invariant I'm thinking is: the Tally on a proposal should be equal to the sum of all weigthed votes. @blushi Do you think that's useful? (If yes, we can definitely add that in another PR).

Indeed, created a new issue to tackle this #326

x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Show resolved Hide resolved
@likhita-809 likhita-809 requested a review from blushi April 15, 2021 10:07
Copy link
Member

@blushi blushi left a comment

Choose a reason for hiding this comment

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

pre-approving, pending small change requests

x/group/server/invariants.go Show resolved Hide resolved
x/group/server/invariants.go Show resolved Hide resolved
@likhita-809 likhita-809 requested a review from blushi April 16, 2021 05:44
Copy link
Member

@blushi blushi left a comment

Choose a reason for hiding this comment

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

Thanks @likhita-809!

@blushi blushi merged commit 40e2a34 into master Apr 16, 2021
@blushi blushi deleted the likhita/add-invariants branch April 16, 2021 14:52
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.

[x/group] Implement AppModule.RegisterInvariants
4 participants