Don't panic during any store operations.

Author: DimitrisJim

modules/light-clients/08-wasm/types/export_test.go

@@ -32,7 +32,7 @@ func NewMigrateProposalWrappedStore(subjectStore, substituteStore storetypes.KVS
 }
 
 // GetStore is a wrapper around getStore to allow the function to be directly called in tests.
-func (ws migrateClientWrappedStore) GetStore(key []byte) storetypes.KVStore {
+func (ws migrateClientWrappedStore) GetStore(key []byte) (bool, storetypes.KVStore) {
 	return ws.getStore(key)
 }
 

modules/light-clients/08-wasm/types/store.go

@@ -3,7 +3,6 @@ package types
 import (
 	"bytes"
 	"errors"
-	"fmt"
 	"io"
 	"reflect"
 	"strings"
@@ -44,72 +43,95 @@ func newMigrateClientWrappedStore(subjectStore, substituteStore storetypes.KVSto
 
 // Get implements the storetypes.KVStore interface. It allows reads from both the subjectStore and substituteStore.
 //
-// Get will panic if the key is not prefixed with either "subject/" or "substitute/".
+// Get will return an empty byte slice if the key is not prefixed with either "subject/" or "substitute/".
 func (ws migrateClientWrappedStore) Get(key []byte) []byte {
 	prefix, key := splitPrefix(key)
+	found, store := ws.getStore(prefix)
+	if !found {
+		// return a nil as KVStore.Get() does by default
+		return []byte(nil)
+	}
 
-	return ws.getStore(prefix).Get(key)
+	return store.Get(key)
 }
 
 // Has implements the storetypes.KVStore interface. It allows reads from both the subjectStore and substituteStore.
 //
 // Note: contracts do not have access to the Has method, it is only implemented here to satisfy the storetypes.KVStore interface.
 func (ws migrateClientWrappedStore) Has(key []byte) bool {
 	prefix, key := splitPrefix(key)
+	found, store := ws.getStore(prefix)
+	if !found {
+		// return false as value when store is not found
+		return false
+	}
 
-	return ws.getStore(prefix).Has(key)
+	return store.Has(key)
 }
 
 // Set implements the storetypes.KVStore interface. It allows writes solely to the subjectStore.
 //
-// Set will panic if the key is not prefixed with "subject/".
+// Set will no-op if the key is not prefixed with "subject/".
 func (ws migrateClientWrappedStore) Set(key, value []byte) {
 	prefix, key := splitPrefix(key)
 	if !bytes.Equal(prefix, subjectPrefix) {
-		panic(fmt.Errorf("writes only allowed on subject store; key must be prefixed with \"%s\"", subjectPrefix))
+		return // no-op
 	}
 
 	ws.subjectStore.Set(key, value)
 }
 
 // Delete implements the storetypes.KVStore interface. It allows deletions solely to the subjectStore.
 //
-// Delete will panic if the key is not prefixed with "subject/".
+// Delete will no-op if the key is not prefixed with "subject/".
 func (ws migrateClientWrappedStore) Delete(key []byte) {
 	prefix, key := splitPrefix(key)
 	if !bytes.Equal(prefix, subjectPrefix) {
-		panic(fmt.Errorf("writes only allowed on subject store; key must be prefixed with \"%s\"", subjectPrefix))
+		return // no-op
 	}
 
 	ws.subjectStore.Delete(key)
 }
 
 // Iterator implements the storetypes.KVStore interface. It allows iteration over both the subjectStore and substituteStore.
 //
-// Iterator will panic if the start or end keys are not prefixed with either "subject/" or "substitute/".
+// Iterator will return a closed iterator if the start or end keys are not prefixed with either "subject/" or "substitute/".
 func (ws migrateClientWrappedStore) Iterator(start, end []byte) storetypes.Iterator {
 	prefixStart, start := splitPrefix(start)
 	prefixEnd, end := splitPrefix(end)
 
 	if !bytes.Equal(prefixStart, prefixEnd) {
-		panic(errors.New("start and end keys must be prefixed with the same prefix"))
+		// return an invalid iterator
+		return ws.closedIterator()
+	}
+	found, store := ws.getStore(prefixStart)
+	if !found {
+		// return an invalid iterator
+		return ws.closedIterator()
 	}
 
-	return ws.getStore(prefixStart).Iterator(start, end)
+	return store.Iterator(start, end)
 }
 
 // ReverseIterator implements the storetypes.KVStore interface. It allows iteration over both the subjectStore and substituteStore.
 //
-// ReverseIterator will panic if the start or end keys are not prefixed with either "subject/" or "substitute/".
+// ReverseIterator will return a closed iterator if the start or end keys are not prefixed with either "subject/" or "substitute/".
 func (ws migrateClientWrappedStore) ReverseIterator(start, end []byte) storetypes.Iterator {
 	prefixStart, start := splitPrefix(start)
 	prefixEnd, end := splitPrefix(end)
 
 	if !bytes.Equal(prefixStart, prefixEnd) {
-		panic(errors.New("start and end keys must be prefixed with the same prefix"))
+		// return an invalid iterator
+		return ws.closedIterator()
+	}
+
+	found, store := ws.getStore(prefixStart)
+	if !found {
+		// return an invalid iterator
+		return ws.closedIterator()
 	}
 
-	return ws.getStore(prefixStart).ReverseIterator(start, end)
+	return store.ReverseIterator(start, end)
 }
 
 // GetStoreType implements the storetypes.KVStore interface, it is implemented solely to satisfy the interface.
@@ -131,14 +153,25 @@ func (ws migrateClientWrappedStore) CacheWrapWithTrace(w io.Writer, tc storetype
 // is returned. If the key is prefixed with "substitute/", the substituteStore is returned.
 //
 // If the key is not prefixed with either "subject/" or "substitute/", a panic is thrown.
-func (ws migrateClientWrappedStore) getStore(prefix []byte) storetypes.KVStore {
+func (ws migrateClientWrappedStore) getStore(prefix []byte) (bool, storetypes.KVStore) {
 	if bytes.Equal(prefix, subjectPrefix) {
-		return ws.subjectStore
+		return true, ws.subjectStore
 	} else if bytes.Equal(prefix, substitutePrefix) {
-		return ws.substituteStore
+		return true, ws.substituteStore
 	}
 
-	panic(fmt.Errorf("key must be prefixed with either \"%s\" or \"%s\"", subjectPrefix, substitutePrefix))
+	return false, nil
+}
+
+// closedIterator returns an iterator that is always closed, used when ws.Iterator() or ws.ReverseIterator() is called
+// with invalid prefixes.
+// TODO(jim): could alternatively create an invalid iterator, callers should always check for validity before using.
+func (ws migrateClientWrappedStore) closedIterator() storetypes.Iterator {
+	// Create a dummy iterator that is always closed right away.
+	it := ws.subjectStore.Iterator([]byte{0}, []byte{1})
+	it.Close()
+
+	return it
 }
 
 // splitPrefix splits the key into the prefix and the key itself, if the key is prefixed with either "subject/" or "substitute/".