Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce hook max gas #119

Merged
merged 2 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions proto/opinit/opchild/v1/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ message Params {
(amino.dont_omitempty) = true,
(gogoproto.moretags) = "yaml:\"fee_whitelist\""
];
// Max gas for hook execution of `MsgFinalizeTokenDeposit`
uint64 hook_max_gas = 7 [(gogoproto.moretags) = "yaml:\"hook_max_gas\""];
}

// Validator defines a validator, together with the total amount of the
Expand Down
19 changes: 18 additions & 1 deletion x/opchild/keeper/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,23 @@
"context"
"fmt"

storetypes "cosmossdk.io/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/initia-labs/OPinit/x/opchild/types"
)

func (k Keeper) handleBridgeHook(ctx sdk.Context, data []byte) (success bool, reason string) {
func (k Keeper) handleBridgeHook(ctx sdk.Context, data []byte, hookMaxGas uint64) (success bool, reason string) {
beer-1 marked this conversation as resolved.
Show resolved Hide resolved
if hookMaxGas == 0 {
return false, "hook max gas is zero"
}

Check warning on line 16 in x/opchild/keeper/deposit.go

View check run for this annotation

Codecov / codecov/patch

x/opchild/keeper/deposit.go#L15-L16

Added lines #L15 - L16 were not covered by tests

originGasMeter := ctx.GasMeter()
gasForHook := originGasMeter.GasRemaining()
if gasForHook > hookMaxGas {
gasForHook = hookMaxGas
}

defer func() {
if r := recover(); r != nil {
reason = fmt.Sprintf("panic: %v", r)
Expand All @@ -18,8 +30,13 @@
if len(reason) > maxReasonLength {
reason = reason[:maxReasonLength] + "..."
}

originGasMeter.ConsumeGas(ctx.GasMeter().GasConsumedToLimit(), "bridge hook")
}()

// use new gas meter with the hook max gas limit
ctx = ctx.WithGasMeter(storetypes.NewGasMeter(gasForHook))

tx, err := k.txDecoder(data)
if err != nil {
reason = fmt.Sprintf("Failed to decode tx: %s", err)
Expand Down
7 changes: 6 additions & 1 deletion x/opchild/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,10 +436,15 @@
sdk.NewAttribute(types.AttributeKeyFinalizeHeight, strconv.FormatUint(req.Height, 10)),
)

params, err := ms.GetParams(ctx)
if err != nil {
return nil, err
}

Check warning on line 442 in x/opchild/keeper/msg_server.go

View check run for this annotation

Codecov / codecov/patch

x/opchild/keeper/msg_server.go#L441-L442

Added lines #L441 - L442 were not covered by tests

// if the deposit is successful and the data is not empty, execute the hook
hookSuccess := true
if depositSuccess && len(req.Data) > 0 {
hookSuccess, reason = ms.handleBridgeHook(sdkCtx, req.Data)
hookSuccess, reason = ms.handleBridgeHook(sdkCtx, req.Data, params.HookMaxGas)
event = event.AppendAttributes(sdk.NewAttribute(types.AttributeKeySuccess, strconv.FormatBool(hookSuccess)))
if !hookSuccess {
event = event.AppendAttributes(sdk.NewAttribute(types.AttributeKeyReason, "hook failed; "+reason))
Expand Down
58 changes: 58 additions & 0 deletions x/opchild/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
testutilsims "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -559,6 +560,63 @@ func Test_MsgServer_Deposit_HookFail(t *testing.T) {
require.Equal(t, math.NewInt(0), afterBalance.Amount)
}

func Test_MsgServer_Deposit_HookFail_WithOutOfGas(t *testing.T) {
ctx, input := createDefaultTestInput(t)
ms := keeper.NewMsgServerImpl(&input.OPChildKeeper)

bz := sha3.Sum256([]byte("test_token"))
denom := "l2/" + hex.EncodeToString(bz[:])

require.Equal(t, math.ZeroInt(), input.BankKeeper.GetBalance(ctx, addrs[1], denom).Amount)

// empty deposit to create account
priv, _, addr := keyPubAddr()
msg := types.NewMsgFinalizeTokenDeposit(addrsStr[0], addrsStr[1], addr.String(), sdk.NewCoin(denom, math.ZeroInt()), 1, 1, "test_token", nil)
_, err := ms.FinalizeTokenDeposit(ctx, msg)
require.NoError(t, err)

// create hook data
acc := input.AccountKeeper.GetAccount(ctx, addr)
require.NotNil(t, acc)
privs, accNums, accSeqs := []cryptotypes.PrivKey{priv}, []uint64{acc.GetAccountNumber()}, []uint64{0}
signedTxBz, err := input.EncodingConfig.TxConfig.TxEncoder()(generateTestTx(
t, input,
[]sdk.Msg{banktypes.NewMsgSend(addr, addrs[2], sdk.NewCoins(sdk.NewCoin(denom, math.NewInt(100))))},
privs, accNums, accSeqs, sdk.UnwrapSDKContext(ctx).ChainID(),
))
require.NoError(t, err)

params, err := input.OPChildKeeper.GetParams(ctx)
require.NoError(t, err)

params.HookMaxGas = 1
input.OPChildKeeper.SetParams(ctx, params)

// valid deposit
ctx = sdk.UnwrapSDKContext(ctx).WithEventManager(sdk.NewEventManager()).WithGasMeter(storetypes.NewGasMeter(2_000_000))
msg = types.NewMsgFinalizeTokenDeposit(addrsStr[0], addrsStr[1], addr.String(), sdk.NewCoin(denom, math.NewInt(100)), 2, 1, "test_token", signedTxBz)
_, err = ms.FinalizeTokenDeposit(ctx, msg)
require.NoError(t, err)

for _, event := range sdk.UnwrapSDKContext(ctx).EventManager().Events() {
if event.Type == types.EventTypeFinalizeTokenDeposit {
attrIdx := slices.Index(event.Attributes, sdk.NewAttribute(types.AttributeKeySuccess, "false").ToKVPair())
require.Positive(t, attrIdx)
require.Equal(t, event.Attributes[attrIdx+1].Key, types.AttributeKeyReason)
require.Contains(t, event.Attributes[attrIdx+1].Value, "hook failed;")
require.Contains(t, event.Attributes[attrIdx+1].Value, "panic:")
}
}

// check addrs[2] balance
afterBalance := input.BankKeeper.GetBalance(ctx, addrs[2], denom)
require.Equal(t, math.NewInt(0), afterBalance.Amount)

// check receiver has no balance
afterBalance = input.BankKeeper.GetBalance(ctx, addr, denom)
require.Equal(t, math.NewInt(0), afterBalance.Amount)
}

func Test_MsgServer_UpdateOracle(t *testing.T) {
ctx, input := createDefaultTestInput(t)
opchildKeeper := input.OPChildKeeper
Expand Down
5 changes: 4 additions & 1 deletion x/opchild/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

var (
DefaultMinGasPrices = sdk.NewDecCoins(sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, math.LegacyNewDecWithPrec(15, 2))) // 0.15
DefaultHookMaxGas = uint64(1_000_000)
)

// DefaultParams returns default move parameters
Expand All @@ -21,18 +22,20 @@ func DefaultParams() Params {
DefaultHistoricalEntries,
DefaultMinGasPrices,
[]string{},
DefaultHookMaxGas,
)
}

// NewParams creates a new Params instance
func NewParams(admin string, bridgeExecutors []string, maxValidators, historicalEntries uint32, minGasPrice sdk.DecCoins, feeWhitelist []string) Params {
func NewParams(admin string, bridgeExecutors []string, maxValidators, historicalEntries uint32, minGasPrice sdk.DecCoins, feeWhitelist []string, hookMaxGas uint64) Params {
return Params{
Admin: admin,
BridgeExecutors: bridgeExecutors,
MaxValidators: maxValidators,
HistoricalEntries: historicalEntries,
MinGasPrices: minGasPrice,
FeeWhitelist: feeWhitelist,
HookMaxGas: hookMaxGas,
}

}
Expand Down
Loading
Loading