Problem: multiple denoms can be mapped to same contract

x/cronos/genesis.go

@@ -21,7 +21,9 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState)
 		if !common.IsHexAddress(m.Contract) {
 			panic(fmt.Sprintf("Invalid contract address: %s", m.Contract))
 		}
-		k.SetExternalContractForDenom(ctx, m.Denom, common.HexToAddress(m.Contract))
+		if err := k.SetExternalContractForDenom(ctx, m.Denom, common.HexToAddress(m.Contract)); err != nil {
+			panic(err)
+		}
 	}
 
 	for _, m := range genState.AutoContracts {

x/cronos/keeper/keeper.go

@@ -111,7 +111,13 @@ func (k Keeper) GetDenomByContract(ctx sdk.Context, contract common.Address) (de
 }
 
 // SetExternalContractForDenom set the external contract for native denom, replace the old one if any existing.
-func (k Keeper) SetExternalContractForDenom(ctx sdk.Context, denom string, address common.Address) {
+func (k Keeper) SetExternalContractForDenom(ctx sdk.Context, denom string, address common.Address) error {
+	// check the contract is not registered already
+	_, found := k.GetDenomByContract(ctx, address)
+	if found {
+		return fmt.Errorf("the contract is already registered: %s", address.Hex())
+	}
+
 	store := ctx.KVStore(k.storeKey)
 	existing, found := k.getExternalContractByDenom(ctx, denom)
 	if found {
@@ -120,6 +126,7 @@ func (k Keeper) SetExternalContractForDenom(ctx sdk.Context, denom string, addre
 	}
 	store.Set(types.DenomToExternalContractKey(denom), address.Bytes())
 	store.Set(types.ContractToDenomKey(address.Bytes()), []byte(denom))
+	return nil
 }
 
 // GetExternalContracts returns all external contract mappings

x/cronos/keeper/keeper_test.go

@@ -116,27 +116,64 @@ func (suite *KeeperTestSuite) MintCoins(address sdk.AccAddress, coins sdk.Coins)
 }
 
 func (suite *KeeperTestSuite) TestDenomContractMap() {
-	suite.SetupTest()
-	keeper := suite.app.CronosKeeper
+	denom1 := "testdenom1"
+	denom2 := "testdenom2"
 
-	denom := "testdenom"
 	autoContract := common.BigToAddress(big.NewInt(1))
 	externalContract := common.BigToAddress(big.NewInt(2))
 
-	contract, found := keeper.GetContractByDenom(suite.ctx, denom)
-	suite.Require().False(found)
+	testCases := []struct {
+		name     string
+		malleate func()
+	}{
+		{
+			"success, happy path",
+			func() {
+				keeper := suite.app.CronosKeeper
 
-	keeper.SetAutoContractForDenom(suite.ctx, denom, autoContract)
+				contract, found := keeper.GetContractByDenom(suite.ctx, denom1)
+				suite.Require().False(found)
 
-	contract, found = keeper.GetContractByDenom(suite.ctx, denom)
-	suite.Require().True(found)
-	suite.Require().Equal(autoContract, contract)
+				keeper.SetAutoContractForDenom(suite.ctx, denom1, autoContract)
 
-	keeper.SetExternalContractForDenom(suite.ctx, denom, externalContract)
+				contract, found = keeper.GetContractByDenom(suite.ctx, denom1)
+				suite.Require().True(found)
+				suite.Require().Equal(autoContract, contract)
 
-	contract, found = keeper.GetContractByDenom(suite.ctx, denom)
-	suite.Require().True(found)
-	suite.Require().Equal(externalContract, contract)
+				keeper.SetExternalContractForDenom(suite.ctx, denom1, externalContract)
+
+				contract, found = keeper.GetContractByDenom(suite.ctx, denom1)
+				suite.Require().True(found)
+				suite.Require().Equal(externalContract, contract)
+			},
+		},
+		{
+			"failure, multiple denoms map to same contract",
+			func() {
+				keeper := suite.app.CronosKeeper
+				keeper.SetAutoContractForDenom(suite.ctx, denom1, autoContract)
+				err := keeper.SetExternalContractForDenom(suite.ctx, denom2, autoContract)
+				suite.Require().Error(err)
+			},
+		},
+		{
+			"failure, multiple denoms map to same external contract",
+			func() {
+				keeper := suite.app.CronosKeeper
+				err := keeper.SetExternalContractForDenom(suite.ctx, denom1, externalContract)
+				suite.Require().NoError(err)
+				err = keeper.SetExternalContractForDenom(suite.ctx, denom2, externalContract)
+				suite.Require().Error(err)
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+			tc.malleate()
+		})
+	}
 }
 
 func (suite *KeeperTestSuite) MintCoinsToModule(module string, coins sdk.Coins) error {

x/cronos/keeper/msg_server.go

@@ -67,6 +67,8 @@ func (k msgServer) UpdateTokenMapping(goCtx context.Context, msg *types.MsgUpdat
 		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "msg sender is authorized")
 	}
 	// msg is already validated
-	k.Keeper.SetExternalContractForDenom(ctx, msg.Denom, common.HexToAddress(msg.Contract))
+	if err := k.Keeper.SetExternalContractForDenom(ctx, msg.Denom, common.HexToAddress(msg.Contract)); err != nil {
+		return nil, err
+	}
 	return &types.MsgUpdateTokenMappingResponse{}, nil
 }

x/cronos/proposal_handler.go

@@ -21,7 +21,9 @@ func NewTokenMappingChangeProposalHandler(k keeper.Keeper) govtypes.Handler {
 			} else {
 				// update the mapping
 				contract := common.HexToAddress(c.Contract)
-				k.SetExternalContractForDenom(ctx, c.Denom, contract)
+				if err := k.SetExternalContractForDenom(ctx, c.Denom, contract); err != nil {
+					return err
+				}
 			}
 			return nil
 		default: