Prune EthereumSignatures and EthereumEventVoteRecords

module/x/gravity/abci.go

@@ -27,6 +27,8 @@ func BeginBlocker(ctx sdk.Context, k keeper.Keeper) {
 // EndBlocker is called at the end of every block
 func EndBlocker(ctx sdk.Context, k keeper.Keeper) {
 	outgoingTxSlashing(ctx, k)
+	// Do we need to concern ourselves with future slashing windows for this pruning?
+	pruneEventVoteRecords(ctx, k)
 	eventVoteRecordTally(ctx, k)
 	updateObservedEthereumHeight(ctx, k)
 }
@@ -100,12 +102,30 @@ func pruneSignerSetTxs(ctx sdk.Context, k keeper.Keeper) {
 		earliestToPrune := currentBlock - params.SignedSignerSetTxsWindow
 		for _, set := range k.GetSignerSetTxs(ctx) {
 			if set.Nonce < lastObserved.Nonce && set.Height < earliestToPrune {
+				k.DeleteEthereumSignatures(ctx, set.GetStoreIndex())
 				k.DeleteOutgoingTx(ctx, set.GetStoreIndex())
 			}
 		}
 	}
 }
 
+// pruneEventVoteRecords deletes all event vote records with nonces that are older than the last observed event nonce
+func pruneEventVoteRecords(ctx sdk.Context, k keeper.Keeper) {
+	lastObservedEventNonce := k.GetLastObservedEventNonce(ctx)
+	k.IterateEthereumEventVoteRecords(ctx, func(key []byte, eventVoteRecord *types.EthereumEventVoteRecord) bool {
+		event, err := types.UnpackEvent(eventVoteRecord.Event)
+		if err != nil {
+			panic(err)
+		}
+		eventNonce := event.GetEventNonce()
+		if eventNonce <= lastObservedEventNonce {
+			k.DeleteEthereumEventVoteRecord(ctx, eventNonce, event.Hash())
+		}
+
+		return false
+	})
+}
+
 // Iterate over all attestations currently being voted on in order of nonce and
 // "Observe" those who have passed the threshold. Break the loop once we see
 // an attestation that has not passed the threshold
