fix: remove old SP key indexes on edit

x/sp/keeper/msg_server.go

@@ -1,6 +1,7 @@
 package keeper
 
 import (
+	"bytes"
 	"context"
 	"encoding/hex"
 	"time"
@@ -189,6 +190,13 @@ func (k msgServer) EditStorageProvider(goCtx context.Context, msg *types.MsgEdit
 		return nil, types.ErrStorageProviderNotFound
 	}
 
+	// Capture the current operational identities before mutation so their stale
+	// secondary-index entries can be revoked once a value is rotated.
+	oldSealAddr := sp.SealAddress
+	oldApprovalAddr := sp.ApprovalAddress
+	oldGcAddr := sp.GcAddress
+	oldBlsKey := sp.BlsKey
+
 	changed := false
 
 	// replace endpoint
@@ -208,18 +216,27 @@ func (k msgServer) EditStorageProvider(goCtx context.Context, msg *types.MsgEdit
 
 	if msg.SealAddress != "" {
 		sealAcc := sdk.MustAccAddressFromHex(msg.SealAddress)
+		if existing, exists := k.GetStorageProviderBySealAddr(ctx, sealAcc); exists && existing.Id != sp.Id {
+			return nil, types.ErrStorageProviderSealAddrExists
+		}
 		sp.SealAddress = sealAcc.String()
 		changed = true
 	}
 
 	if msg.ApprovalAddress != "" {
 		approvalAcc := sdk.MustAccAddressFromHex(msg.ApprovalAddress)
+		if existing, exists := k.GetStorageProviderByApprovalAddr(ctx, approvalAcc); exists && existing.Id != sp.Id {
+			return nil, types.ErrStorageProviderApprovalAddrExists
+		}
 		sp.ApprovalAddress = approvalAcc.String()
 		changed = true
 	}
 
 	if msg.GcAddress != "" {
 		gcAcc := sdk.MustAccAddressFromHex(msg.GcAddress)
+		if existing, exists := k.GetStorageProviderByGcAddr(ctx, gcAcc); exists && existing.Id != sp.Id {
+			return nil, types.ErrStorageProviderGcAddrExists
+		}
 		sp.GcAddress = gcAcc.String()
 		changed = true
 	}
@@ -233,6 +250,9 @@ func (k msgServer) EditStorageProvider(goCtx context.Context, msg *types.MsgEdit
 		if err != nil || len(blsPk) != sdk.BLSPubKeyLength {
 			return nil, types.ErrStorageProviderInvalidBlsKey
 		}
+		if existing, exists := k.GetStorageProviderByBlsKey(ctx, blsPk); exists && existing.Id != sp.Id {
+			return nil, types.ErrStorageProviderBlsKeyExists
+		}
 		if err = k.checkBlsProof(blsPk, msg.BlsProof); err != nil {
 			return nil, err
 		}
@@ -251,6 +271,22 @@ func (k msgServer) EditStorageProvider(goCtx context.Context, msg *types.MsgEdit
 	k.SetStorageProviderByGcAddr(ctx, sp)
 	k.SetStorageProviderByBlsKey(ctx, sp)
 
+	// Revoke the stale secondary indexes so a rotated operational key no longer
+	// resolves to this SP in the privileged x/storage authorization paths.
+	store := ctx.KVStore(k.storeKey)
+	if oldSealAddr != sp.SealAddress {
+		store.Delete(types.GetStorageProviderBySealAddrKey(sdk.MustAccAddressFromHex(oldSealAddr)))
+	}
+	if oldApprovalAddr != sp.ApprovalAddress {
+		store.Delete(types.GetStorageProviderByApprovalAddrKey(sdk.MustAccAddressFromHex(oldApprovalAddr)))
+	}
+	if oldGcAddr != sp.GcAddress {
+		store.Delete(types.GetStorageProviderByGcAddrKey(sdk.MustAccAddressFromHex(oldGcAddr)))
+	}
+	if !bytes.Equal(oldBlsKey, sp.BlsKey) {
+		store.Delete(types.GetStorageProviderByBlsKeyKey(oldBlsKey))
+	}
+
 	if err := ctx.EventManager().EmitTypedEvents(&types.EventEditStorageProvider{
 		SpId:               sp.Id,
 		SpAddress:          operatorAcc.String(),

x/sp/keeper/msg_server_edit_revoke_test.go

@@ -0,0 +1,119 @@
+package keeper_test
+
+import (
+	"encoding/hex"
+
+	sdkmath "cosmossdk.io/math"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/bnb-chain/greenfield/testutil/sample"
+	"github.com/bnb-chain/greenfield/x/sp/types"
+)
+
+// TestEditStorageProviderRevokesStaleOperationalIndexes verifies that rotating an
+// SP's seal/gc/approval/bls identities through MsgEditStorageProvider revokes the
+// old secondary-index entries, so a leaked or retired key no longer resolves to the
+// SP in the privileged x/storage authorization paths.
+func (s *KeeperTestSuite) TestEditStorageProviderRevokesStaleOperationalIndexes() {
+	oldSeal := sample.RandAccAddress()
+	oldApproval := sample.RandAccAddress()
+	oldGC := sample.RandAccAddress()
+	oldBlsKey, _ := sample.RandBlsPubKeyAndBlsProof()
+
+	sp, err := types.NewStorageProvider(
+		1,
+		sample.RandAccAddress(),
+		sample.RandAccAddress(),
+		oldSeal,
+		oldApproval,
+		oldGC,
+		sample.RandAccAddress(),
+		sdkmath.NewInt(1),
+		"https://sp.example",
+		types.NewDescription("sp", "", "", ""),
+		oldBlsKey,
+	)
+	s.Require().NoError(err)
+
+	s.spKeeper.SetStorageProvider(s.ctx, &sp)
+	s.spKeeper.SetStorageProviderByOperatorAddr(s.ctx, &sp)
+	s.spKeeper.SetStorageProviderByFundingAddr(s.ctx, &sp)
+	s.spKeeper.SetStorageProviderBySealAddr(s.ctx, &sp)
+	s.spKeeper.SetStorageProviderByApprovalAddr(s.ctx, &sp)
+	s.spKeeper.SetStorageProviderByGcAddr(s.ctx, &sp)
+	s.spKeeper.SetStorageProviderByBlsKey(s.ctx, &sp)
+
+	newSeal := sample.RandAccAddress()
+	newApproval := sample.RandAccAddress()
+	newGC := sample.RandAccAddress()
+	newBlsKey, newBlsProof := sample.RandBlsPubKeyAndBlsProof()
+
+	_, err = s.msgServer.EditStorageProvider(sdk.WrapSDKContext(s.ctx), &types.MsgEditStorageProvider{
+		SpAddress:       sp.OperatorAddress,
+		SealAddress:     newSeal.String(),
+		ApprovalAddress: newApproval.String(),
+		GcAddress:       newGC.String(),
+		BlsKey:          newBlsKey,
+		BlsProof:        newBlsProof,
+	})
+	s.Require().NoError(err)
+
+	// Old identities must no longer resolve after rotation.
+	_, found := s.spKeeper.GetStorageProviderBySealAddr(s.ctx, oldSeal)
+	s.Require().False(found, "stale seal key should be revoked after rotation")
+	_, found = s.spKeeper.GetStorageProviderByApprovalAddr(s.ctx, oldApproval)
+	s.Require().False(found, "stale approval key should be revoked after rotation")
+	_, found = s.spKeeper.GetStorageProviderByGcAddr(s.ctx, oldGC)
+	s.Require().False(found, "stale gc key should be revoked after rotation")
+	oldBlsBz, _ := hex.DecodeString(oldBlsKey)
+	_, found = s.spKeeper.GetStorageProviderByBlsKey(s.ctx, oldBlsBz)
+	s.Require().False(found, "stale bls key should be revoked after rotation")
+
+	// New identities must resolve to the same SP.
+	got, found := s.spKeeper.GetStorageProviderBySealAddr(s.ctx, newSeal)
+	s.Require().True(found)
+	s.Require().Equal(sp.Id, got.Id)
+	got, found = s.spKeeper.GetStorageProviderByGcAddr(s.ctx, newGC)
+	s.Require().True(found)
+	s.Require().Equal(sp.Id, got.Id)
+	newBlsBz, _ := hex.DecodeString(newBlsKey)
+	got, found = s.spKeeper.GetStorageProviderByBlsKey(s.ctx, newBlsBz)
+	s.Require().True(found)
+	s.Require().Equal(sp.Id, got.Id)
+}
+
+// TestEditStorageProviderRejectsIdentityOwnedByAnotherSP verifies an SP cannot
+// hijack another SP's operational identity through an edit.
+func (s *KeeperTestSuite) TestEditStorageProviderRejectsIdentityOwnedByAnotherSP() {
+	victimSeal := sample.RandAccAddress()
+	victimBls, _ := sample.RandBlsPubKeyAndBlsProof()
+	victim, err := types.NewStorageProvider(
+		1, sample.RandAccAddress(), sample.RandAccAddress(),
+		victimSeal, sample.RandAccAddress(), sample.RandAccAddress(), sample.RandAccAddress(),
+		sdkmath.NewInt(1), "https://victim.example", types.NewDescription("victim", "", "", ""), victimBls,
+	)
+	s.Require().NoError(err)
+	s.spKeeper.SetStorageProvider(s.ctx, &victim)
+	s.spKeeper.SetStorageProviderBySealAddr(s.ctx, &victim)
+
+	attackerBls, _ := sample.RandBlsPubKeyAndBlsProof()
+	attacker, err := types.NewStorageProvider(
+		2, sample.RandAccAddress(), sample.RandAccAddress(),
+		sample.RandAccAddress(), sample.RandAccAddress(), sample.RandAccAddress(), sample.RandAccAddress(),
+		sdkmath.NewInt(1), "https://attacker.example", types.NewDescription("attacker", "", "", ""), attackerBls,
+	)
+	s.Require().NoError(err)
+	s.spKeeper.SetStorageProvider(s.ctx, &attacker)
+	s.spKeeper.SetStorageProviderByOperatorAddr(s.ctx, &attacker)
+
+	_, err = s.msgServer.EditStorageProvider(sdk.WrapSDKContext(s.ctx), &types.MsgEditStorageProvider{
+		SpAddress:   attacker.OperatorAddress,
+		SealAddress: victimSeal.String(),
+	})
+	s.Require().ErrorIs(err, types.ErrStorageProviderSealAddrExists)
+
+	// Victim's seal index must still point to the victim.
+	got, found := s.spKeeper.GetStorageProviderBySealAddr(s.ctx, victimSeal)
+	s.Require().True(found)
+	s.Require().Equal(victim.Id, got.Id)
+}

x/sp/keeper/sp.go

@@ -140,7 +140,7 @@ func (k Keeper) Exit(ctx sdk.Context, sp *types.StorageProvider) error {
 	store.Delete(types.GetStorageProviderByApprovalAddrKey(sdk.MustAccAddressFromHex(sp.ApprovalAddress)))
 	store.Delete(types.GetStorageProviderByGcAddrKey(sdk.MustAccAddressFromHex(sp.GcAddress)))
 	store.Delete(types.GetStorageProviderKey(k.spSequence.EncodeSequence(sp.Id)))
-	store.Delete(types.GetStorageProviderByBlsKeyKey(types.GetStorageProviderByBlsKeyKey(sp.GetBlsKey())))
+	store.Delete(types.GetStorageProviderByBlsKeyKey(sp.GetBlsKey()))
 	return nil
 }