Pr comments

x/btccheckpoint/keeper/keeper.go

@@ -27,11 +27,11 @@ type (
 		expectedCheckpointTag txformat.BabylonTag
 	}
 
-	SubmissionBtcState int
+	SubmissionBtcStatus int
 )
 
 const (
-	Submitted SubmissionBtcState = iota
+	Submitted SubmissionBtcStatus = iota
 	Confirmed
 	Finalized
 	OnFork
@@ -64,7 +64,7 @@ func NewKeeper(
 	}
 }
 
-func (btcState SubmissionBtcState) onMainChain() bool {
+func (btcState SubmissionBtcStatus) onMainChain() bool {
 	return btcState != OnFork
 }
 
@@ -89,7 +89,7 @@ func (k Keeper) CheckHeaderIsOnMainChain(ctx sdk.Context, hash *bbn.BTCHeaderHas
 	return err == nil && depth >= 0
 }
 
-func (k Keeper) GetSubmissionBtcState(ctx sdk.Context, sk types.SubmissionKey) (SubmissionBtcState, error) {
+func (k Keeper) GetSubmissionBtcState(ctx sdk.Context, sk types.SubmissionKey) (SubmissionBtcStatus, error) {
 	var submissionDepth uint64 = math.MaxUint64
 	for _, tk := range sk.Key {
 		blockDepth, err := k.btcLightClientKeeper.MainChainDepth(ctx, tk.Hash)
@@ -160,7 +160,7 @@ func (k Keeper) AddEpochSubmission(
 	epochNum uint64,
 	sk types.SubmissionKey,
 	sd types.SubmissionData,
-	submissionBtcState SubmissionBtcState,
+	submissionBtcState SubmissionBtcStatus,
 	epochRawCheckpoint []byte) error {
 
 	ed := k.GetEpochData(ctx, epochNum)
@@ -196,15 +196,15 @@ func (k Keeper) AddEpochSubmission(
 	// always start submission lifecycle from unconfirmed state, even if it is
 	// confirmed or finalized. It will quickly reach next states with btc
 	// light client blocks
-	k.addToUnconfirmed(ctx, sk)
+	k.addToSubmitted(ctx, sk)
 	k.saveEpochData(ctx, epochNum, ed)
 	k.saveSubmission(ctx, sk, sd)
 	return nil
 }
 
-func (k Keeper) addToUnconfirmed(ctx sdk.Context, sk types.SubmissionKey) {
+func (k Keeper) addToSubmitted(ctx sdk.Context, sk types.SubmissionKey) {
 	store := ctx.KVStore(k.storeKey)
-	uk := types.UnconfiredSubmissionsKey(k.cdc, &sk)
+	uk := types.SubmittedSubmissionsKey(k.cdc, &sk)
 	v := k.cdc.MustMarshal(&sk)
 	store.Set(uk, v)
 }
@@ -247,10 +247,10 @@ func (k Keeper) getSubmissionDataExists(ctx sdk.Context, sk types.SubmissionKey)
 	return sd
 }
 
-func (k Keeper) promoteUnconfirmedToConfirmed(ctx sdk.Context, sk types.SubmissionKey) {
+func (k Keeper) promoteSubmittedToConfirmed(ctx sdk.Context, sk types.SubmissionKey) {
 	store := ctx.KVStore(k.storeKey)
 	subKey := k.cdc.MustMarshal(&sk)
-	unconfirmedKey := types.UnconfiredSubmissionsKey(k.cdc, &sk)
+	unconfirmedKey := types.SubmittedSubmissionsKey(k.cdc, &sk)
 	confirmedKey := types.ConfirmedSubmissionsKey(k.cdc, &sk)
 
 	// Promotion of submision from submitted state to confirmed state is just
@@ -279,14 +279,14 @@ func (k Keeper) promoteConfirmedToFinalized(ctx sdk.Context, sk types.Submission
 // Approach with iterator was taken as:
 // - There can be many unconfirmed submissions across many epochs
 // - pruning is a bit more streight forward with iterator apporoach
-func (k Keeper) checkUnconfirmed(ctx sdk.Context) {
+func (k Keeper) checkSubmitted(ctx sdk.Context) {
 
 	newConfirmed := []types.SubmissionKey{}
 
 	store := ctx.KVStore(k.storeKey)
 
 	// iterator over all unconfirmed submissions
-	iterator := sdk.KVStorePrefixIterator(store, types.UnconfirmedIndexPrefix)
+	iterator := sdk.KVStorePrefixIterator(store, types.SubmittedIndexPrefix)
 
 	defer iterator.Close()
 
@@ -388,7 +388,7 @@ func (k Keeper) checkUnconfirmed(ctx sdk.Context) {
 		// Promote all newly confirmed keys
 		// It could be done in loop which handles epoch but it is a bit cleaner that way
 		// this will be especially clear when working on rewards
-		k.promoteUnconfirmedToConfirmed(ctx, newConfirmedSubKey)
+		k.promoteSubmittedToConfirmed(ctx, newConfirmedSubKey)
 	}
 }
 
@@ -503,7 +503,7 @@ func (k Keeper) getSubmissionsWithPrefix(ctx sdk.Context, prefix []byte) []types
 }
 
 func (k Keeper) GetAllUnconfirmedSubmissions(ctx sdk.Context) []types.SubmissionKey {
-	return k.getSubmissionsWithPrefix(ctx, types.UnconfirmedIndexPrefix)
+	return k.getSubmissionsWithPrefix(ctx, types.SubmittedIndexPrefix)
 }
 
 func (k Keeper) GetAllConfirmedSubmissions(ctx sdk.Context) []types.SubmissionKey {
@@ -516,6 +516,6 @@ func (k Keeper) GetAllFinalizedSubmissions(ctx sdk.Context) []types.SubmissionKe
 
 // Callback to be called when btc light client tip change
 func (k Keeper) OnTipChange(ctx sdk.Context) {
-	k.checkUnconfirmed(ctx)
+	k.checkSubmitted(ctx)
 	k.checkConfirmed(ctx)
 }

x/btccheckpoint/keeper/msg_server.go

@@ -53,29 +53,40 @@ func checkHashesFromOneBlock(hs []*btypes.BTCHeaderHashBytes) bool {
 	return true
 }
 
-func (m msgServer) checkHashesAreOnTheSameFork(ctx sdk.Context, hs []*btypes.BTCHeaderHashBytes) bool {
-	if len(hs) < 2 {
-		// cannot have ancestry relations with only 0 or 1 hash
-		return false
+// checkHashesAreOnTheSameFork checks if provided hases are on the same fork, if
+// one of the hashes is not known to light client it returns error
+func (m msgServer) checkHashesAreOnTheSameFork(ctx sdk.Context, hs []*btypes.BTCHeaderHashBytes) (bool, error) {
+
+	if len(hs) == 0 {
+		// with empty hashes, cannot check for ancestry
+		return false, nil
+	}
+
+	if len(hs) == 1 {
+		// there is only one hash, it is by defintion on one fork.
+		return true, nil
 	}
 
 	for i := 1; i < len(hs); i++ {
 		onTheSameFork, err := m.onTheSameFork(ctx, hs[i-1], hs[i])
 
 		if err != nil {
-			// TODO: Light client lost knowledge of one of the chekpoint hashes.
-			// decide what to do here. For now returning false, as we cannot check ancestry.
-			return false
+			return false, err
 		}
 
 		if !onTheSameFork {
 			// all block hashes are known to light client, but are no longer at the same
 			// fork. Checkpoint defacto lost its validity due to some reorg happening.
-			return false
+			return false, nil
 		}
 	}
 
-	return true
+	return true, nil
+}
+
+func (m msgServer) submissionKeyOnOneFork(ctx sdk.Context, key *types.SubmissionKey) (bool, error) {
+	keyHashes := key.GetKeyBlockHashes()
+	return m.checkHashesAreOnTheSameFork(ctx, keyHashes)
 }
 
 func (m msgServer) checkHeaderIsDescentantOfPreviousEpoch(
@@ -117,7 +128,15 @@ func (m msgServer) checkHeaderIsDescentantOfPreviousEpoch(
 		} else {
 			// hashes are not from the same block i.e this checkpoint was split between
 			// at least two blocks, check if those blocks are still on the same fork
-			if !m.checkHashesAreOnTheSameFork(ctx, hs) {
+			onSameFork, err := m.checkHashesAreOnTheSameFork(ctx, hs)
+
+			if err != nil {
+				// Submission is no longer known to light client
+				// TODO it could probably be delted.
+				continue
+			}
+
+			if !onSameFork {
 				// checkpoint blockhashes no longer form a chain. Cannot check ancestry
 				// with new submission. Move to the next checkpoint
 				continue
@@ -182,15 +201,14 @@ func (m msgServer) InsertBTCSpvProof(ctx context.Context, req *types.MsgInsertBT
 	// is implicit.
 	// TODO: Either get rid of accepting subbmisions on forks, or design some better
 	// mechanism to deal with that case
-	if !rawSubmission.InOneBlock() && submissionState == OnFork {
-		onTheSameFork, err := m.onTheSameFork(
+	if submissionState == OnFork {
+		onTheSameFork, err := m.submissionKeyOnOneFork(
 			sdkCtx,
-			&rawSubmission.Proof1.BlockHash,
-			&rawSubmission.Proof2.BlockHash,
+			&submissionKey,
 		)
 
 		if err != nil {
-			panic("Headers which are should have been known to btclight client")
+			panic("Headers which shoud have been known to btc light client")
 		}
 
 		if !onTheSameFork {

x/btccheckpoint/types/keys.go

@@ -23,11 +23,11 @@ const (
 )
 
 var (
-	SubmisionKeyPrefix     = []byte{3}
-	UnconfirmedIndexPrefix = []byte{4}
-	ConfirmedIndexPrefix   = []byte{5}
-	FinalizedIndexPrefix   = []byte{6}
-	EpochDataPrefix        = []byte{7}
+	SubmisionKeyPrefix   = []byte{3}
+	SubmittedIndexPrefix = []byte{4}
+	ConfirmedIndexPrefix = []byte{5}
+	FinalizedIndexPrefix = []byte{6}
+	EpochDataPrefix      = []byte{7}
 )
 
 func KeyPrefix(p string) []byte {
@@ -38,8 +38,8 @@ func PrefixedSubmisionKey(cdc codec.BinaryCodec, k *SubmissionKey) []byte {
 	return append(SubmisionKeyPrefix, cdc.MustMarshal(k)...)
 }
 
-func UnconfiredSubmissionsKey(cdc codec.BinaryCodec, k *SubmissionKey) []byte {
-	return append(UnconfirmedIndexPrefix, cdc.MustMarshal(k)...)
+func SubmittedSubmissionsKey(cdc codec.BinaryCodec, k *SubmissionKey) []byte {
+	return append(SubmittedIndexPrefix, cdc.MustMarshal(k)...)
 }
 
 func ConfirmedSubmissionsKey(cdc codec.BinaryCodec, k *SubmissionKey) []byte {