Cosmos-sdk: Simulation: Proper Operation (Message) Execution

[alexanderbez]

Problem

The BaseApp mechanics are vital to transaction execution and understanding state transitions in the state machine of an application.

The simulation test suite operates fundamentally by generating a series of "operations" (messages in a tx) per block and executes them via a "block simulator". These operations are defined on a per-module basis.

However, and I'm not sure if this was intended when simulation was first implemented, (cc @cwgoes ) but some of these operations bypass the BaseApp entirely. In other words, they generate a sdk.Msg, create a cached Context and execute the respective module's handler...essentially mimicking BaseApp to a degree.

e.g.

// x/staking/simulation/operations/msgs.go

msg := staking.NewMsgBeginRedelegate(
    delegatorAddress, srcValidatorAddress, destValidatorAddress, sdk.NewCoin(denom, amount),
)
if msg.ValidateBasic() != nil {
    return simulation.NoOpMsg(staking.ModuleName), nil, fmt.Errorf("expected msg to pass ValidateBasic: %s", msg.GetSignBytes())
}

ctx, write := ctx.CacheContext()
ok := handler(ctx, msg).IsOK()
if ok {
    write()
}

opMsg = simulation.NewOperationMsg(msg, ok, "")
return opMsg, nil, nil

To my understanding, this can be dangerous as we're not testing the true state machine execution paths. e.g. I spent a few hours debugging why some of my changes to BaseApp weren't getting reflected in simulation and as a result were failing.

Proposal

Module operation generators should instead create a tx with the message and rely on the BaseApp that's already provided to them:

e.g.

res := app.Deliver(tx)
if !res.IsOK() {
    // ...
}

// ...

---

For Admin Use

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

[alexanderbez]

If there was a reason we intentionally did this, please lmk @cwgoes 👍

[cwgoes]

As far as I can recall, this happened because we originally tried to have both module-specific simulations and whole-app simulations, and we wanted to share code between them.

If the former is no longer necessary switching to the app logic directly seems fine to me.

[alexanderbez]

Thanks for confirming @cwgoes!

[jackzampolin]

Module specific simulations would be a really nice feature.