Cosmos-sdk: Invariants check not registered to module manager as expected

[selectAname]

Summary of Bug

In simapp/app.go, invariants check for all modules are registered to crisisKeeper after construction of app's module manager, which makes invaraints check will not actually be called
since the module manager's crisisKeeper holds none invariants.

Above problems also exists for gaia.

Version

cosmos-sdk v0.36.0-rc1
simapp/app.go:211

Steps to Reproduce

Do some invariant breaking actions such as:

1. send some coins to the bonded_tokens_pool ModuleAccount
2. send tx MsgVerifyInvariant

All invariants check pass since none checks are called actually.

Scenario: send coins to bonded_tokens_pool ModuleAccount
  Given CoinEx Chain is started
    And node0 has 90000_0000_0000 sato.CET
    And node0 send 1 sato.CET to bonded_tokens_pool address coinex1fl48vsnmsdzcv85q5d2q4z5ajdha8yu37uc2e4
    And wait at least 3 blocks
    #"module_name": "bonded_tokens_pool",
    #coinex1fl48vsnmsdzcv85q5d2q4z5ajdha8yu37uc2e4

Invariants are not broken.

---

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[whunmr]

FYI:
The mentioned Invariant broken issue already submitted at #4795

In #4795, the invariant broken can be detected, that's because we have a temporary fix to replace the empty-invariants crisis Module instance.

func (app *CetChainApp) replaceEmptyCrisisModule(modules *[]module.AppModule) {
    crisisWithInvariants := crisis.NewAppModule(app.crisisKeeper)

    app.mm.Modules[crisis.ModuleName] = crisisWithInvariants

    for i, module := range *modules {
        if module.Name() == crisis.ModuleName {
            (*modules)[i] = crisisWithInvariants
        }
    }
}

[alexanderbez]

I don't quite follow? After construction of the module manager, an app calls RegisterInvariants(&app.crisisKeeper). This internally loops through each registered module and calls RegisterInvariants. This in turn, does the actual registration of invariants on the crisis keeper's registry.

Why do you believe invariants are not getting registered?

Let's say I intentionally break the supply invariant:

// This will always break.
broken := !expectedTotal.IsEqual(supply.GetTotal().Add(expectedTotal))

Then run simulation which checks invariants every block:

$ SEED=297; go test -mod=readonly github.com/cosmos/cosmos-sdk/simapp -run TestFullAppSimulation -Enabled=true -NumBlocks=1000 -Commit=true -Seed=${SEED} -Period=1 -v -timeout 24h 

I will get a failed invariant as expected:

...
Invariants broken after BeginBlock

supply: total supply invariant
        sum of accounts coins: 13850691847589stake
        supply.Total:          13850691847589stake
invariant broken: true

Logs to writing to /Users/aleksbez/.simapp/simulations/2019-08-05_15:20:20.log
--- FAIL: TestFullAppSimulation (0.21s)

Are you stating this works for simulation, but not for the app?

[selectAname]

@alexanderbez Yes, it relates to how your tests are constructed.
In TestFullAppSimulation, invariants check are explicitly called after BeginBlock. The invariants passed are generated through invaraints(app), which returns all invariants registered to app.crisisKeeper.

// simapp/sim_test.go
func invariants(app *SimApp) []sdk.Invariant {
    // TODO: fix PeriodicInvariants, it doesn't seem to call individual invariants for a period of 1
    // Ref: https://github.com/cosmos/cosmos-sdk/issues/4631
    if period == 1 {
        return app.crisisKeeper.Invariants()
    }
    return simulation.PeriodicInvariants(app.crisisKeeper.Invariants(), period, 0)
}

When real app runs by the way you design, invariant check are called in 3 ways:

• crisis.EndBlocker() calls during a specific period
• send an Invariant check tx
• crisis.initGenesis calls

However, in all ways above invaraints check are called through crisis.AppModule.keeper, not app.crisisKeeper:

// crisis/module.go
func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate {
    EndBlocker(ctx, am.keeper)
    return []abci.ValidatorUpdate{}
}

The problem is that all the invariants are registered to app.crisisKeeper, not crisis.AppModule.keeper. Since crisisKeeper is passed by value to construct crisis.AppModule, and invariants are registered to crisisKeeper after that. Thus no invariant check is called actually when a real app is running.

[alexanderbez]

I see. Thanks for clarifying @selectAname, you are correct.