Problem: MigrateGravityContract does not delete all ethereum signatur…

…es (PeggyJV#145)

module/x/gravity/keeper/keeper.go

@@ -652,28 +652,27 @@ func (k Keeper) IterateEthereumHeightVotes(ctx sdk.Context, cb func(val sdk.ValA
 // This just does keeper state cleanup if a new gravity contract has been deployed
 func (k Keeper) MigrateGravityContract(ctx sdk.Context, newBridgeAddress string, bridgeDeploymentHeight uint64) {
 	// Delete Any Outgoing TXs.
-
 	prefixStoreOtx := prefix.NewStore(ctx.KVStore(k.storeKey), []byte{types.OutgoingTxKey})
 	iterOtx := prefixStoreOtx.ReverseIterator(nil, nil)
 	defer iterOtx.Close()
+	var oxtToDeleteKeys [][]byte
 	for ; iterOtx.Valid(); iterOtx.Next() {
+		oxtToDeleteKeys = append(oxtToDeleteKeys, iterOtx.Key())
+	}
+	for _, key := range oxtToDeleteKeys {
+		prefixStoreOtx.Delete(key)
+	}
 
-		var any cdctypes.Any
-		k.cdc.MustUnmarshal(iterOtx.Value(), &any)
-		var otx types.OutgoingTx
-		if err := k.cdc.UnpackAny(&any, &otx); err != nil {
-			panic(err)
-		}
-		// Delete any partial Eth Signatures handging around
-		prefixStoreSig := prefix.NewStore(ctx.KVStore(k.storeKey), append([]byte{types.EthereumSignatureKey}, otx.GetStoreIndex()...))
-		iterSig := prefixStoreSig.Iterator(nil, nil)
-		defer iterSig.Close()
-
-		for ; iterSig.Valid(); iterSig.Next() {
-			prefixStoreSig.Delete(iterSig.Key())
-		}
-
-		prefixStoreOtx.Delete(iterOtx.Key())
+	// Delete all ethereum signature for all Outgoing TXs.
+	prefixStoreSig := prefix.NewStore(ctx.KVStore(k.storeKey), []byte{types.EthereumSignatureKey})
+	iterSig := prefixStoreSig.Iterator(nil, nil)
+	defer iterSig.Close()
+	var sigToDeleteKeys [][]byte
+	for ; iterSig.Valid(); iterSig.Next() {
+		sigToDeleteKeys = append(sigToDeleteKeys, iterSig.Key())
+	}
+	for _, key := range sigToDeleteKeys {
+		prefixStoreSig.Delete(key)
 	}
 
 	// Reset the last observed signer set nonce
@@ -700,8 +699,12 @@ func (k Keeper) MigrateGravityContract(ctx sdk.Context, newBridgeAddress string,
 	prefixStoreEthereumEvent := prefix.NewStore(ctx.KVStore(k.storeKey), []byte{types.EthereumEventVoteRecordKey})
 	iterEvent := prefixStoreEthereumEvent.Iterator(nil, nil)
 	defer iterEvent.Close()
+	var eventToDeleteKeys [][]byte
 	for ; iterEvent.Valid(); iterEvent.Next() {
-		prefixStoreEthereumEvent.Delete(iterEvent.Key())
+		eventToDeleteKeys = append(eventToDeleteKeys, iterEvent.Key())
+	}
+	for _, key := range eventToDeleteKeys {
+		prefixStoreEthereumEvent.Delete(key)
 	}
 
 	// Set the Last oberved Ethereum Blockheight to zero

module/x/gravity/keeper/keeper_test.go

@@ -551,6 +551,28 @@ func TestKeeper_Migration(t *testing.T) {
 		},
 	}
 
+	// assume it is an expired event
+	stce2 := &types.SendToCosmosEvent{
+		EventNonce:     3,
+		TokenContract:  EthAddrs[0].Hex(),
+		EthereumSender: EthAddrs[0].Hex(),
+		CosmosReceiver: AccAddrs[0].String(),
+		EthereumHeight: 10,
+		Amount:         sdk.NewInt(1000000),
+	}
+	stcea2, err := types.PackEvent(stce2)
+	require.NoError(t, err)
+
+	evr21 := &types.EthereumEventVoteRecord{
+		Event: stcea2,
+		Votes: []string{
+			ValAddrs[0].String(),
+			ValAddrs[1].String(),
+			ValAddrs[2].String(),
+		},
+		Accepted: false,
+	}
+
 	//Put an outgoing transaction into the system
 
 	var (
@@ -587,6 +609,10 @@ func TestKeeper_Migration(t *testing.T) {
 	gk.setEthereumEventVoteRecord(ctx, cctxe.GetEventNonce(), cctxe.Hash(), evr2)
 	gk.setLastObservedEventNonce(ctx, cctxe.GetEventNonce())
 
+	// set only the event vote record of the expired event to simulate the bug from (https://github.com/crypto-org-chain/gravity-bridge/issues/112)
+	gk.setEthereumEventVoteRecord(ctx, stce2.GetEventNonce(), stce2.Hash(), evr21)
+	gk.setLastObservedEventNonce(ctx, cctxe.GetEventNonce())
+
 	stored := gk.GetEthereumEventVoteRecord(ctx, stce.GetEventNonce(), stce.Hash())
 	require.NotNil(t, stored)
 
@@ -643,6 +669,9 @@ func TestKeeper_Migration(t *testing.T) {
 	stored2AfterMigrate := gk.GetEthereumEventVoteRecord(ctx, cctxe.GetEventNonce(), cctxe.Hash())
 	require.Nil(t, stored2AfterMigrate)
 
+	stored3AfterMigrate := gk.GetEthereumEventVoteRecord(ctx, stce2.GetEventNonce(), stce2.Hash())
+	require.Nil(t, stored3AfterMigrate)
+
 	nonce2 := gk.GetLastObservedEventNonce(ctx)
 	require.Equal(t, uint64(0), nonce2)