Oracle: Add sender checks when removing an operator (#345)

app/app.go

@@ -334,6 +334,7 @@ func NewShentuApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest
 		app.distrKeeper,
 		&app.stakingKeeper,
 		app.bankKeeper,
+		app.certKeeper,
 		app.GetSubspace(oracletypes.ModuleName),
 	)
 	app.slashingKeeper = slashingkeeper.NewKeeper(

simapp/simapp.go

@@ -353,6 +353,7 @@ func NewSimApp(
 		app.DistrKeeper,
 		&app.StakingKeeper,
 		app.BankKeeper,
+		app.CertKeeper,
 		app.GetSubspace(oracletypes.ModuleName),
 	)
 	app.SlashingKeeper = slashingkeeper.NewKeeper(

x/oracle/keeper/keeper.go

@@ -14,11 +14,12 @@ type Keeper struct {
 	bankKeeper    types.BankKeeper
 	distrKeeper   types.DistrKeeper
 	stakingKeeper types.StakingKeeper
+	CertKeeper    types.CertKeeper
 	paramSpace    types.ParamSubspace
 }
 
 func NewKeeper(cdc codec.BinaryCodec, storeKey sdk.StoreKey, authKeeper types.AccountKeeper, distriKeeper types.DistrKeeper,
-	stakingKeeper types.StakingKeeper, bankKeeper types.BankKeeper, paramSpace types.ParamSubspace) Keeper {
+	stakingKeeper types.StakingKeeper, bankKeeper types.BankKeeper, certKeeper types.CertKeeper, paramSpace types.ParamSubspace) Keeper {
 	return Keeper{
 		cdc:           cdc,
 		paramSpace:    paramSpace,
@@ -27,6 +28,7 @@ func NewKeeper(cdc codec.BinaryCodec, storeKey sdk.StoreKey, authKeeper types.Ac
 		distrKeeper:   distriKeeper,
 		stakingKeeper: stakingKeeper,
 		bankKeeper:    bankKeeper,
+		CertKeeper:    certKeeper,
 	}
 }
 

x/oracle/keeper/keeper_test.go

@@ -240,7 +240,7 @@ func (suite *KeeperTestSuite) TestOperator_Remove() {
 			operator2, err := suite.keeper.GetOperator(suite.ctx, tc.args.senderAddr2)
 			suite.Require().NoError(err, tc.name)
 			// remove operator1
-			err = suite.keeper.RemoveOperator(suite.ctx, operator1Addr)
+			err = suite.keeper.RemoveOperator(suite.ctx, operator1Addr.String(), operator1Addr.String())
 			allOperators := suite.keeper.GetAllOperators(suite.ctx)
 			if tc.errArgs.shouldPass {
 				suite.Require().NoError(err, tc.name)

x/oracle/keeper/msg_server.go

@@ -52,12 +52,7 @@ func (k msgServer) CreateOperator(goCtx context.Context, msg *types.MsgCreateOpe
 func (k msgServer) RemoveOperator(goCtx context.Context, msg *types.MsgRemoveOperator) (*types.MsgRemoveOperatorResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	addr, err := sdk.AccAddressFromBech32(msg.Address)
-	if err != nil {
-		return nil, err
-	}
-
-	if err := k.Keeper.RemoveOperator(ctx, addr); err != nil {
+	if err := k.Keeper.RemoveOperator(ctx, msg.Address, msg.Proposer); err != nil {
 		return nil, err
 	}
 	ctx.EventManager().EmitEvents(sdk.Events{

x/oracle/keeper/operator.go

@@ -83,29 +83,39 @@ func (k Keeper) CreateOperator(ctx sdk.Context, address sdk.AccAddress, collater
 }
 
 // RemoveOperator removes an operator, creates an withdrawal for collateral and gives back rewards immediately.
-func (k Keeper) RemoveOperator(ctx sdk.Context, address sdk.AccAddress) error {
-	if !k.IsOperator(ctx, address) {
+func (k Keeper) RemoveOperator(ctx sdk.Context, operatorAddress, proposerAddress string) error {
+	// Ensure that the sender of the tx is either the operator to be removed itself or a certifier.
+	proposerAddr, err := sdk.AccAddressFromBech32(proposerAddress)
+	if err != nil {
+		return err
+	}
+	if operatorAddress != proposerAddress && !k.CertKeeper.IsCertifier(ctx, proposerAddr) {
+		return types.ErrUnqualifiedRemover
+	}
+
+	operatorAddr, err := sdk.AccAddressFromBech32(operatorAddress)
+	if err != nil {
+		return err
+	}
+	if !k.IsOperator(ctx, operatorAddr) {
 		return types.ErrNoOperatorFound
 	}
-	operator, err := k.GetOperator(ctx, address)
+
+	operator, err := k.GetOperator(ctx, operatorAddr)
 	if err != nil {
 		return nil
 	}
 	if err := k.ReduceTotalCollateral(ctx, operator.Collateral); err != nil {
 		return err
 	}
-	operatorAddr, err := sdk.AccAddressFromBech32(operator.Address)
-	if err != nil {
-		panic(err)
-	}
 	if err := k.CreateWithdraw(ctx, operatorAddr, operator.Collateral); err != nil {
 		return err
 	}
-	if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, address,
+	if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, operatorAddr,
 		operator.AccumulatedRewards); err != nil {
 		return err
 	}
-	return k.DeleteOperator(ctx, address)
+	return k.DeleteOperator(ctx, operatorAddr)
 }
 
 // GetAllOperators gets all operators.

x/oracle/keeper/querier_test.go

@@ -50,7 +50,7 @@ func TestQueryOperators(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, bz)
 
-	err = ok2.RemoveOperator(ctx, addrs[2])
+	err = ok2.RemoveOperator(ctx, addrs[2].String(), addrs[2].String())
 	require.Nil(t, err)
 
 	path = []string{"withdraws"}

x/oracle/types/errors.go

@@ -15,6 +15,7 @@ const (
 	errNoEnoughCollateral
 	errInvalidPoolParams
 	errInvalidTaskParams
+	errUnqualifiedRemover
 )
 
 const (
@@ -42,6 +43,7 @@ var (
 	ErrNoEnoughCollateral      = sdkerrors.Register(ModuleName, errNoEnoughCollateral, "collateral not enough")
 	ErrInvalidPoolParams       = sdkerrors.Register(ModuleName, errInvalidPoolParams, "invalid pool params")
 	ErrInvalidTaskParams       = sdkerrors.Register(ModuleName, errInvalidTaskParams, "invalid task params")
+	ErrUnqualifiedRemover      = sdkerrors.Register(ModuleName, errUnqualifiedRemover, "unauthorized to remove this operator - not the operator itself nor a certifier")
 
 	ErrTaskNotExists       = sdkerrors.Register(ModuleName, errTaskNotExists, "task does not exist")
 	ErrUnqualifiedOperator = sdkerrors.Register(ModuleName, errUnqualifiedOperator, "operator is not qualified")

x/oracle/types/expected_keepers.go

@@ -21,6 +21,10 @@ type BankKeeper interface {
 	SpendableCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins
 }
 
+type CertKeeper interface {
+	IsCertifier(ctx sdk.Context, addr sdk.AccAddress) bool
+}
+
 type DistrKeeper interface {
 	FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error
 }