Skip to content

Causing all events in wasm.sudo execution to be lost Due to Forgetting to Pass the Event in subctx to the Upper Layer #9499

@Hellobloc

Description

@Hellobloc

Introduction

Note that the following hook is used in the bank module to execute the wasm contract logic in the bank module, and you already handle the passing of the gasmeter in ctx.

https://github.com/osmosis-labs/cosmos-sdk/blob/5b4e03484cf88e7d1fce003ba75b4fdc5c81f15f/x/bank/types/hooks.go#L18

But you forgot to pass events, which caused all events in the hook execution to be lost, which could eventually lead to inconsistencies between your project's key events and actual state changes.

func (k Keeper) callBeforeSendListener(context context.Context, from, to sdk.AccAddress, amount sdk.Coins, blockBeforeSend bool) (err error) {
defer func() {
if r := recover(); r != nil {
err = errorsmod.Wrapf(types.ErrBeforeSendHookOutOfGas, "%v", r)
}
}()
ctx := sdk.UnwrapSDKContext(context)
for _, coin := range amount {
cosmwasmAddress := k.GetBeforeSendHook(ctx, coin.Denom)
if cosmwasmAddress != "" {
cwAddr, err := sdk.AccAddressFromBech32(cosmwasmAddress)
if err != nil {
return err
}
var msgBz []byte
// get msgBz, either BlockBeforeSend or TrackBeforeSend
// Note that for trackBeforeSend, we need to gas meter computations to prevent infinite loop
// specifically because module to module sends are not gas metered.
// We don't need to do this for blockBeforeSend since blockBeforeSend is not called during module to module sends.
if blockBeforeSend {
msg := types.BlockBeforeSendSudoMsg{
BlockBeforeSend: types.BlockBeforeSendMsg{
From: from.String(),
To: to.String(),
Amount: osmoutils.CWCoinFromSDKCoin(coin),
},
}
msgBz, err = json.Marshal(msg)
} else {
msg := types.TrackBeforeSendSudoMsg{
TrackBeforeSend: types.TrackBeforeSendMsg{
From: from.String(),
To: to.String(),
Amount: osmoutils.CWCoinFromSDKCoin(coin),
},
}
msgBz, err = json.Marshal(msg)
}
if err != nil {
return err
}
em := sdk.NewEventManager()
// Check remaining gas in parent context and use the lesser of the fixed limit and remaining gas
gasLimit := min(ctx.GasMeter().GasRemaining(), types.BeforeSendHookGasLimit)
childCtx := ctx.WithGasMeter(storetypes.NewGasMeter(gasLimit))
_, err = k.contractKeeper.Sudo(childCtx.WithEventManager(em), cwAddr, msgBz)
if err != nil {
if strings.Contains(err.Error(), "no such contract") {
return nil
}
if k.IsModuleAcc(ctx, from) {
return nil
}
return errorsmod.Wrapf(err, "failed to call before send hook for denom %s", coin.Denom)
}
// consume gas used for calling contract to the parent ctx
ctx.GasMeter().ConsumeGas(childCtx.GasMeter().GasConsumed(), "track before send gas")
}
}
return nil
}

Fix Suggest:

Considering that wasm.sudo as a virtual machine can cause arbitrary target events, we recommend supplementing the event delivery mechanism in a timely manner.

em := sdk.NewEventManager() 
    ...
 	 _, err = k.contractKeeper.Sudo(childCtx.WithEventManager(em), cwAddr, msgBz) 
 	...
    [+]ctx.EventManager().EmitEvents(em.Events())
 	// consume gas used for calling contract to the parent ctx 
 	ctx.GasMeter().ConsumeGas(childCtx.GasMeter().GasConsumed(), "track before send gas") 
 		} 
 	} 

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status

    Needs Triage 🔍

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions