fix replacement and signing process

nkitlabs · nkitlabs · commit 3b85e7e3cda6 · 2024-06-14T10:48:43.000+07:00
diff --git a/x/bandtss/keeper/group.go b/x/bandtss/keeper/group.go
@@ -49,11 +49,8 @@ func (k Keeper) CreateGroupReplacement(
 	}
 
 	// Execute the handler to process the replacement request.
-	msg, err := k.tssKeeper.ConvertContentToBytes(ctx, types.NewReplaceGroupSignatureOrder(newGroup.PubKey))
-	if err != nil {
-		return 0, err
-	}
-	signing, err := k.tssKeeper.CreateSigning(ctx, currentGroup, msg)
+	content := types.NewReplaceGroupSignatureOrder(newGroup.PubKey)
+	signing, err := k.tssKeeper.CreateSigning(ctx, currentGroup, content)
 	if err != nil {
 		return 0, err
 	}
diff --git a/x/bandtss/keeper/group_test.go b/x/bandtss/keeper/group_test.go
@@ -37,8 +37,7 @@ func TestSuccessCreateGroupReplacement(t *testing.T) {
 	k.SetCurrentGroupID(ctx, currentGroupID)
 	s.MockTSSKeeper.EXPECT().GetGroup(ctx, currentGroupID).Return(currentGroup, nil).AnyTimes()
 	s.MockTSSKeeper.EXPECT().GetGroup(ctx, newGroupID).Return(newGroup, nil).AnyTimes()
-	s.MockTSSKeeper.EXPECT().ConvertContentToBytes(ctx, types.NewReplaceGroupSignatureOrder(newGroup.PubKey)).Return([]byte("test-msg"), nil)
-	s.MockTSSKeeper.EXPECT().CreateSigning(ctx, currentGroup, []byte("test-msg")).Return(expectSigning, nil)
+	s.MockTSSKeeper.EXPECT().CreateSigning(ctx, currentGroup, types.NewReplaceGroupSignatureOrder(newGroup.PubKey)).Return(expectSigning, nil)
 
 	signingID, err := k.CreateGroupReplacement(ctx, newGroupID, execTime)
 	require.NoError(t, err)
diff --git a/x/bandtss/keeper/member.go b/x/bandtss/keeper/member.go
@@ -64,7 +64,7 @@ func (k Keeper) SetLastActive(ctx sdk.Context, address sdk.AccAddress) error {
 	return nil
 }
 
-// DeactivateMember flags is_active to false. This function will panic if the given address
+// DeactivateMember flags is_active to false. This function will return error if the given address
 // isn't the member of the current group.
 func (k Keeper) DeactivateMember(ctx sdk.Context, address sdk.AccAddress) error {
 	member, err := k.GetMember(ctx, address)
diff --git a/x/bandtss/keeper/msg_server.go b/x/bandtss/keeper/msg_server.go
@@ -79,8 +79,14 @@ func (k msgServer) RequestSignature(
 		return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid address: %s", err)
 	}
 
+	content := req.GetContent()
+	if content.OrderRoute() == types.RouterKey && content.OrderType() == types.ReplaceGroupPath {
+		return nil, types.ErrInvalidRequestSignature.Wrapf(
+			"invalid request order route: %s order type: %s", content.OrderRoute(), content.OrderType())
+	}
+
 	// Execute the handler to process the request.
-	_, err = k.HandleCreateSigning(ctx, req.GetContent(), feePayer, req.FeeLimit)
+	_, err = k.HandleCreateSigning(ctx, content, feePayer, req.FeeLimit)
 	if err != nil {
 		return nil, err
 	}
diff --git a/x/bandtss/keeper/signing.go b/x/bandtss/keeper/signing.go
@@ -132,12 +132,6 @@ func (k Keeper) HandleCreateSigning(
 	sender sdk.AccAddress,
 	feeLimit sdk.Coins,
 ) (types.SigningID, error) {
-	// Execute the handler to process the request.
-	msg, err := k.tssKeeper.ConvertContentToBytes(ctx, content)
-	if err != nil {
-		return 0, err
-	}
-
 	currentGroupID := k.GetCurrentGroupID(ctx)
 	if currentGroupID == 0 {
 		return 0, types.ErrNoActiveGroup
@@ -169,7 +163,7 @@ func (k Keeper) HandleCreateSigning(
 		}
 	}
 
-	currentGroupSigning, err := k.tssKeeper.CreateSigning(ctx, currentGroup, msg)
+	currentGroupSigning, err := k.tssKeeper.CreateSigning(ctx, currentGroup, content)
 	if err != nil {
 		return 0, err
 	}
@@ -181,7 +175,7 @@ func (k Keeper) HandleCreateSigning(
 			return 0, err
 		}
 
-		replacingGroupSigning, err := k.tssKeeper.CreateSigning(ctx, replacingGroup, msg)
+		replacingGroupSigning, err := k.tssKeeper.CreateSigning(ctx, replacingGroup, content)
 		if err != nil {
 			return 0, err
 		}
diff --git a/x/bandtss/keeper/signing_test.go b/x/bandtss/keeper/signing_test.go
@@ -32,7 +32,6 @@ func TestHandleCreateSigning(t *testing.T) {
 	err := k.SetParams(ctx, params)
 	require.NoError(t, err)
 
-	s.MockTSSKeeper.EXPECT().ConvertContentToBytes(ctx, content).Return([]byte("test"), nil).AnyTimes()
 	s.MockTSSKeeper.EXPECT().GetGroup(ctx, currentGroupID).Return(currentGroup, nil).AnyTimes()
 
 	type input struct {
@@ -59,7 +58,7 @@ func TestHandleCreateSigning(t *testing.T) {
 				s.MockTSSKeeper.EXPECT().CreateSigning(
 					ctx,
 					currentGroup,
-					[]byte("test"),
+					content,
 				).Return(expectCurrentGroupSigning, nil)
 				s.MockBankKeeper.EXPECT().SendCoinsFromAccountToModule(
 					ctx,
@@ -102,7 +101,7 @@ func TestHandleCreateSigning(t *testing.T) {
 				s.MockTSSKeeper.EXPECT().CreateSigning(
 					ctx,
 					currentGroup,
-					[]byte("test"),
+					content,
 				).Return(expectCurrentGroupSigning, nil)
 			},
 			postCheck: func() {
@@ -163,12 +162,12 @@ func TestHandleCreateSigning(t *testing.T) {
 				s.MockTSSKeeper.EXPECT().CreateSigning(
 					ctx,
 					currentGroup,
-					[]byte("test"),
+					content,
 				).Return(expectCurrentGroupSigning, nil)
 				s.MockTSSKeeper.EXPECT().CreateSigning(
 					ctx,
 					replaceGroup,
-					[]byte("test"),
+					content,
 				).Return(expectReplaceGroupSigning, nil)
 				s.MockBankKeeper.EXPECT().SendCoinsFromAccountToModule(
 					ctx,
@@ -223,7 +222,7 @@ func TestHandleCreateSigning(t *testing.T) {
 				s.MockTSSKeeper.EXPECT().CreateSigning(
 					ctx,
 					currentGroup,
-					[]byte("test"),
+					content,
 				).Return(expectCurrentGroupSigning, nil)
 				s.MockBankKeeper.EXPECT().SendCoinsFromAccountToModule(
 					ctx,
diff --git a/x/bandtss/testutil/mock_expected_keepers.go b/x/bandtss/testutil/mock_expected_keepers.go
diff --git a/x/bandtss/types/errors.go b/x/bandtss/types/errors.go
@@ -16,4 +16,5 @@ var (
 	ErrMemberAlreadyActive     = errorsmod.Register(ModuleName, 11, "member already active")
 	ErrMemberDuplicate         = errorsmod.Register(ModuleName, 12, "duplicated member found within the list")
 	ErrInvalidSigningThreshold = errorsmod.Register(ModuleName, 13, "invalid signing threshold number")
+	ErrInvalidRequestSignature = errorsmod.Register(ModuleName, 14, "request signature is invalid")
 )
diff --git a/x/bandtss/types/expected_keepers.go b/x/bandtss/types/expected_keepers.go
@@ -91,7 +91,7 @@ type TSSKeeper interface {
 	CreateSigning(
 		ctx sdk.Context,
 		group tsstypes.Group,
-		message []byte,
+		content tsstypes.Content,
 	) (*tsstypes.Signing, error)
 
 	MustGetMembers(ctx sdk.Context, groupID tss.GroupID) []tsstypes.Member
@@ -105,6 +105,5 @@ type TSSKeeper interface {
 	GetPenalizedMembersExpiredSigning(ctx sdk.Context, signing tsstypes.Signing) ([]sdk.AccAddress, error)
 
 	GetSigning(ctx sdk.Context, signingID tss.SigningID) (tsstypes.Signing, error)
-	ConvertContentToBytes(ctx sdk.Context, content tsstypes.Content) ([]byte, error)
 	GetSigningResult(ctx sdk.Context, signingID tss.SigningID) (*tsstypes.SigningResult, error)
 }
diff --git a/x/oracle/client/cli/tx.go b/x/oracle/client/cli/tx.go
@@ -695,7 +695,7 @@ func GetCmdRequestSignature() *cobra.Command {
 		Long: strings.TrimSpace(
 			fmt.Sprintf(`Request signature from request id.
 Example:
-$ %s tx bandtss request-signature oracle-result 1 --fee-limit 10uband
+$ %s tx bandtss request-signature oracle-result 1 2 --fee-limit 10uband
 `,
 				version.AppName,
 			),
diff --git a/x/tss/keeper/signing.go b/x/tss/keeper/signing.go
@@ -373,8 +373,13 @@ func (k Keeper) ConvertContentToBytes(
 func (k Keeper) CreateSigning(
 	ctx sdk.Context,
 	group types.Group,
-	message []byte,
+	content types.Content,
 ) (*types.Signing, error) {
+	message, err := k.ConvertContentToBytes(ctx, content)
+	if err != nil {
+		return nil, err
+	}
+
 	// assigned members within the context of the group.
 	assignedMembers, err := k.AssignMembers(ctx, group, message)
 	if err != nil {
diff --git a/x/tss/keeper/signing_test.go b/x/tss/keeper/signing_test.go
@@ -438,12 +438,7 @@ func (s *KeeperTestSuite) TestCreateSigning() {
 
 	// Create a new request for the request signature
 	content := types.NewTextSignatureOrder([]byte("example"))
-
-	// execute HandleRequestSign
-	msg, err := k.ConvertContentToBytes(ctx, content)
-	s.Require().NoError(err)
-
-	signing, err := k.CreateSigning(ctx, group, msg)
+	signing, err := k.CreateSigning(ctx, group, content)
 	s.Require().NoError(err)
 
 	s.Require().NoError(err)
diff --git a/x/tss/types/keys.go b/x/tss/types/keys.go
@@ -69,7 +69,7 @@ var (
 	// Round2InfoCountStoreKeyPrefix is the key that keeps the round 2 data count.
 	Round2InfoCountStoreKeyPrefix = []byte{0x08}
 
-	// ComplainWithStatusStoreKeyPrefix is the key that keeps complain with status.
+	// ComplainsWithStatusStoreKeyPrefix is the key that keeps complain with status.
 	ComplainsWithStatusStoreKeyPrefix = []byte{0x09}
 
 	// ConfirmComplainCountStoreKeyPrefix is the key for keep track of the progress of round 3.

Original file line number	Diff line number	Diff line change
`@@ -49,11 +49,8 @@ func (k Keeper) CreateGroupReplacement(`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	`// Execute the handler to process the replacement request.`
`52`		`- msg, err := k.tssKeeper.ConvertContentToBytes(ctx, types.NewReplaceGroupSignatureOrder(newGroup.PubKey))`
`53`		`- if err != nil {`
`54`		`- return 0, err`
`55`		`- }`
`56`		`- signing, err := k.tssKeeper.CreateSigning(ctx, currentGroup, msg)`
	`52`	`+ content := types.NewReplaceGroupSignatureOrder(newGroup.PubKey)`
	`53`	`+ signing, err := k.tssKeeper.CreateSigning(ctx, currentGroup, content)`
`57`	`54`	`if err != nil {`
`58`	`55`	`return 0, err`
`59`	`56`	`}`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ func (k Keeper) SetLastActive(ctx sdk.Context, address sdk.AccAddress) error {`
`64`	`64`	`return nil`
`65`	`65`	`}`
`66`	`66`
`67`		`-// DeactivateMember flags is_active to false. This function will panic if the given address`
	`67`	`+// DeactivateMember flags is_active to false. This function will return error if the given address`
`68`	`68`	`// isn't the member of the current group.`
`69`	`69`	`func (k Keeper) DeactivateMember(ctx sdk.Context, address sdk.AccAddress) error {`
`70`	`70`	`member, err := k.GetMember(ctx, address)`
Original file line number	Diff line number	Diff line change
`@@ -132,12 +132,6 @@ func (k Keeper) HandleCreateSigning(`
`132`	`132`	`sender sdk.AccAddress,`
`133`	`133`	`feeLimit sdk.Coins,`
`134`	`134`	`) (types.SigningID, error) {`
`135`		`- // Execute the handler to process the request.`
`136`		`- msg, err := k.tssKeeper.ConvertContentToBytes(ctx, content)`
`137`		`- if err != nil {`
`138`		`- return 0, err`
`139`		`- }`
`140`		`-`
`141`	`135`	`currentGroupID := k.GetCurrentGroupID(ctx)`
`142`	`136`	`if currentGroupID == 0 {`
`143`	`137`	`return 0, types.ErrNoActiveGroup`
`@@ -169,7 +163,7 @@ func (k Keeper) HandleCreateSigning(`
`169`	`163`	`}`
`170`	`164`	`}`
`171`	`165`
`172`		`- currentGroupSigning, err := k.tssKeeper.CreateSigning(ctx, currentGroup, msg)`
	`166`	`+ currentGroupSigning, err := k.tssKeeper.CreateSigning(ctx, currentGroup, content)`
`173`	`167`	`if err != nil {`
`174`	`168`	`return 0, err`
`175`	`169`	`}`
`@@ -181,7 +175,7 @@ func (k Keeper) HandleCreateSigning(`
`181`	`175`	`return 0, err`
`182`	`176`	`}`
`183`	`177`
`184`		`- replacingGroupSigning, err := k.tssKeeper.CreateSigning(ctx, replacingGroup, msg)`
	`178`	`+ replacingGroupSigning, err := k.tssKeeper.CreateSigning(ctx, replacingGroup, content)`
`185`	`179`	`if err != nil {`
`186`	`180`	`return 0, err`
`187`	`181`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,4 +16,5 @@ var (`
`16`	`16`	`ErrMemberAlreadyActive = errorsmod.Register(ModuleName, 11, "member already active")`
`17`	`17`	`ErrMemberDuplicate = errorsmod.Register(ModuleName, 12, "duplicated member found within the list")`
`18`	`18`	`ErrInvalidSigningThreshold = errorsmod.Register(ModuleName, 13, "invalid signing threshold number")`
	`19`	`+ ErrInvalidRequestSignature = errorsmod.Register(ModuleName, 14, "request signature is invalid")`
`19`	`20`	`)`