refactor: move tryAttestion from abci to claimMsg (FunctionX#101)

x/crosschain/keeper/abci.go

@@ -13,7 +13,6 @@ import (
 func (k Keeper) EndBlocker(ctx sdk.Context) {
 	signedWindow := k.GetSignedWindow(ctx)
 	k.slashing(ctx, signedWindow)
-	k.attestationTally(ctx)
 	k.cleanupTimedOutBatches(ctx)
 	k.createOracleSetRequest(ctx)
 	k.pruneOracleSet(ctx, signedWindow)
@@ -124,62 +123,6 @@ func (k Keeper) batchSlashing(ctx sdk.Context, oracles types.Oracles, signedWind
 	return hasSlash
 }
 
-// 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
-func (k Keeper) attestationTally(ctx sdk.Context) {
-	type attClaim struct {
-		*types.Attestation
-		types.ExternalClaim
-	}
-	attMap := make(map[uint64][]attClaim)
-	// We make a slice with all the event nonces that are in the attestation mapping
-	var nonces []uint64
-	k.IterateAttestationAndClaim(ctx, func(att *types.Attestation, claim types.ExternalClaim) bool {
-		if v, ok := attMap[claim.GetEventNonce()]; !ok {
-			attMap[claim.GetEventNonce()] = []attClaim{{att, claim}}
-			nonces = append(nonces, claim.GetEventNonce())
-		} else {
-			attMap[claim.GetEventNonce()] = append(v, attClaim{att, claim})
-		}
-		return false
-	})
-	// Then we sort it
-	sort.Slice(nonces, func(i, j int) bool {
-		return nonces[i] < nonces[j]
-	})
-
-	// This iterates over all nonces (event nonces) in the attestation mapping. Each value contains
-	// a slice with one or more attestations at that event nonce. There can be multiple attestations
-	// at one event nonce when Oracles disagree about what event happened at that nonce.
-	for _, nonce := range nonces {
-		// This iterates over all attestations at a particular event nonce.
-		// They are ordered by when the first attestation at the event nonce was received.
-		// This order is not important.
-		for _, att := range attMap[nonce] {
-			// We check if the event nonce is exactly 1 higher than the last attestation that was
-			// observed. If it is not, we just move on to the next nonce. This will skip over all
-			// attestations that have already been observed.
-			//
-			// Once we hit an event nonce that is one higher than the last observed event, we stop
-			// skipping over this conditional and start calling tryAttestation (counting votes)
-			// Once an attestation at a given event nonce has enough votes and becomes observed,
-			// every other attestation at that nonce will be skipped, since the lastObservedEventNonce
-			// will be incremented.
-			//
-			// Then we go to the next event nonce in the attestation mapping, if there is one. This
-			// nonce will once again be one higher than the lastObservedEventNonce.
-			// If there is an attestation at this event nonce which has enough votes to be observed,
-			// we skip the other attestations and move on to the next nonce again.
-			// If no attestation becomes observed, when we get to the next nonce, every attestation in
-			// it will be skipped. The same will happen for every nonce after that.
-			if nonce == k.GetLastObservedEventNonce(ctx)+1 {
-				k.TryAttestation(ctx, att.Attestation, att.ExternalClaim)
-			}
-		}
-	}
-}
-
 // cleanupTimedOutBatches 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

x/crosschain/keeper/attestation.go

@@ -43,9 +43,12 @@ func (k Keeper) Attest(ctx sdk.Context, oracleAddr sdk.AccAddress, claim types.E
 
 	// Add the oracle's vote to this attestation
 	att.Votes = append(att.Votes, oracleAddr.String())
-
 	k.SetAttestation(ctx, claim.GetEventNonce(), claim.ClaimHash(), att)
 
+	if !att.Observed && claim.GetEventNonce() == k.GetLastObservedEventNonce(ctx)+1 {
+		k.TryAttestation(ctx, att, claim)
+	}
+
 	ctx = ctx.WithGasMeter(gasMeter)
 	k.SetLastEventNonceByOracle(ctx, oracleAddr, claim.GetEventNonce())
 	k.SetLastEventBlockHeightByOracle(ctx, oracleAddr, claim.GetBlockHeight())
@@ -57,10 +60,6 @@ func (k Keeper) Attest(ctx sdk.Context, oracleAddr sdk.AccAddress, claim types.E
 // and has not already been marked Observed, then calls processAttestation to actually apply it to the state,
 // and then marks it Observed and emits an event.
 func (k Keeper) TryAttestation(ctx sdk.Context, att *types.Attestation, claim types.ExternalClaim) {
-	if att.Observed {
-		// We panic here because this should never happen
-		panic("attempting to process observed attestation")
-	}
 	// If the attestation has not yet been Observed, sum up the votes and see if it is ready to apply to the state.
 	// This conditional stops the attestation from accidentally being applied twice.
 	// Sum the current powers of all validators who have voted and see if it passes the current threshold
@@ -82,14 +81,7 @@ func (k Keeper) TryAttestation(ctx sdk.Context, att *types.Attestation, claim ty
 		if attestationPower.LT(requiredPower) {
 			continue
 		}
-		// If the power of all the validators that have voted on the attestation is higher or equal to the threshold,
-		// process the attestation, set Observed to true, and break
-		lastEventNonce := k.GetLastObservedEventNonce(ctx)
-		// this check is performed at the next level up so this should never panic
-		// outside of programmer error.
-		if claim.GetEventNonce() != lastEventNonce+1 {
-			panic("attempting to apply events to state out of order")
-		}
+
 		k.SetLastObservedEventNonce(ctx, claim.GetEventNonce())
 		k.SetLastObservedBlockHeight(ctx, claim.GetBlockHeight(), uint64(ctx.BlockHeight()))
 

x/crosschain/keeper/msg_server_test.go

@@ -725,6 +725,10 @@ func (suite *KeeperTestSuite) TestClaimMsgGasConsumed() {
 			execute: func(claimMsg types.ExternalClaim) (minGas, maxGas, avgGas uint64) {
 				msg, ok := claimMsg.(*types.MsgSendToExternalClaim)
 				suite.True(ok)
+				suite.Require().NoError(suite.Keeper().StoreBatch(suite.ctx, &types.OutgoingTxBatch{
+					BatchNonce:    msg.BatchNonce,
+					TokenContract: msg.TokenContract,
+				}))
 				for i, oracle := range suite.oracleAddrs {
 					eventNonce := suite.Keeper().GetLastEventNonceByOracle(suite.ctx, oracle)
 					msg.EventNonce = eventNonce + 1