@@ -155,11 +175,11 @@ func eventVoteRecordTally(ctx sdk.Context, k keeper.Keeper) {
 // order to keep this information current regardless of the level of bridge activity.
 //
 // We determine if we should update the latest heights based on the following criteria:
-// 1. A consensus of validators agrees that the proposed height is equal to or less than their
-//    last observed height, in order to reconcile the many different heights that will be submitted.
-//    The highest height that meets this criteria will be the proposed height.
-// 2. The proposed consensus heights from this process are greater than the values stored from the last time
-//    we observed an Ethereum event from the bridge
+//  1. A consensus of validators agrees that the proposed height is equal to or less than their
+//     last observed height, in order to reconcile the many different heights that will be submitted.
+//     The highest height that meets this criteria will be the proposed height.
+//  2. The proposed consensus heights from this process are greater than the values stored from the last time
+//     we observed an Ethereum event from the bridge
 func updateObservedEthereumHeight(ctx sdk.Context, k keeper.Keeper) {
 	// wait some minutes before checking the height votes
 	if ctx.BlockHeight()%50 != 0 {
@@ -228,12 +248,15 @@ func updateObservedEthereumHeight(ctx sdk.Context, k keeper.Keeper) {
 // cleanupTimedOutBatchTxs deletes batches that have passed their expiration on Ethereum
 // keep in mind several things when modifying this function
 // A) unlike nonces timeouts are not monotonically increasing, meaning batch 5 can have a later timeout than batch 6
-//    this means that we MUST only cleanup a single batch at a time
+//
+//	this means that we MUST only cleanup a single batch at a time
+//
 // B) it is possible for ethereumHeight to be zero if no events have ever occurred, make sure your code accounts for this
 // C) When we compute the timeout we do our best to estimate the Ethereum block height at that very second. But what we work with
-//    here is the Ethereum block height at the time of the last Deposit or Withdraw to be observed. It's very important we do not
-//    project, if we do a slowdown on ethereum could cause a double spend. Instead timeouts will *only* occur after the timeout period
-//    AND any deposit or withdraw has occurred to update the Ethereum block height.
+//
+//	here is the Ethereum block height at the time of the last Deposit or Withdraw to be observed. It's very important we do not
+//	project, if we do a slowdown on ethereum could cause a double spend. Instead timeouts will *only* occur after the timeout period
+//	AND any deposit or withdraw has occurred to update the Ethereum block height.
 func cleanupTimedOutBatchTxs(ctx sdk.Context, k keeper.Keeper) {
 	ethereumHeight := k.GetLastObservedEthereumBlockHeight(ctx).EthereumHeight
 	k.IterateOutgoingTxsByType(ctx, types.BatchTxPrefixByte, func(key []byte, otx types.OutgoingTx) bool {
@@ -250,17 +273,21 @@ func cleanupTimedOutBatchTxs(ctx sdk.Context, k keeper.Keeper) {
 // cleanupTimedOutContractCallTxs deletes logic calls that have passed their expiration on Ethereum
 // keep in mind several things when modifying this function
 // A) unlike nonces timeouts are not monotonically increasing, meaning call 5 can have a later timeout than batch 6
-//    this means that we MUST only cleanup a single call at a time
+//
+//	this means that we MUST only cleanup a single call at a time
+//
 // B) it is possible for ethereumHeight to be zero if no events have ever occurred, make sure your code accounts for this
 // C) When we compute the timeout we do our best to estimate the Ethereum block height at that very second. But what we work with
-//    here is the Ethereum block height at the time of the last Deposit or Withdraw to be observed. It's very important we do not
-//    project, if we do a slowdown on ethereum could cause a double spend. Instead timeouts will *only* occur after the timeout period
-//    AND any deposit or withdraw has occurred to update the Ethereum block height.
+//
+//	here is the Ethereum block height at the time of the last Deposit or Withdraw to be observed. It's very important we do not
+//	project, if we do a slowdown on ethereum could cause a double spend. Instead timeouts will *only* occur after the timeout period
+//	AND any deposit or withdraw has occurred to update the Ethereum block height.
 func cleanupTimedOutContractCallTxs(ctx sdk.Context, k keeper.Keeper) {
 	ethereumHeight := k.GetLastObservedEthereumBlockHeight(ctx).EthereumHeight
 	k.IterateOutgoingTxsByType(ctx, types.ContractCallTxPrefixByte, func(_ []byte, otx types.OutgoingTx) bool {
 		cctx, _ := otx.(*types.ContractCallTx)
 		if cctx.Timeout < ethereumHeight {
+			k.DeleteEthereumSignatures(ctx, cctx.GetStoreIndex())
 			k.DeleteOutgoingTx(ctx, cctx.GetStoreIndex())
 		}
 		return true

module/x/gravity/handler_test.go

@@ -274,18 +274,24 @@ func TestMsgSubmitEthreumEventSendToCosmosMultiValidator(t *testing.T) {
 	balance2 := input.BankKeeper.GetAllBalances(ctx, myCosmosAddr)
 	require.Equal(t, sdk.Coins{sdk.NewInt64Coin(denom, 12)}, balance2)
 
+	// and vouchers added to the account
+	// this happens when 2/3 of the voting power have submitted claims, so the third isn't required
+	balance3 := input.BankKeeper.GetAllBalances(ctx, myCosmosAddr)
+	require.Equal(t, sdk.Coins{sdk.NewInt64Coin(denom, 12)}, balance3)
+
 	// when
 	ctx = ctx.WithBlockTime(myBlockTime)
 	_, err = h(ctx, ethClaim3Msg)
 	gravity.EndBlocker(ctx, input.GravityKeeper)
 	require.NoError(t, err)
 
-	// and attestation persisted
+	// and attestations pruned
+	a1 = input.GravityKeeper.GetEthereumEventVoteRecord(ctx, myNonce, ethClaim1.Hash())
+	a2 = input.GravityKeeper.GetEthereumEventVoteRecord(ctx, myNonce, ethClaim2.Hash())
 	a3 := input.GravityKeeper.GetEthereumEventVoteRecord(ctx, myNonce, ethClaim3.Hash())
-	require.NotNil(t, a3)
-	// and no additional added to the account
-	balance3 := input.BankKeeper.GetAllBalances(ctx, myCosmosAddr)
-	require.Equal(t, sdk.Coins{sdk.NewInt64Coin(denom, 12)}, balance3)
+	require.Nil(t, a1)
+	require.Nil(t, a2)
+	require.Nil(t, a3)
 }
 
 func TestMsgSetDelegateAddresses(t *testing.T) {

module/x/gravity/keeper/batch.go

@@ -15,12 +15,12 @@ import (
 const BatchTxSize = 100
 
 // BuildBatchTx starts the following process chain:
-// - find bridged denominator for given voucher type
-// - determine if a an unexecuted batch is already waiting for this token type, if so confirm the new batch would
-//   have a higher total fees. If not exit withtout creating a batch
-// - select available transactions from the outgoing transaction pool sorted by fee desc
-// - persist an outgoing batch object with an incrementing ID = nonce
-// - emit an event
+//   - find bridged denominator for given voucher type
+//   - determine if a an unexecuted batch is already waiting for this token type, if so confirm the new batch would
+//     have a higher total fees. If not exit withtout creating a batch
+//   - select available transactions from the outgoing transaction pool sorted by fee desc
+//   - persist an outgoing batch object with an incrementing ID = nonce
+//   - emit an event
 func (k Keeper) BuildBatchTx(ctx sdk.Context, contractAddress common.Address, maxElements int) *types.BatchTx {
 	// if there is a more profitable batch for this token type do not create a new batch
 	if lastBatch := k.getLastOutgoingBatchByTokenType(ctx, contractAddress); lastBatch != nil {
@@ -81,6 +81,7 @@ func (k Keeper) batchTxExecuted(ctx sdk.Context, tokenContract common.Address, n
 		}
 		return false
 	})
+	k.DeleteEthereumSignatures(ctx, batchTx.GetStoreIndex())
 	k.DeleteOutgoingTx(ctx, batchTx.GetStoreIndex())
 }
 
@@ -123,6 +124,7 @@ func (k Keeper) CancelBatchTx(ctx sdk.Context, batch *types.BatchTx) {
 	}
 
 	// Delete batch since it is finished
+	k.DeleteEthereumSignatures(ctx, batch.GetStoreIndex())
 	k.DeleteOutgoingTx(ctx, batch.GetStoreIndex())
 
 	ctx.EventManager().EmitEvent(

module/x/gravity/keeper/contract_call.go

@@ -23,10 +23,12 @@ func (k Keeper) contractCallExecuted(ctx sdk.Context, invalidationScope []byte,
 		cctx, _ := otx.(*types.ContractCallTx)
 		if (cctx.InvalidationNonce < completedCallTx.InvalidationNonce) &&
 			bytes.Equal(cctx.InvalidationScope, completedCallTx.InvalidationScope) {
+			k.DeleteEthereumSignatures(ctx, cctx.GetStoreIndex())
 			k.DeleteOutgoingTx(ctx, cctx.GetStoreIndex())
 		}
 		return false
 	})
 
+	k.DeleteEthereumSignatures(ctx, completedCallTx.GetStoreIndex())
 	k.DeleteOutgoingTx(ctx, completedCallTx.GetStoreIndex())
 }

module/x/gravity/keeper/ethereum_event_vote.go

@@ -50,7 +50,11 @@ func (k Keeper) recordEventVote(
 	// Add the validator's vote to this EthereumEventVoteRecord
 	eventVoteRecord.Votes = append(eventVoteRecord.Votes, val.String())
 
-	k.setEthereumEventVoteRecord(ctx, event.GetEventNonce(), event.Hash(), eventVoteRecord)
+	// ignore "old" event votes but still record the nonce for the submitting validator
+	if event.GetEventNonce() > k.GetLastObservedEventNonce(ctx) {
+		k.setEthereumEventVoteRecord(ctx, event.GetEventNonce(), event.Hash(), eventVoteRecord)
+	}
+
 	k.setLastEventNonceByValidator(ctx, val, event.GetEventNonce())
 
 	return eventVoteRecord, nil
@@ -140,6 +144,10 @@ func (k Keeper) setEthereumEventVoteRecord(ctx sdk.Context, eventNonce uint64, c
 	ctx.KVStore(k.storeKey).Set(types.MakeEthereumEventVoteRecordKey(eventNonce, claimHash), k.cdc.MustMarshal(eventVoteRecord))
 }
 
+func (k Keeper) DeleteEthereumEventVoteRecord(ctx sdk.Context, eventNonce uint64, claimHash []byte) {
+	ctx.KVStore(k.storeKey).Delete(types.MakeEthereumEventVoteRecordKey(eventNonce, claimHash))
+}
+
 // GetEthereumEventVoteRecord return a vote record given a nonce
 func (k Keeper) GetEthereumEventVoteRecord(ctx sdk.Context, eventNonce uint64, claimHash []byte) *types.EthereumEventVoteRecord {
 	if bz := ctx.KVStore(k.storeKey).Get(types.MakeEthereumEventVoteRecordKey(eventNonce, claimHash)); bz == nil {
@@ -154,7 +162,7 @@ func (k Keeper) GetEthereumEventVoteRecord(ctx sdk.Context, eventNonce uint64, c
 // GetEthereumEventVoteRecordMapping returns a mapping of eventnonce -> attestations at that nonce
 func (k Keeper) GetEthereumEventVoteRecordMapping(ctx sdk.Context) (out map[uint64][]*types.EthereumEventVoteRecord) {
 	out = make(map[uint64][]*types.EthereumEventVoteRecord)
-	k.iterateEthereumEventVoteRecords(ctx, func(key []byte, eventVoteRecord *types.EthereumEventVoteRecord) bool {
+	k.IterateEthereumEventVoteRecords(ctx, func(key []byte, eventVoteRecord *types.EthereumEventVoteRecord) bool {
 		event, err := types.UnpackEvent(eventVoteRecord.Event)
 		if err != nil {
 			panic(err)
@@ -170,7 +178,7 @@ func (k Keeper) GetEthereumEventVoteRecordMapping(ctx sdk.Context) (out map[uint
 }
 
 // iterateEthereumEventVoteRecords iterates through all attestations
-func (k Keeper) iterateEthereumEventVoteRecords(ctx sdk.Context, cb func([]byte, *types.EthereumEventVoteRecord) bool) {
+func (k Keeper) IterateEthereumEventVoteRecords(ctx sdk.Context, cb func([]byte, *types.EthereumEventVoteRecord) bool) {
 	store := prefix.NewStore(ctx.KVStore(k.storeKey), []byte{types.EthereumEventVoteRecordKey})
 	iter := store.Iterator(nil, nil)
 	defer iter.Close()

module/x/gravity/keeper/keeper.go

@@ -129,12 +129,12 @@ func (k Keeper) GetLastUnbondingBlockHeight(ctx sdk.Context) uint64 {
 //     ETHEREUM SIGNATURES   //
 ///////////////////////////////
 
-// getEthereumSignature returns a valset confirmation by a nonce and validator address
+// getEthereumSignature returns an ethereum signature by a nonce and validator address
 func (k Keeper) getEthereumSignature(ctx sdk.Context, storeIndex []byte, validator sdk.ValAddress) []byte {
 	return ctx.KVStore(k.storeKey).Get(types.MakeEthereumSignatureKey(storeIndex, validator))
 }
 
-// SetEthereumSignature sets a valset confirmation
+// SetEthereumSignature sets an ethereum signature
 func (k Keeper) SetEthereumSignature(ctx sdk.Context, sig types.EthereumTxConfirmation, val sdk.ValAddress) []byte {
 	key := types.MakeEthereumSignatureKey(sig.GetStoreIndex(), val)
 	ctx.KVStore(k.storeKey).Set(key, sig.GetSignature())
@@ -151,6 +151,14 @@ func (k Keeper) GetEthereumSignatures(ctx sdk.Context, storeIndex []byte) map[st
 	return signatures
 }
 
+// DeleteEthereumSignatures deletes all ethereum signatures for a given outgoing tx by store index
+func (k Keeper) DeleteEthereumSignatures(ctx sdk.Context, storeIndex []byte) {
+	k.iterateEthereumSignatures(ctx, storeIndex, func(val sdk.ValAddress, h []byte) bool {
+		ctx.KVStore(k.storeKey).Delete(types.MakeEthereumSignatureKey(storeIndex, val))
+		return false
+	})
+}
+
 // iterateEthereumSignatures iterates through all valset confirms by nonce in ASC order
 func (k Keeper) iterateEthereumSignatures(ctx sdk.Context, storeIndex []byte, cb func(sdk.ValAddress, []byte) bool) {
 	prefixStore := prefix.NewStore(ctx.KVStore(k.storeKey), append([]byte{types.EthereumSignatureKey}, storeIndex...))
@@ -680,7 +688,7 @@ func (k Keeper) MigrateGravityContract(ctx sdk.Context, newBridgeAddress string,
 
 	// Reset all ethereum event nonces to zero
 	k.setLastObservedEventNonce(ctx, 0)
-	k.iterateEthereumEventVoteRecords(ctx, func(_ []byte, voteRecord *types.EthereumEventVoteRecord) bool {
+	k.IterateEthereumEventVoteRecords(ctx, func(_ []byte, voteRecord *types.EthereumEventVoteRecord) bool {
 		for _, vote := range voteRecord.Votes {
 			val, err := sdk.ValAddressFromBech32(vote)
 

module/x/gravity/keeper/keeper_test.go

@@ -80,7 +80,7 @@ func TestAttestationIterator(t *testing.T) {
 	input.GravityKeeper.setEthereumEventVoteRecord(ctx, dep2.EventNonce, dep2.Hash(), att2)
 
 	var atts []*types.EthereumEventVoteRecord
-	input.GravityKeeper.iterateEthereumEventVoteRecords(ctx, func(_ []byte, att *types.EthereumEventVoteRecord) bool {
+	input.GravityKeeper.IterateEthereumEventVoteRecords(ctx, func(_ []byte, att *types.EthereumEventVoteRecord) bool {
 		atts = append(atts, att)
 		return false
 	})