From 86a5efc85e51825f6aa2acea6317a8c07ff8316a Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Mon, 3 Jul 2023 13:37:37 +0300 Subject: [PATCH 1/4] fix: cached context --- x/interchaintxs/keeper/ibc_handlers.go | 27 ++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/x/interchaintxs/keeper/ibc_handlers.go b/x/interchaintxs/keeper/ibc_handlers.go index 7644b0447..7dc43e23a 100644 --- a/x/interchaintxs/keeper/ibc_handlers.go +++ b/x/interchaintxs/keeper/ibc_handlers.go @@ -34,27 +34,30 @@ func (k *Keeper) outOfGasRecovery( k.Logger(ctx).Debug("Out of gas", "Gas meter", gasMeter.String()) k.contractManagerKeeper.AddContractFailure(ctx, packet.SourceChannel, senderAddress.String(), packet.GetSequence(), failureAckType) - // FIXME: add distribution call } } func (k *Keeper) createCachedContext(ctx sdk.Context) (cacheCtx sdk.Context, writeFn func(), newGasMeter sdk.GasMeter) { - gasLeft := ctx.GasMeter().Limit() - ctx.GasMeter().GasConsumed() + gasMeter := ctx.GasMeter() + gasMeterIsLimited := strings.HasPrefix(ctx.GasMeter().String(), "BasicGasMeter") - var newLimit uint64 - if gasLeft < GasReserve { - newLimit = 0 - } else { - newLimit = gasLeft - GasReserve - } + cacheCtx, writeFn = ctx.CacheContext() - newGasMeter = sdk.NewGasMeter(newLimit) + if gasMeterIsLimited { + gasLeft := gasMeter.Limit() - gasMeter.GasConsumed() - cacheCtx, writeFn = ctx.CacheContext() - if strings.HasPrefix(ctx.GasMeter().String(), "BasicGasMeter") { - cacheCtx = ctx.WithGasMeter(newGasMeter) + var newLimit uint64 + if gasLeft < GasReserve { + newLimit = 0 + } else { + newLimit = gasLeft - GasReserve + } + + gasMeter = sdk.NewGasMeter(newLimit) } + cacheCtx = cacheCtx.WithGasMeter(gasMeter) + return } From 51b6f2661ce1e69852456f944526702410315b3f Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Mon, 3 Jul 2023 15:55:53 +0300 Subject: [PATCH 2/4] more fixes --- x/interchaintxs/keeper/ibc_handlers.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/interchaintxs/keeper/ibc_handlers.go b/x/interchaintxs/keeper/ibc_handlers.go index 7dc43e23a..d5f15162a 100644 --- a/x/interchaintxs/keeper/ibc_handlers.go +++ b/x/interchaintxs/keeper/ibc_handlers.go @@ -37,11 +37,11 @@ func (k *Keeper) outOfGasRecovery( } } -func (k *Keeper) createCachedContext(ctx sdk.Context) (cacheCtx sdk.Context, writeFn func(), newGasMeter sdk.GasMeter) { +func (k *Keeper) createCachedContext(ctx sdk.Context) (sdk.Context, func(), sdk.GasMeter) { gasMeter := ctx.GasMeter() gasMeterIsLimited := strings.HasPrefix(ctx.GasMeter().String(), "BasicGasMeter") - cacheCtx, writeFn = ctx.CacheContext() + cacheCtx, writeFn := ctx.CacheContext() if gasMeterIsLimited { gasLeft := gasMeter.Limit() - gasMeter.GasConsumed() @@ -58,7 +58,7 @@ func (k *Keeper) createCachedContext(ctx sdk.Context) (cacheCtx sdk.Context, wri cacheCtx = cacheCtx.WithGasMeter(gasMeter) - return + return cacheCtx, writeFn, gasMeter } // HandleAcknowledgement passes the acknowledgement data to the appropriate contract via a Sudo call. From 5c8d6f3ba6da3fcaecfcafd2f98028bcb01a9dd8 Mon Sep 17 00:00:00 2001 From: pr0n00gler Date: Mon, 3 Jul 2023 16:56:52 +0300 Subject: [PATCH 3/4] comments --- x/interchaintxs/keeper/ibc_handlers.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/x/interchaintxs/keeper/ibc_handlers.go b/x/interchaintxs/keeper/ibc_handlers.go index d5f15162a..01253cdba 100644 --- a/x/interchaintxs/keeper/ibc_handlers.go +++ b/x/interchaintxs/keeper/ibc_handlers.go @@ -37,12 +37,27 @@ func (k *Keeper) outOfGasRecovery( } } +// createCachedContext creates a cached context for handling Sudo calls to CosmWasm smart-contracts. +// If there is an error during Sudo call, we can safely revert changes made in cached context. func (k *Keeper) createCachedContext(ctx sdk.Context) (sdk.Context, func(), sdk.GasMeter) { gasMeter := ctx.GasMeter() + // determines type of gas meter by its prefix: + // * BasicGasMeter - basic gas meter which is used for processing tx directly in block; + // * InfiniteGasMeter - is used to process txs during simulation calls. We don't need to create a limit for such meter, + // since it's infinite. gasMeterIsLimited := strings.HasPrefix(ctx.GasMeter().String(), "BasicGasMeter") cacheCtx, writeFn := ctx.CacheContext() + // if gas meter is limited: + // 1. calculate how much free gas left we have for a Sudo call; + // 2. If gasLeft less than reserved gas (GasReserved), we set gas limit for cached context to zero, meaning we can't + // process Sudo call; + // 3. If we have more gas left than reserved gas (GasReserved) for Sudo call, we set gas limit for cached context to + // difference between gas left and reserved gas: (gasLeft - GasReserve); + // + // GasReserve is the amount of gas on the context gas meter we need to reserve in order to add contract failure to keeper + // and process failed Sudo call if gasMeterIsLimited { gasLeft := gasMeter.Limit() - gasMeter.GasConsumed() From fbe35d0110751561341a2a8e2179d140403de0ef Mon Sep 17 00:00:00 2001 From: pr0n00gler Date: Mon, 3 Jul 2023 17:24:12 +0300 Subject: [PATCH 4/4] fix createCachedContext for transfer keeper --- x/transfer/ibc_handlers.go | 47 ++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/x/transfer/ibc_handlers.go b/x/transfer/ibc_handlers.go index ddc4eb51a..9ac40d335 100644 --- a/x/transfer/ibc_handlers.go +++ b/x/transfer/ibc_handlers.go @@ -37,24 +37,43 @@ func (im IBCModule) outOfGasRecovery( } } -func (im IBCModule) createCachedContext(ctx sdk.Context) (cacheCtx sdk.Context, writeFn func(), newGasMeter sdk.GasMeter) { - gasLeft := ctx.GasMeter().Limit() - ctx.GasMeter().GasConsumed() +// createCachedContext creates a cached context for handling Sudo calls to CosmWasm smart-contracts. +// If there is an error during Sudo call, we can safely revert changes made in cached context. +func (im *IBCModule) createCachedContext(ctx sdk.Context) (sdk.Context, func(), sdk.GasMeter) { + gasMeter := ctx.GasMeter() + // determines type of gas meter by its prefix: + // * BasicGasMeter - basic gas meter which is used for processing tx directly in block; + // * InfiniteGasMeter - is used to process txs during simulation calls. We don't need to create a limit for such meter, + // since it's infinite. + gasMeterIsLimited := strings.HasPrefix(ctx.GasMeter().String(), "BasicGasMeter") + + cacheCtx, writeFn := ctx.CacheContext() + + // if gas meter is limited: + // 1. calculate how much free gas left we have for a Sudo call; + // 2. If gasLeft less than reserved gas (GasReserved), we set gas limit for cached context to zero, meaning we can't + // process Sudo call; + // 3. If we have more gas left than reserved gas (GasReserved) for Sudo call, we set gas limit for cached context to + // difference between gas left and reserved gas: (gasLeft - GasReserve); + // + // GasReserve is the amount of gas on the context gas meter we need to reserve in order to add contract failure to keeper + // and process failed Sudo call + if gasMeterIsLimited { + gasLeft := gasMeter.Limit() - gasMeter.GasConsumed() + + var newLimit uint64 + if gasLeft < GasReserve { + newLimit = 0 + } else { + newLimit = gasLeft - GasReserve + } - var newLimit uint64 - if gasLeft < GasReserve { - newLimit = 0 - } else { - newLimit = gasLeft - GasReserve + gasMeter = sdk.NewGasMeter(newLimit) } - newGasMeter = sdk.NewGasMeter(newLimit) - - cacheCtx, writeFn = ctx.CacheContext() - if strings.HasPrefix(ctx.GasMeter().String(), "BasicGasMeter") { - cacheCtx = ctx.WithGasMeter(newGasMeter) - } + cacheCtx = cacheCtx.WithGasMeter(gasMeter) - return + return cacheCtx, writeFn, gasMeter } // HandleAcknowledgement passes the acknowledgement data to the appropriate contract via a Sudo call.