From 4e5b68663ab1b2cc2d94456c7c2152e5d5ca3f3d Mon Sep 17 00:00:00 2001
From: Maks Nabokov <max.nab@equilibrium.io>
Date: Wed, 11 Sep 2024 16:08:26 +0300
Subject: [PATCH] review fixes

---
 warden/x/act/keeper/actions.go  | 41 ++++++++++++++++++---------------
 warden/x/warden/keeper/rules.go |  8 +++++--
 2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/warden/x/act/keeper/actions.go b/warden/x/act/keeper/actions.go
index 524b7eb50..fda0b549d 100644
--- a/warden/x/act/keeper/actions.go
+++ b/warden/x/act/keeper/actions.go
@@ -3,7 +3,7 @@ package keeper
 import (
 	"context"
 	"fmt"
-	"log"
+	"reflect"
 	"runtime/debug"
 
 	"cosmossdk.io/errors"
@@ -103,7 +103,6 @@ func (k Keeper) TryRejectVotedAction(ctx context.Context, act *types.Action) err
 // result in the database.
 func (k Keeper) TryExecuteAction(ctx context.Context, act *types.Action) error {
 	ready, err := k.checkActionReady(ctx, *act)
-	log.Printf("\nready %v\n", ready)
 	if err != nil {
 		return err
 	}
@@ -215,11 +214,11 @@ func (k Keeper) AddAction(ctx context.Context, creator string, msg sdk.Msg, time
 		return nil, errors.Wrapf(types.ErrNoRuleRegistryHandler, "%v", err)
 	}
 
-	if approveRule.Expression.String() != expectedApproveExpression.String() {
+	if !reflect.DeepEqual(approveRule.Expression, expectedApproveExpression) {
 		return nil, types.ErrApproveExpressionNotMatched
 	}
 
-	if rejectRule.Expression.String() != expectedRejectExpression.String() {
+	if !reflect.DeepEqual(rejectRule.Expression, expectedRejectExpression) {
 		return nil, types.ErrRejectExpressionNotMatched
 	}
 
@@ -239,20 +238,7 @@ func (k Keeper) AddAction(ctx context.Context, creator string, msg sdk.Msg, time
 		return nil, err
 	}
 
-	mentions := approveMentions
-
-	approveMentionsSet := make(map[string]struct{})
-
-	for _, approveMention := range approveMentions {
-		approveMentionsSet[approveMention] = struct{}{}
-	}
-
-	for _, rejectMention := range rejectMentions {
-		_, exists := approveMentionsSet[rejectMention]
-		if !exists {
-			mentions = append(mentions, rejectMention)
-		}
-	}
+	mentions := mergeMentions(approveMentions, rejectMentions)
 
 	// update the rule of this Action with the preprocessed expression
 	// todo: should be removed with removing Rule field in Action
@@ -314,3 +300,22 @@ func (k Keeper) validateActionMsgSigners(msg sdk.Msg) error {
 
 	return nil
 }
+
+func mergeMentions(approveMentions []string, rejectMentions []string) []string {
+	mentions := approveMentions
+
+	approveMentionsSet := make(map[string]struct{})
+
+	for _, approveMention := range approveMentions {
+		approveMentionsSet[approveMention] = struct{}{}
+	}
+
+	for _, rejectMention := range rejectMentions {
+		_, exists := approveMentionsSet[rejectMention]
+		if !exists {
+			mentions = append(mentions, rejectMention)
+		}
+	}
+
+	return mentions
+}
diff --git a/warden/x/warden/keeper/rules.go b/warden/x/warden/keeper/rules.go
index 22771a23e..d43f93b70 100644
--- a/warden/x/warden/keeper/rules.go
+++ b/warden/x/warden/keeper/rules.go
@@ -224,7 +224,9 @@ func (k Keeper) getRejectNewKeyRequestRule(ctx context.Context, space v1beta3.Sp
 }
 
 func (k Keeper) getApproveUpdateKeyRule(ctx context.Context, space v1beta3.Space, key v1beta3.Key) (acttypes.Rule, error) {
-	if space.ApproveSignRuleId > 0 {
+	if key.ApproveRuleId > 0 {
+		return k.actKeeper.GetRule(ctx, key.ApproveRuleId)
+	} else if space.ApproveSignRuleId > 0 {
 		return k.actKeeper.GetRule(ctx, space.ApproveSignRuleId)
 	} else {
 		return space.RuleApproveUpdateKey(), nil
@@ -232,7 +234,9 @@ func (k Keeper) getApproveUpdateKeyRule(ctx context.Context, space v1beta3.Space
 }
 
 func (k Keeper) getRejectUpdateKeyRule(ctx context.Context, space v1beta3.Space, key v1beta3.Key) (acttypes.Rule, error) {
-	if space.RejectSignRuleId > 0 {
+	if key.ApproveRuleId > 0 {
+		return k.actKeeper.GetRule(ctx, key.RejectRuleId)
+	} else if space.RejectSignRuleId > 0 {
 		return k.actKeeper.GetRule(ctx, space.RejectSignRuleId)
 	} else {
 		return space.RuleRejectUpdateKey(), nil