-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Description
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, nilTo 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