From d145189d26be88acbc8c9185909c81a9d97713d0 Mon Sep 17 00:00:00 2001 From: sh-cha Date: Tue, 21 May 2024 17:57:33 +0900 Subject: [PATCH 1/4] check if withdrawing token is from l1 --- x/opchild/keeper/keeper.go | 18 ++++++++++++++ x/opchild/keeper/msg_server.go | 21 ++++++++++++++++ x/opchild/keeper/msg_server_test.go | 38 ++++++++++++++++++++++++++--- x/opchild/types/errors.go | 1 + x/opchild/types/expected_keepers.go | 1 + 5 files changed, 75 insertions(+), 4 deletions(-) diff --git a/x/opchild/keeper/keeper.go b/x/opchild/keeper/keeper.go index 94f1cf39..09636527 100644 --- a/x/opchild/keeper/keeper.go +++ b/x/opchild/keeper/keeper.go @@ -157,6 +157,15 @@ func (k Keeper) setDenomMetadata(ctx context.Context, baseDenom, denom string) { k.bankKeeper.SetDenomMetaData(ctx, metadata) } +func (k Keeper) getBaseDenomFromMetadata(ctx context.Context, denom string) (string, bool) { + metadata, ok := k.bankKeeper.GetDenomMetaData(ctx, denom) + if !ok { + return "", false + } + + return metadata.Display, true +} + // UpdateHostValidatorSet updates the host validator set. func (k Keeper) UpdateHostValidatorSet(ctx context.Context, clientID string, height int64, validatorSet *cmtproto.ValidatorSet) error { if clientID == "" { @@ -195,3 +204,12 @@ func (k Keeper) L1ChainId(ctx context.Context) (string, error) { return info.L1ChainId, nil } + +func (k Keeper) BridgeId(ctx context.Context) (uint64, error) { + info, err := k.BridgeInfo.Get(ctx) + if err != nil { + return 0, err + } + + return info.BridgeId, nil +} diff --git a/x/opchild/keeper/msg_server.go b/x/opchild/keeper/msg_server.go index ebcefcd1..e9246526 100644 --- a/x/opchild/keeper/msg_server.go +++ b/x/opchild/keeper/msg_server.go @@ -13,6 +13,7 @@ import ( govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/initia-labs/OPinit/x/opchild/types" + ophosttypes "github.com/initia-labs/OPinit/x/ophost/types" ) type MsgServer struct { @@ -445,6 +446,26 @@ func (ms MsgServer) InitiateTokenWithdrawal(ctx context.Context, req *types.MsgI sdkCtx := sdk.UnwrapSDKContext(ctx) coin := req.Amount + bridgeId, err := ms.BridgeId(ctx) + if err != nil { + return nil, err + } + + // register denom metadata + if ok := ms.bankKeeper.HasDenomMetaData(ctx, coin.Denom); !ok { + return nil, types.ErrNotTokenFromL1 + } + + baseDenom, ok := ms.getBaseDenomFromMetadata(ctx, coin.Denom) + if !ok { + return nil, types.ErrNotTokenFromL1 + } + + expectedL2Denom := ophosttypes.L2Denom(bridgeId, baseDenom) + if expectedL2Denom != coin.Denom { + return nil, types.ErrNotTokenFromL1 + } + senderAddr, err := ms.authKeeper.AddressCodec().StringToBytes(req.Sender) if err != nil { return nil, err diff --git a/x/opchild/keeper/msg_server_test.go b/x/opchild/keeper/msg_server_test.go index bf1638c3..e9a61ee4 100644 --- a/x/opchild/keeper/msg_server_test.go +++ b/x/opchild/keeper/msg_server_test.go @@ -272,16 +272,46 @@ func Test_MsgServer_Withdraw(t *testing.T) { ctx, input := createDefaultTestInput(t) ms := keeper.NewMsgServerImpl(input.OPChildKeeper) - bz := sha3.Sum256([]byte("test_token")) - denom := "l2/" + hex.EncodeToString(bz[:]) + info := types.BridgeInfo{ + BridgeId: 1, + BridgeAddr: addrsStr[1], + L1ChainId: "test-chain-id", + L1ClientId: "test-client-id", + BridgeConfig: ophosttypes.BridgeConfig{ + Challenger: addrsStr[2], + Proposer: addrsStr[3], + BatchInfo: ophosttypes.BatchInfo{ + Submitter: addrsStr[4], + Chain: "l1", + }, + SubmissionInterval: time.Minute, + FinalizationPeriod: time.Hour, + SubmissionStartTime: time.Now().UTC(), + Metadata: []byte("metadata"), + }, + } + + _, err := ms.SetBridgeInfo(ctx, types.NewMsgSetBridgeInfo(addrsStr[0], info)) + require.NoError(t, err) + baseDenom := "test_token" + denom := ophosttypes.L2Denom(1, baseDenom) + + _, err = ms.FinalizeTokenDeposit(ctx, types.NewMsgFinalizeTokenDeposit(addrsStr[0], addrsStr[1], addrsStr[1], sdk.NewCoin(denom, math.NewInt(100)), 1, "test_token", nil)) + require.NoError(t, err) + + coins := sdk.NewCoins(sdk.NewCoin("foo", math.NewInt(1_000_000_000)), sdk.NewCoin(denom, math.NewInt(1_000_000_000))) // fund asset - account := input.Faucet.NewFundedAccount(ctx, sdk.NewCoin(denom, math.NewInt(1_000_000_000))) + account := input.Faucet.NewFundedAccount(ctx, coins...) accountAddr, err := input.AccountKeeper.AddressCodec().BytesToString(account) require.NoError(t, err) + // not token from l1 + msg := types.NewMsgInitiateTokenWithdrawal(accountAddr, addrsStr[1], sdk.NewCoin("foo", math.NewInt(100))) + _, err = ms.InitiateTokenWithdrawal(ctx, msg) + // valid - msg := types.NewMsgInitiateTokenWithdrawal(accountAddr, addrsStr[1], sdk.NewCoin(denom, math.NewInt(100))) + msg = types.NewMsgInitiateTokenWithdrawal(accountAddr, addrsStr[1], sdk.NewCoin(denom, math.NewInt(100))) _, err = ms.InitiateTokenWithdrawal(ctx, msg) require.NoError(t, err) diff --git a/x/opchild/types/errors.go b/x/opchild/types/errors.go index d4acaeb6..cd8c0ddd 100644 --- a/x/opchild/types/errors.go +++ b/x/opchild/types/errors.go @@ -26,4 +26,5 @@ var ( ErrInvalidPrices = errorsmod.Register(ModuleName, 19, "invalid oracle prices") ErrMaxValidatorsExceeded = errorsmod.Register(ModuleName, 20, "max validators exceeded") ErrMaxValidatorsLowerThanCurrent = errorsmod.Register(ModuleName, 21, "max validators cannot be lower than current number of validators") + ErrNotTokenFromL1 = errorsmod.Register(ModuleName, 22, "token is not from L1") ) diff --git a/x/opchild/types/expected_keepers.go b/x/opchild/types/expected_keepers.go index 69cc5e7f..0487c02d 100644 --- a/x/opchild/types/expected_keepers.go +++ b/x/opchild/types/expected_keepers.go @@ -48,6 +48,7 @@ type BankKeeper interface { HasDenomMetaData(ctx context.Context, denom string) bool SetDenomMetaData(ctx context.Context, denomMetaData banktypes.Metadata) + GetDenomMetaData(ctx context.Context, denom string) (banktypes.Metadata, bool) } type OracleKeeper interface { From a7a1fb17269b080b1cedb42b47342c86f71f22f3 Mon Sep 17 00:00:00 2001 From: sh-cha Date: Tue, 21 May 2024 17:58:16 +0900 Subject: [PATCH 2/4] miss error check --- x/opchild/keeper/msg_server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/opchild/keeper/msg_server_test.go b/x/opchild/keeper/msg_server_test.go index e9a61ee4..d3b7aa0e 100644 --- a/x/opchild/keeper/msg_server_test.go +++ b/x/opchild/keeper/msg_server_test.go @@ -309,12 +309,12 @@ func Test_MsgServer_Withdraw(t *testing.T) { // not token from l1 msg := types.NewMsgInitiateTokenWithdrawal(accountAddr, addrsStr[1], sdk.NewCoin("foo", math.NewInt(100))) _, err = ms.InitiateTokenWithdrawal(ctx, msg) + require.Error(t, err) // valid msg = types.NewMsgInitiateTokenWithdrawal(accountAddr, addrsStr[1], sdk.NewCoin(denom, math.NewInt(100))) _, err = ms.InitiateTokenWithdrawal(ctx, msg) require.NoError(t, err) - } ///////////////////////////////////////// From 33097d9fd02efe295750a99bed1070bd4d071ba4 Mon Sep 17 00:00:00 2001 From: beer-1 <147697694+beer-1@users.noreply.github.com> Date: Thu, 23 May 2024 14:56:46 +0900 Subject: [PATCH 3/4] store denom pair for withdraw validation --- x/opchild/keeper/keeper.go | 11 ++--------- x/opchild/keeper/msg_server.go | 29 ++++++++++++----------------- x/opchild/keeper/msg_server_test.go | 2 ++ x/opchild/types/keys.go | 7 +++++-- 4 files changed, 21 insertions(+), 28 deletions(-) diff --git a/x/opchild/keeper/keeper.go b/x/opchild/keeper/keeper.go index 09636527..d817e7ba 100644 --- a/x/opchild/keeper/keeper.go +++ b/x/opchild/keeper/keeper.go @@ -50,6 +50,7 @@ type Keeper struct { Validators collections.Map[[]byte, types.Validator] ValidatorsByConsAddr collections.Map[[]byte, []byte] HistoricalInfos collections.Map[int64, cosmostypes.HistoricalInfo] + DenomPairs collections.Map[string, string] ExecutorChangePlans map[uint64]types.ExecutorChangePlan @@ -107,6 +108,7 @@ func NewKeeper( Validators: collections.NewMap(sb, types.ValidatorsPrefix, "validators", collections.BytesKey, codec.CollValue[types.Validator](cdc)), ValidatorsByConsAddr: collections.NewMap(sb, types.ValidatorsByConsAddrPrefix, "validators_by_cons_addr", collections.BytesKey, collections.BytesValue), HistoricalInfos: collections.NewMap(sb, types.HistoricalInfoPrefix, "historical_infos", collections.Int64Key, codec.CollValue[cosmostypes.HistoricalInfo](cdc)), + DenomPairs: collections.NewMap(sb, types.DenomPairPrefix, "denom_pairs", collections.StringKey, collections.StringValue), ExecutorChangePlans: make(map[uint64]types.ExecutorChangePlan), HostValidatorStore: hostValidatorStore, @@ -157,15 +159,6 @@ func (k Keeper) setDenomMetadata(ctx context.Context, baseDenom, denom string) { k.bankKeeper.SetDenomMetaData(ctx, metadata) } -func (k Keeper) getBaseDenomFromMetadata(ctx context.Context, denom string) (string, bool) { - metadata, ok := k.bankKeeper.GetDenomMetaData(ctx, denom) - if !ok { - return "", false - } - - return metadata.Display, true -} - // UpdateHostValidatorSet updates the host validator set. func (k Keeper) UpdateHostValidatorSet(ctx context.Context, clientID string, height int64, validatorSet *cmtproto.ValidatorSet) error { if clientID == "" { diff --git a/x/opchild/keeper/msg_server.go b/x/opchild/keeper/msg_server.go index e9246526..b752711d 100644 --- a/x/opchild/keeper/msg_server.go +++ b/x/opchild/keeper/msg_server.go @@ -13,7 +13,6 @@ import ( govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/initia-labs/OPinit/x/opchild/types" - ophosttypes "github.com/initia-labs/OPinit/x/ophost/types" ) type MsgServer struct { @@ -407,6 +406,15 @@ func (ms MsgServer) FinalizeTokenDeposit(ctx context.Context, req *types.MsgFina ms.setDenomMetadata(ctx, req.BaseDenom, coin.Denom) } + // register denom pair + if ok, err := ms.DenomPairs.Has(ctx, coin.Denom); err != nil { + return nil, err + } else if !ok { + if err := ms.DenomPairs.Set(ctx, coin.Denom, req.BaseDenom); err != nil { + return nil, err + } + } + event := sdk.NewEvent( types.EventTypeFinalizeTokenDeposit, sdk.NewAttribute(types.AttributeKeyL1Sequence, strconv.FormatUint(req.Sequence, 10)), @@ -446,23 +454,10 @@ func (ms MsgServer) InitiateTokenWithdrawal(ctx context.Context, req *types.MsgI sdkCtx := sdk.UnwrapSDKContext(ctx) coin := req.Amount - bridgeId, err := ms.BridgeId(ctx) - if err != nil { + // check denom pair existence + if ok, err := ms.DenomPairs.Has(ctx, coin.Denom); err != nil { return nil, err - } - - // register denom metadata - if ok := ms.bankKeeper.HasDenomMetaData(ctx, coin.Denom); !ok { - return nil, types.ErrNotTokenFromL1 - } - - baseDenom, ok := ms.getBaseDenomFromMetadata(ctx, coin.Denom) - if !ok { - return nil, types.ErrNotTokenFromL1 - } - - expectedL2Denom := ophosttypes.L2Denom(bridgeId, baseDenom) - if expectedL2Denom != coin.Denom { + } else if !ok { return nil, types.ErrNotTokenFromL1 } diff --git a/x/opchild/keeper/msg_server_test.go b/x/opchild/keeper/msg_server_test.go index d3b7aa0e..0b2677a3 100644 --- a/x/opchild/keeper/msg_server_test.go +++ b/x/opchild/keeper/msg_server_test.go @@ -145,6 +145,7 @@ func Test_MsgServer_AddValidator(t *testing.T) { require.Error(t, err) params, err := ms.GetParams(ctx) + require.NoError(t, err) params.MaxValidators = 1 ms.SetParams(ctx, params) @@ -156,6 +157,7 @@ func Test_MsgServer_AddValidator(t *testing.T) { require.Error(t, err) params, err = ms.GetParams(ctx) + require.NoError(t, err) params.MaxValidators = 2 ms.SetParams(ctx, params) diff --git a/x/opchild/types/keys.go b/x/opchild/types/keys.go index 7496c0fc..241171df 100644 --- a/x/opchild/types/keys.go +++ b/x/opchild/types/keys.go @@ -23,6 +23,9 @@ var ( ValidatorsByConsAddrPrefix = []byte{0x41} // prefix for each key to a validator index, by pubkey FinalizedL1SequencePrefix = []byte{0x51} // prefix for finalized deposit sequences HistoricalInfoPrefix = []byte{0x61} // prefix for the historical info - HostHeightKey = []byte{0x71} - HostValidatorsPrefix = []byte{0x81} + DenomPairPrefix = []byte{0x71} // prefix for the denom pair + + // HostValidatorStore keys + HostHeightKey = []byte{0x81} + HostValidatorsPrefix = []byte{0x82} ) From fc82b985643f9c94922c9e5ad73902bc9653de9c Mon Sep 17 00:00:00 2001 From: beer-1 <147697694+beer-1@users.noreply.github.com> Date: Thu, 23 May 2024 15:00:35 +0900 Subject: [PATCH 4/4] rename error --- x/opchild/keeper/keeper.go | 9 --------- x/opchild/keeper/msg_server.go | 2 +- x/opchild/types/errors.go | 2 +- 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/x/opchild/keeper/keeper.go b/x/opchild/keeper/keeper.go index d817e7ba..d51f8192 100644 --- a/x/opchild/keeper/keeper.go +++ b/x/opchild/keeper/keeper.go @@ -197,12 +197,3 @@ func (k Keeper) L1ChainId(ctx context.Context) (string, error) { return info.L1ChainId, nil } - -func (k Keeper) BridgeId(ctx context.Context) (uint64, error) { - info, err := k.BridgeInfo.Get(ctx) - if err != nil { - return 0, err - } - - return info.BridgeId, nil -} diff --git a/x/opchild/keeper/msg_server.go b/x/opchild/keeper/msg_server.go index b752711d..104eac31 100644 --- a/x/opchild/keeper/msg_server.go +++ b/x/opchild/keeper/msg_server.go @@ -458,7 +458,7 @@ func (ms MsgServer) InitiateTokenWithdrawal(ctx context.Context, req *types.MsgI if ok, err := ms.DenomPairs.Has(ctx, coin.Denom); err != nil { return nil, err } else if !ok { - return nil, types.ErrNotTokenFromL1 + return nil, types.ErrNonL1Token } senderAddr, err := ms.authKeeper.AddressCodec().StringToBytes(req.Sender) diff --git a/x/opchild/types/errors.go b/x/opchild/types/errors.go index cd8c0ddd..bc2aa4e3 100644 --- a/x/opchild/types/errors.go +++ b/x/opchild/types/errors.go @@ -26,5 +26,5 @@ var ( ErrInvalidPrices = errorsmod.Register(ModuleName, 19, "invalid oracle prices") ErrMaxValidatorsExceeded = errorsmod.Register(ModuleName, 20, "max validators exceeded") ErrMaxValidatorsLowerThanCurrent = errorsmod.Register(ModuleName, 21, "max validators cannot be lower than current number of validators") - ErrNotTokenFromL1 = errorsmod.Register(ModuleName, 22, "token is not from L1") + ErrNonL1Token = errorsmod.Register(ModuleName, 22, "token is not from L1") )