refactor: Adding improvements from the official Gravity Bridge

x/peggy/abci.go

@@ -1,6 +1,7 @@
 package peggy
 
 import (
+	"fmt"
 	"sort"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -157,58 +158,83 @@ func (h *BlockHandler) cleanupTimedOutBatches(ctx sdk.Context) {
 	}
 }
 
+// prepValsetConfirms loads all confirmations into a hashmap indexed by validatorAddr
+// reducing the lookup time dramatically and separating out the task of looking up
+// the orchestrator for each validator.
+func prepValsetConfirms(ctx sdk.Context, k keeper.Keeper, nonce uint64) map[string]*types.MsgValsetConfirm {
+	confirms := k.GetValsetConfirms(ctx, nonce)
+	ret := make(map[string]*types.MsgValsetConfirm)
+	for _, confirm := range confirms {
+		// TODO this presents problems for delegate key rotation see issue #344
+		confVal, err := sdk.AccAddressFromBech32(confirm.Orchestrator)
+		if err != nil {
+			panic(fmt.Sprintf("invalid claim confirmation address: %s", confirm.Orchestrator))
+		}
+		val, foundValidator := k.GetOrchestratorValidator(ctx, confVal)
+		if !foundValidator {
+			panic(fmt.Sprintf("failed to find validator by orchestrator address: %s", confVal))
+		}
+		ret[val.String()] = confirm
+	}
+	return ret
+}
+
 func (h *BlockHandler) valsetSlashing(ctx sdk.Context, params *types.Params) {
-	maxHeight := uint64(0)
 
 	// don't slash in the beginning before there aren't even SignedValsetsWindow blocks yet
-	if uint64(ctx.BlockHeight()) > params.SignedValsetsWindow {
-		maxHeight = uint64(ctx.BlockHeight()) - params.SignedValsetsWindow
-	} else {
-		// we can't slash anyone if SignedValsetWindow blocks have not passed
+	if uint64(ctx.BlockHeight()) <= params.SignedValsetsWindow {
 		return
 	}
 
-	unslashedValsets := h.k.GetUnslashedValsets(ctx, maxHeight)
+	unslashedValsets := h.k.GetUnslashedValsets(ctx, params.SignedValsetsWindow)
+
+	currentBondedSet := h.k.StakingKeeper.GetBondedValidatorsByPower(ctx)
 
 	// unslashedValsets are sorted by nonce in ASC order
 	for _, vs := range unslashedValsets {
-		confirms := h.k.GetValsetConfirms(ctx, vs.Nonce)
+		confirms := prepValsetConfirms(ctx, h.k, vs.Nonce)
 
 		// SLASH BONDED VALIDATORS who didn't attest valset request
-		currentBondedSet := h.k.StakingKeeper.GetBondedValidatorsByPower(ctx)
-
 		for _, val := range currentBondedSet {
-			consAddr, _ := val.GetConsAddr()
+			consAddr, err := val.GetConsAddr()
+			if err != nil {
+				panic("Failed to get validator consensus addr")
+			}
 			valSigningInfo, exist := h.k.SlashingKeeper.GetValidatorSigningInfo(ctx, consAddr)
 
-			//  Slash validator ONLY if he joined after valset is created
-			if exist && valSigningInfo.StartHeight < int64(vs.Height) {
+			// slash validator ONLY if it joined before the valset was created
+			startedBeforeValsetCreated := valSigningInfo.StartHeight < int64(vs.Height)
+
+			if exist && startedBeforeValsetCreated {
 				// Check if validator has confirmed valset or not
-				found := false
-				for _, conf := range confirms {
-					ethAddress, exists := h.k.GetEthAddressByValidator(ctx, val.GetOperator())
-					// This may have an issue if the validator changes their eth address
-					// TODO this presents problems for delegate key rotation see issue #344
-					if exists && common.HexToAddress(conf.EthAddress) == ethAddress {
-						found = true
-						break
-					}
-				}
+				_, found := confirms[val.GetOperator().String()]
+
 				// slash validators for not confirming valsets
 				if !found {
-					cons, _ := val.GetConsAddr()
-					consPower := val.ConsensusPower(h.k.StakingKeeper.PowerReduction(ctx))
-
-					h.k.StakingKeeper.Slash(
-						ctx,
-						cons,
-						ctx.BlockHeight(),
-						consPower,
-						params.SlashFractionValset,
-					)
+					// refresh validator before slashing/jailing
+					val, foundVal := h.k.StakingKeeper.GetValidator(ctx, val.GetOperator())
+					if !foundVal {
+						// this should be impossible, we haven't even progressed a single block since we got the list
+						panic("Validator exited set during endblocker?")
+					}
 
 					if !val.IsJailed() {
-						h.k.StakingKeeper.Jail(ctx, cons)
+						consPower := val.ConsensusPower(h.k.StakingKeeper.PowerReduction(ctx))
+						h.k.StakingKeeper.Slash(
+							ctx,
+							consAddr,
+							ctx.BlockHeight(),
+							consPower,
+							params.SlashFractionValset,
+						)
+						ctx.EventManager().EmitEvent(
+							sdk.NewEvent(
+								sdk.EventTypeMessage,
+								sdk.NewAttribute("ValsetSignatureSlashing", consAddr.String()),
+							),
+						)
+
+						h.k.StakingKeeper.Jail(ctx, consAddr)
 					}
 				}
 			}
@@ -247,9 +273,21 @@ func (h *BlockHandler) valsetSlashing(ctx sdk.Context, params *types.Params) {
 
 					// slash validators for not confirming valsets
 					if !found {
-						consPower := validator.ConsensusPower(h.k.StakingKeeper.PowerReduction(ctx))
+						// refresh validator before slashing/jailing
+						validator, foundVal := h.k.StakingKeeper.GetValidator(ctx, validator.GetOperator())
+						if !foundVal {
+							// this should be impossible, we haven't even progressed a single block since we got the list
+							panic("Validator exited set during endblocker?")
+						}
 
+						consPower := validator.ConsensusPower(h.k.StakingKeeper.PowerReduction(ctx))
 						h.k.StakingKeeper.Slash(ctx, valConsAddr, ctx.BlockHeight(), consPower, params.SlashFractionValset)
+						ctx.EventManager().EmitEvent(
+							sdk.NewEvent(
+								sdk.EventTypeMessage,
+								sdk.NewAttribute("ValsetSignatureSlashing", valConsAddr.String()),
+							),
+						)
 
 						if !validator.IsJailed() {
 							h.k.StakingKeeper.Jail(ctx, valConsAddr)
@@ -264,6 +302,24 @@ func (h *BlockHandler) valsetSlashing(ctx sdk.Context, params *types.Params) {
 	}
 }
 
+// prepBatchConfirms loads all confirmations into a hashmap indexed by validatorAddr
+// reducing the lookup time dramatically and separating out the task of looking up
+// the orchestrator for each validator.
+func prepBatchConfirms(ctx sdk.Context, k keeper.Keeper, batch *types.OutgoingTxBatch) map[string]*types.MsgConfirmBatch {
+	confirms := k.GetBatchConfirmByNonceAndTokenContract(ctx, batch.BatchNonce, common.HexToAddress(batch.TokenContract))
+	ret := make(map[string]*types.MsgConfirmBatch)
+	for _, confirm := range confirms {
+		// TODO this presents problems for delegate key rotation see issue #344
+		confVal, _ := sdk.AccAddressFromBech32(confirm.Orchestrator)
+		val, foundValidator := k.GetOrchestratorValidator(ctx, confVal)
+		if !foundValidator {
+			panic("Confirm from validator we can't identify?")
+		}
+		ret[val.String()] = confirm
+	}
+	return ret
+}
+
 func (h *BlockHandler) batchSlashing(ctx sdk.Context, params *types.Params) {
 	// #2 condition
 	// We look through the full bonded set (not just the active set, include unbonding validators)
@@ -283,7 +339,8 @@ func (h *BlockHandler) batchSlashing(ctx sdk.Context, params *types.Params) {
 	for _, batch := range unslashedBatches {
 		// SLASH BONDED VALIDTORS who didn't attest batch requests
 		currentBondedSet := h.k.StakingKeeper.GetBondedValidatorsByPower(ctx)
-		confirms := h.k.GetBatchConfirmByNonceAndTokenContract(ctx, batch.BatchNonce, common.HexToAddress(batch.TokenContract))
+		confirms := prepBatchConfirms(ctx, h.k, batch)
+
 		for _, val := range currentBondedSet {
 			// Don't slash validators who joined after batch is created
 			consAddr, _ := val.GetConsAddr()
@@ -293,25 +350,27 @@ func (h *BlockHandler) batchSlashing(ctx sdk.Context, params *types.Params) {
 				continue
 			}
 
-			found := false
-			for _, batchConfirmation := range confirms {
-				// TODO this presents problems for delegate key rotation see issue #344
-				orchestratorAcc, _ := sdk.AccAddressFromBech32(batchConfirmation.Orchestrator)
-				delegatedOperator, delegatedFound := h.k.GetOrchestratorValidator(ctx, orchestratorAcc)
-				if delegatedFound && delegatedOperator.Equals(val.GetOperator()) {
-					found = true
-					break
-				}
-			}
+			_, found := confirms[val.GetOperator().String()]
 
 			if !found {
-				cons, _ := val.GetConsAddr()
-				consPower := val.ConsensusPower(h.k.StakingKeeper.PowerReduction(ctx))
+				// refresh validator before slashing/jailing
+				val, foundVal := h.k.StakingKeeper.GetValidator(ctx, val.GetOperator())
+				if !foundVal {
+					// this should be impossible, we haven't even progressed a single block since we got the list
+					panic("Validator exited set during endblocker?")
+				}
 
-				h.k.StakingKeeper.Slash(ctx, cons, ctx.BlockHeight(), consPower, params.SlashFractionBatch)
+				consPower := val.ConsensusPower(h.k.StakingKeeper.PowerReduction(ctx))
 
 				if !val.IsJailed() {
-					h.k.StakingKeeper.Jail(ctx, cons)
+					h.k.StakingKeeper.Slash(ctx, consAddr, ctx.BlockHeight(), consPower, params.SlashFractionBatch)
+					ctx.EventManager().EmitEvent(
+						sdk.NewEvent(
+							sdk.EventTypeMessage,
+							sdk.NewAttribute("BatchSignatureSlashing", consAddr.String()),
+						),
+					)
+					h.k.StakingKeeper.Jail(ctx, consAddr)
 				}
 			}
 		}

x/peggy/abci_test.go

@@ -78,27 +78,28 @@ func TestValsetSlashing_ValsetCreated_After_ValidatorBonded(t *testing.T) {
 	params := input.PeggyKeeper.GetParams(ctx)
 
 	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + int64(params.SignedValsetsWindow) + 2)
-	val := input.StakingKeeper.Validator(ctx, testpeggy.ValAddrs[0])
 	vs := pk.GetCurrentValset(ctx)
 	height := uint64(ctx.BlockHeight()) - (params.SignedValsetsWindow + 1)
 	vs.Height = height
-	vs.Nonce = height
+	vs.Nonce = pk.GetLatestValsetNonce(ctx) + 1
+
 	pk.StoreValsetUnsafe(ctx, vs)
 
-	for i, val := range testpeggy.AccAddrs {
+	for i, orch := range testpeggy.OrchAddrs {
 		if i == 0 {
 			// don't sign with first validator
 			continue
 		}
 
-		valsetConfirm := types.NewMsgValsetConfirm(vs.Nonce, testpeggy.EthAddrs[i].String(), val, "dummysig")
+		valsetConfirm := types.NewMsgValsetConfirm(vs.Nonce, testpeggy.EthAddrs[i].String(), orch, "dummysig")
 		pk.SetValsetConfirm(ctx, valsetConfirm)
 	}
 
+	assert.Equal(t, uint64(1), pk.GetLatestValsetNonce(ctx))
 	NewBlockHandler(pk).EndBlocker(ctx)
 
 	// ensure that the  validator who is bonded before valset is created is slashed
-	val = input.StakingKeeper.Validator(ctx, testpeggy.ValAddrs[0])
+	val := input.StakingKeeper.Validator(ctx, testpeggy.ValAddrs[0])
 	require.True(t, val.IsJailed())
 
 	// ensure that the  validator who attested the valset is not slashed.
@@ -131,10 +132,7 @@ func TestValsetSlashing_UnbondingValidator_UnbondWindow_NotExpired(t *testing.T)
 
 	// Create Valset request
 	ctx = ctx.WithBlockHeight(valsetRequestHeight)
-	vs := pk.GetCurrentValset(ctx)
-	vs.Height = uint64(valsetRequestHeight)
-	vs.Nonce = uint64(valsetRequestHeight)
-	pk.StoreValsetUnsafe(ctx, vs)
+	vs := pk.SetValsetRequest(ctx)
 
 	// Start Unbonding validators
 	// Validator-1  Unbond slash window is not expired. if not attested, slash
@@ -146,12 +144,12 @@ func TestValsetSlashing_UnbondingValidator_UnbondWindow_NotExpired(t *testing.T)
 	undelegateMsg2 := testpeggy.NewTestMsgUnDelegateValidator(testpeggy.ValAddrs[1], testpeggy.StakingAmount)
 	sh(input.Context, undelegateMsg2)
 
-	for i, val := range testpeggy.AccAddrs {
+	for i, orch := range testpeggy.OrchAddrs {
 		if i == 0 {
 			// don't sign with first validator
 			continue
 		}
-		valsetConfirm := types.NewMsgValsetConfirm(vs.Nonce, testpeggy.EthAddrs[i].String(), val, "dummysig")
+		valsetConfirm := types.NewMsgValsetConfirm(vs.Nonce, testpeggy.EthAddrs[i].String(), orch, "dummysig")
 		pk.SetValsetConfirm(ctx, valsetConfirm)
 	}
 	staking.EndBlocker(input.Context, input.StakingKeeper)
@@ -187,7 +185,7 @@ func TestBatchSlashing(t *testing.T) {
 	}
 	pk.StoreBatchUnsafe(ctx, batch)
 
-	for i, val := range testpeggy.AccAddrs {
+	for i, orch := range testpeggy.OrchAddrs {
 		if i == 0 {
 			// don't sign with first validator
 			continue
@@ -204,7 +202,7 @@ func TestBatchSlashing(t *testing.T) {
 			Nonce:         batch.BatchNonce,
 			TokenContract: testpeggy.TokenContractAddrs[0],
 			EthSigner:     testpeggy.EthAddrs[i].String(),
-			Orchestrator:  val.String(),
+			Orchestrator:  orch.String(),
 		})
 	}
 
@@ -238,7 +236,7 @@ func TestValsetEmission(t *testing.T) {
 
 	// EndBlocker should set a new validator set
 	NewBlockHandler(pk).EndBlocker(ctx)
-	require.NotNil(t, pk.GetValset(ctx, uint64(ctx.BlockHeight())))
+	require.NotNil(t, pk.GetValset(ctx, uint64(pk.GetLatestValsetNonce(ctx))))
 	valsets := pk.GetValsets(ctx)
 	require.True(t, len(valsets) == 2)
 }

x/peggy/keeper/attestation_handler.go

@@ -3,6 +3,7 @@ package keeper
 import (
 	"fmt"
 	"math/big"
+	"strconv"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
@@ -74,10 +75,25 @@ func (a AttestationHandler) Handle(ctx sdk.Context, claim types.EthereumClaim) e
 			}
 		}
 
+		ctx.EventManager().EmitEvent(
+			sdk.NewEvent(
+				types.EventTypeAttestation,
+				sdk.NewAttribute(types.AttributeKeySendToCosmosAmount, claim.Amount.String()),
+				sdk.NewAttribute(types.AttributeKeySendToCosmosNonce, strconv.Itoa(int(claim.GetEventNonce()))),
+				sdk.NewAttribute(types.AttributeKeySendToCosmosToken, claim.TokenContract),
+			),
+		)
+
 		// withdraw in this context means a withdraw from the Ethereum side of the bridge
 	case *types.MsgWithdrawClaim:
 		tokenContract := common.HexToAddress(claim.TokenContract)
 		a.keeper.OutgoingTxBatchExecuted(ctx, tokenContract, claim.BatchNonce)
+		ctx.EventManager().EmitEvent(
+			sdk.NewEvent(
+				types.EventTypeAttestation,
+				sdk.NewAttribute(types.AttributeKeyBatchSendToEthNonce, strconv.Itoa(int(claim.BatchNonce))),
+			),
+		)
 		return nil
 
 	case *types.MsgERC20DeployedClaim:
@@ -87,6 +103,15 @@ func (a AttestationHandler) Handle(ctx sdk.Context, claim types.EthereumClaim) e
 
 		// add to ERC20 mapping
 		a.keeper.SetCosmosOriginatedDenomToERC20(ctx, claim.CosmosDenom, common.HexToAddress(claim.TokenContract))
+
+		ctx.EventManager().EmitEvent(
+			sdk.NewEvent(
+				types.EventTypeAttestation,
+				sdk.NewAttribute(types.AttributeKeyERC20DeployedToken, claim.TokenContract),
+				sdk.NewAttribute(types.AttributeKeyERC20DeployedNonce, strconv.Itoa(int(claim.GetEventNonce()))),
+			),
+		)
+
 		return nil
 
 	case *types.MsgValsetUpdatedClaim:
@@ -100,6 +125,13 @@ func (a AttestationHandler) Handle(ctx sdk.Context, claim types.EthereumClaim) e
 			RewardToken:  claim.RewardToken,
 		})
 
+		ctx.EventManager().EmitEvent(
+			sdk.NewEvent(
+				types.EventTypeAttestation,
+				sdk.NewAttribute(types.AttributeKeyValsetUpdatedNonce, strconv.Itoa(int(claim.GetEventNonce()))),
+			),
+		)
+
 	default:
 		panic(fmt.Sprintf("invalid event type for attestations %s", claim.GetType()))
 	}

x/peggy/keeper/batch.go

@@ -282,13 +282,12 @@ func (k *Keeper) GetLastSlashedBatchBlock(ctx sdk.Context) uint64 {
 // GetUnslashedBatches returns all the unslashed batches in state
 func (k *Keeper) GetUnslashedBatches(ctx sdk.Context, maxHeight uint64) (out []*types.OutgoingTxBatch) {
 	lastSlashedBatchBlock := k.GetLastSlashedBatchBlock(ctx)
-	k.IterateBatchBySlashedBatchBlock(ctx, lastSlashedBatchBlock, maxHeight, func(_ []byte, batch *types.OutgoingTxBatch) bool {
-		if batch.Block > lastSlashedBatchBlock {
+	batches := k.GetOutgoingTxBatches(ctx)
+	for _, batch := range batches {
+		if batch.Block > lastSlashedBatchBlock && batch.Block < maxHeight {
 			out = append(out, batch)
 		}
-		return false
-	})
-
+	}
 	return
 }