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

Prevent invariants from changing the state #14587

Closed
0Tech opened this issue Jan 12, 2023 · 0 comments · Fixed by #14588
Closed

Prevent invariants from changing the state #14587

0Tech opened this issue Jan 12, 2023 · 0 comments · Fixed by #14588

Comments

@0Tech
Copy link
Contributor

0Tech commented Jan 12, 2023

Summary

Prevent calling the invariants from changing the state.

Problem Definition

The role of the invariants is checking whether a blockchain invariant is broken. And any module can register its own invariants. However, the logic inside the invariant can change the chain state due to its side effect. The module developers will do their best, but there is no guarantee that it can be prevented. As an innocuous example, there are some getter logics (like GetModuleAccount()) which actually modifies the chain state. So I think x/crisis should take responsibility and prevent it from changing the state of the chain.

There are two places in which x/crisis calls the invariants.

  1. k.AssertInvariants() in its EndBlock.
  2. VerifyInvariant() in its message server.

While the case 2 uses CacheContext() in prior to the call, there is no such instrument in the case 1.
It seems there is no critical side effect right now, it would be better to prevent the possible risks.

Proposal

Use CacheContext() in x/crisis AssertInvariants().

Pros:

  • Executing invariants cannot cause side effects.

Cons:

  • Impacts the performance.
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 a pull request may close this issue.

1 participant