diff --git a/x/opchild/keeper/deposit.go b/x/opchild/keeper/deposit.go index 76f6b15..b313005 100644 --- a/x/opchild/keeper/deposit.go +++ b/x/opchild/keeper/deposit.go @@ -59,8 +59,11 @@ func (k Keeper) handleBridgeHook(ctx sdk.Context, data []byte) (success bool, re func (ms MsgServer) safeDepositToken(ctx context.Context, toAddr sdk.AccAddress, coins sdk.Coins) (success bool, reason string) { // if coin is zero, just create an account if coins.IsZero() { - newAcc := ms.authKeeper.NewAccountWithAddress(ctx, toAddr) - ms.authKeeper.SetAccount(ctx, newAcc) + if !ms.authKeeper.HasAccount(ctx, toAddr) { + newAcc := ms.authKeeper.NewAccountWithAddress(ctx, toAddr) + ms.authKeeper.SetAccount(ctx, newAcc) + } + return true, "" } diff --git a/x/opchild/keeper/executor_change.go b/x/opchild/keeper/executor_change.go index 8d943ab..ed64c0b 100644 --- a/x/opchild/keeper/executor_change.go +++ b/x/opchild/keeper/executor_change.go @@ -71,6 +71,9 @@ func (k Keeper) ChangeExecutor(ctx context.Context, plan types.ExecutorChangePla if err := k.SetValidator(ctx, plan.NextValidator); err != nil { return err } + if err = k.SetValidatorByConsAddr(ctx, plan.NextValidator); err != nil { + return err + } params, err := k.GetParams(ctx) if err != nil { diff --git a/x/opchild/keeper/executor_change_test.go b/x/opchild/keeper/executor_change_test.go index 26f6869..14495e6 100644 --- a/x/opchild/keeper/executor_change_test.go +++ b/x/opchild/keeper/executor_change_test.go @@ -9,6 +9,8 @@ import ( "testing" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" + testutilsims "github.com/cosmos/cosmos-sdk/testutil/sims" + "github.com/initia-labs/OPinit/x/opchild/types" "github.com/stretchr/testify/require" ) @@ -53,3 +55,65 @@ func Test_RegisterExecutorChangePlan(t *testing.T) { Info: info, }, input.OPChildKeeper.ExecutorChangePlans[height.Uint64()]) } + +func Test_ExecuteChangePlan(t *testing.T) { + ctx, input := createDefaultTestInput(t) + + valPubKeys := testutilsims.CreateTestPubKeys(2) + val1, err := types.NewValidator(valAddrs[1], valPubKeys[0], "validator1") + require.NoError(t, err) + + val2, err := types.NewValidator(valAddrs[2], valPubKeys[1], "validator2") + require.NoError(t, err) + + // set validators + require.NoError(t, input.OPChildKeeper.SetValidator(ctx, val1)) + require.NoError(t, input.OPChildKeeper.SetValidatorByConsAddr(ctx, val1)) + require.NoError(t, input.OPChildKeeper.SetValidator(ctx, val2)) + require.NoError(t, input.OPChildKeeper.SetValidatorByConsAddr(ctx, val2)) + + // Arguments + l1ProposalID, err := rand.Int(rand.Reader, big.NewInt(math.MaxInt64)) + require.NoError(t, err) + height, err := rand.Int(rand.Reader, big.NewInt(math.MaxInt64)) + require.NoError(t, err) + nextValAddr := valAddrsStr[0] + nextExecutorAddr := []string{addrsStr[0], addrsStr[1]} + consensusPubKey := "l7aqGv+Zjbm0rallfqfqz+3iN31iOmgJCafWV5pGs6o=" + moniker := "moniker" + info := "info" + + err = input.OPChildKeeper.RegisterExecutorChangePlan( + l1ProposalID.Uint64(), height.Uint64(), nextValAddr, + moniker, + fmt.Sprintf(`{"@type":"/cosmos.crypto.ed25519.PubKey","key":"%s"}`, consensusPubKey), + info, + nextExecutorAddr, + ) + require.NoError(t, err) + require.Len(t, input.OPChildKeeper.ExecutorChangePlans, 1) + + err = input.OPChildKeeper.ChangeExecutor(ctx, input.OPChildKeeper.ExecutorChangePlans[height.Uint64()]) + require.NoError(t, err) + + // Check if the validator has been updated + validator, found := input.OPChildKeeper.GetValidator(ctx, valAddrs[0]) + require.True(t, found) + require.Equal(t, moniker, validator.GetMoniker()) + require.Equal(t, int64(1), validator.ConsPower) + + consAddr, err := validator.GetConsAddr() + require.NoError(t, err) + v := input.OPChildKeeper.ValidatorByConsAddr(ctx, consAddr) + require.Equal(t, moniker, v.GetMoniker()) + require.Equal(t, int64(1), v.GetConsensusPower()) + + // Check if the old validators have been updated + validator, found = input.OPChildKeeper.GetValidator(ctx, valAddrs[1]) + require.True(t, found) + require.Equal(t, int64(0), validator.ConsPower) + + validator, found = input.OPChildKeeper.GetValidator(ctx, valAddrs[2]) + require.True(t, found) + require.Equal(t, int64(0), validator.ConsPower) +} diff --git a/x/opchild/keeper/msg_server.go b/x/opchild/keeper/msg_server.go index 137fbce..4d44d37 100644 --- a/x/opchild/keeper/msg_server.go +++ b/x/opchild/keeper/msg_server.go @@ -395,15 +395,15 @@ func (ms MsgServer) FinalizeTokenDeposit(ctx context.Context, req *types.MsgFina } // deposit token - var success bool + var depositSuccess bool var reason string toAddr, err := ms.authKeeper.AddressCodec().StringToBytes(req.To) if err != nil { - success = false + depositSuccess = false reason = fmt.Sprintf("failed to convert recipient address: %s", err) } else { // rollback if the deposit is failed - success, reason = ms.safeDepositToken(ctx, toAddr, sdk.NewCoins(coin)) + depositSuccess, reason = ms.safeDepositToken(ctx, toAddr, sdk.NewCoins(coin)) } // updae l1 sequence @@ -434,22 +434,39 @@ func (ms MsgServer) FinalizeTokenDeposit(ctx context.Context, req *types.MsgFina sdk.NewAttribute(types.AttributeKeyBaseDenom, req.BaseDenom), sdk.NewAttribute(types.AttributeKeyAmount, coin.Amount.String()), sdk.NewAttribute(types.AttributeKeyFinalizeHeight, strconv.FormatUint(req.Height, 10)), - sdk.NewAttribute(types.AttributeKeySuccess, strconv.FormatBool(success)), - sdk.NewAttribute(types.AttributeKeyReason, reason), ) // if the deposit is successful and the data is not empty, execute the hook - if success && len(req.Data) > 0 { - success, reason := ms.handleBridgeHook(sdkCtx, req.Data) - event = event.AppendAttributes(sdk.NewAttribute(types.AttributeKeyHookSuccess, strconv.FormatBool(success))) - event = event.AppendAttributes(sdk.NewAttribute(types.AttributeKeyHookReason, reason)) + hookSuccess := true + if depositSuccess && len(req.Data) > 0 { + hookSuccess, reason = ms.handleBridgeHook(sdkCtx, req.Data) + event = event.AppendAttributes(sdk.NewAttribute(types.AttributeKeySuccess, strconv.FormatBool(hookSuccess))) + if !hookSuccess { + event = event.AppendAttributes(sdk.NewAttribute(types.AttributeKeyReason, "hook failed; "+reason)) + } + } else { + event = event.AppendAttributes(sdk.NewAttribute(types.AttributeKeySuccess, strconv.FormatBool(depositSuccess))) + if !depositSuccess { + event = event.AppendAttributes(sdk.NewAttribute(types.AttributeKeyReason, "deposit failed; "+reason)) + } } // emit deposit event sdkCtx.EventManager().EmitEvent(event) // if the deposit is failed, initate a withdrawal - if !success { + if !depositSuccess || !hookSuccess { + if depositSuccess { + // reclaim and burn coins + burnCoins := sdk.NewCoins(coin) + if err := ms.bankKeeper.SendCoinsFromAccountToModule(ctx, toAddr, types.ModuleName, burnCoins); err != nil { + return nil, err + } + if err := ms.bankKeeper.BurnCoins(ctx, types.ModuleName, burnCoins); err != nil { + return nil, err + } + } + l2Sequence, err := ms.IncreaseNextL2Sequence(ctx) if err != nil { return nil, err @@ -482,11 +499,8 @@ func (ms MsgServer) InitiateTokenWithdrawal(ctx context.Context, req *types.MsgI } // send coins to the module account only if the amount is positive - // - pending deposits are already accounted for - if coin.IsPositive() { - if err := ms.bankKeeper.SendCoinsFromAccountToModule(ctx, senderAddr, types.ModuleName, sdk.NewCoins(coin)); err != nil { - return nil, err - } + if err := ms.bankKeeper.SendCoinsFromAccountToModule(ctx, senderAddr, types.ModuleName, burnCoins); err != nil { + return nil, err } // burn withdrawn coins from the module account diff --git a/x/opchild/keeper/msg_server_test.go b/x/opchild/keeper/msg_server_test.go index 7464d33..f3e88f6 100644 --- a/x/opchild/keeper/msg_server_test.go +++ b/x/opchild/keeper/msg_server_test.go @@ -413,13 +413,22 @@ func Test_MsgServer_Deposit_ToModuleAccount(t *testing.T) { _, 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, "deposit failed;") + } + } + afterToBalance := input.BankKeeper.GetBalance(ctx, addrs[1], denom) require.Equal(t, math.ZeroInt(), afterToBalance.Amount) afterModuleBalance := input.BankKeeper.GetBalance(ctx, opchildModuleAddress, denom) require.True(t, afterModuleBalance.Amount.IsZero()) - // token withdrawal inititated + // token withdrawal initiated events := sdk.UnwrapSDKContext(ctx).EventManager().Events() lastEvent := events[len(events)-1] require.Equal(t, sdk.NewEvent( @@ -491,7 +500,7 @@ func Test_MsgServer_Deposit_HookSuccess(t *testing.T) { for _, event := range sdk.UnwrapSDKContext(ctx).EventManager().Events() { if event.Type == types.EventTypeFinalizeTokenDeposit { - require.True(t, slices.Contains(event.Attributes, sdk.NewAttribute(types.AttributeKeyHookSuccess, "true").ToKVPair())) + require.True(t, slices.Contains(event.Attributes, sdk.NewAttribute(types.AttributeKeySuccess, "true").ToKVPair())) } } @@ -534,13 +543,20 @@ func Test_MsgServer_Deposit_HookFail(t *testing.T) { for _, event := range sdk.UnwrapSDKContext(ctx).EventManager().Events() { if event.Type == types.EventTypeFinalizeTokenDeposit { - require.True(t, slices.Contains(event.Attributes, sdk.NewAttribute(types.AttributeKeyHookSuccess, "false").ToKVPair())) + 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;") } } // 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) { diff --git a/x/opchild/keeper/val_state_change.go b/x/opchild/keeper/val_state_change.go index 4d1557e..5462b6b 100644 --- a/x/opchild/keeper/val_state_change.go +++ b/x/opchild/keeper/val_state_change.go @@ -31,12 +31,7 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx context.Context) ([]abci.V } updates := []abci.ValidatorUpdate{} - maxValidators, err := k.MaxValidators(ctx) - if err != nil { - return nil, err - } - - validators, err := k.GetValidators(ctx, maxValidators) + validators, err := k.GetAllValidators(ctx) if err != nil { return nil, err } diff --git a/x/opchild/types/events.go b/x/opchild/types/events.go index 7729e05..b08fb3d 100644 --- a/x/opchild/types/events.go +++ b/x/opchild/types/events.go @@ -28,8 +28,6 @@ const ( AttributeKeyMetadata = "metadata" AttributeKeyL1Sequence = "l1_sequence" AttributeKeyFinalizeHeight = "finalize_height" - AttributeKeyHookSuccess = "hook_success" - AttributeKeyHookReason = "hook_reason" AttributeKeyFrom = "from" AttributeKeyTo = "to" AttributeKeyL2Sequence = "l2_sequence" diff --git a/x/opchild/types/tx.go b/x/opchild/types/tx.go index 9639c1a..b95673c 100644 --- a/x/opchild/types/tx.go +++ b/x/opchild/types/tx.go @@ -171,7 +171,7 @@ func (msg MsgInitiateTokenWithdrawal) Validate(ac address.Codec) error { return sdkerrors.ErrInvalidAddress.Wrap("to address cannot be empty") } - if !msg.Amount.IsValid() || msg.Amount.IsZero() { + if !msg.Amount.IsValid() || !msg.Amount.IsPositive() { return ErrInvalidAmount } @@ -251,11 +251,8 @@ func (msg MsgFinalizeTokenDeposit) Validate(ac address.Codec) error { return sdkerrors.ErrInvalidAddress.Wrap("from address cannot be empty") } - // allow hook as a special address - if msg.To != "hook" { - if _, err := ac.StringToBytes(msg.To); err != nil { - return err - } + if _, err := ac.StringToBytes(msg.To); err != nil { + return err } // allow zero amount diff --git a/x/ophost/keeper/msg_server_test.go b/x/ophost/keeper/msg_server_test.go index af684bb..44e064c 100644 --- a/x/ophost/keeper/msg_server_test.go +++ b/x/ophost/keeper/msg_server_test.go @@ -163,7 +163,7 @@ func Test_InitiateTokenDeposit(t *testing.T) { input.Faucet.Fund(ctx, addrs[1], amount) _, err = ms.InitiateTokenDeposit( ctx, - types.NewMsgInitiateTokenDeposit(addrsStr[1], 1, "hook", amount, []byte("messages")), + types.NewMsgInitiateTokenDeposit(addrsStr[1], 1, "l2_addr", amount, []byte("messages")), ) require.NoError(t, err) require.True(t, input.BankKeeper.GetBalance(ctx, addrs[1], sdk.DefaultBondDenom).IsZero())