implement CommitKVStore using a migration router

sei-db/state_db/sc/migration/router_kvstore.go

@@ -0,0 +1,115 @@
+package migration
+
+import (
+	"context"
+	"fmt"
+
+	ics23 "github.com/confio/ics23/go"
+	"github.com/sei-protocol/sei-chain/sei-db/proto"
+	"github.com/sei-protocol/sei-chain/sei-db/state_db/sc/types"
+	db "github.com/tendermint/tm-db"
+)
+
+// rootHashSize matches the digest length used by the other CommitKVStore
+// implementations in this codebase: memiavl returns sha256 (32 B) and flatkv
+// returns Blake3-256 (32 B).
+const rootHashSize = 32
+
+var _ types.CommitKVStore = (*RouterCommitKVStore)(nil)
+
+// RouterCommitKVStore adapts a [Router] (which is keyed by store name on every
+// call) to the store-name-less [types.CommitKVStore] interface by binding the
+// view to a single module store name.
+//
+// The CommitKVStore interface does not return errors. Any error returned by the
+// underlying router is therefore surfaced as a panic. This is a short-term
+// limitation; the long-term plan is to plumb errors through the interface.
+type RouterCommitKVStore struct {
+	router          Router
+	storeName       string
+	versionProvider func() int64
+}
+
+func NewRouterCommitKVStore(
+	router Router,
+	storeName string,
+	versionProvider func() int64,
+) *RouterCommitKVStore {
+	return &RouterCommitKVStore{
+		router:          router,
+		storeName:       storeName,
+		versionProvider: versionProvider,
+	}
+}
+
+// Close is illegal during the standard CommitKVStore lifecycle for this type:
+// the wrapped Router is owned by the caller and must outlive this view.
+func (r *RouterCommitKVStore) Close() error {
+	return fmt.Errorf("RouterCommitKVStore.Close: illegal during standard lifecycle")
+}
+
+func (r *RouterCommitKVStore) Get(key []byte) []byte {
+	value, _, err := r.router.Read(r.storeName, key)
+	if err != nil {
+		panic(fmt.Errorf("RouterCommitKVStore.Get(store=%q): %w", r.storeName, err))
+	}
+	return value
+}
+
+func (r *RouterCommitKVStore) Has(key []byte) bool {
+	_, found, err := r.router.Read(r.storeName, key)
+	if err != nil {
+		panic(fmt.Errorf("RouterCommitKVStore.Has(store=%q): %w", r.storeName, err))
+	}
+	return found
+}
+
+func (r *RouterCommitKVStore) Set(key []byte, value []byte) {
+	r.applyOne(&proto.KVPair{Key: key, Value: value})
+}
+
+func (r *RouterCommitKVStore) Remove(key []byte) {
+	r.applyOne(&proto.KVPair{Key: key, Delete: true})
+}
+
+// applyOne dispatches a single KV change as a one-pair NamedChangeSet through
+// the router, panicking on any router error.
+func (r *RouterCommitKVStore) applyOne(pair *proto.KVPair) {
+	cs := []*proto.NamedChangeSet{{
+		Name:      r.storeName,
+		Changeset: proto.ChangeSet{Pairs: []*proto.KVPair{pair}},
+	}}
+	if err := r.router.ApplyChangeSets(context.Background(), cs); err != nil {
+		panic(fmt.Errorf("RouterCommitKVStore.ApplyChangeSets(store=%q): %w", r.storeName, err))
+	}
+}
+
+func (r *RouterCommitKVStore) Iterator(start []byte, end []byte, ascending bool) db.Iterator {
+	it, err := r.router.Iterator(r.storeName, start, end, ascending)
+	if err != nil {
+		panic(fmt.Errorf("RouterCommitKVStore.Iterator(store=%q): %w", r.storeName, err))
+	}
+	return it
+}
+
+func (r *RouterCommitKVStore) GetProof(key []byte) *ics23.CommitmentProof {
+	proof, err := r.router.GetProof(r.storeName, key)
+	if err != nil {
+		panic(fmt.Errorf("RouterCommitKVStore.GetProof(store=%q): %w", r.storeName, err))
+	}
+	return proof
+}
+
+// RootHash is a placeholder that returns a fresh zeroed 32-byte slice on every
+// call. The CommitKVStore contract permits callers to mutate the returned
+// slice, so a fresh allocation is required to keep the placeholder safe.
+//
+// TODO: revisit before shipping to production once the production usage of
+// RootHash() across this code path is understood.
+func (r *RouterCommitKVStore) RootHash() []byte {
+	return make([]byte, rootHashSize)
+}
+
+func (r *RouterCommitKVStore) Version() int64 {
+	return r.versionProvider()
+}

sei-db/state_db/sc/migration/router_kvstore_test.go

@@ -0,0 +1,215 @@
+package migration
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	ics23 "github.com/confio/ics23/go"
+	"github.com/sei-protocol/sei-chain/sei-db/proto"
+	"github.com/stretchr/testify/require"
+	dbm "github.com/tendermint/tm-db"
+)
+
+const testRouterStoreName = "bank"
+
+// newRouterCommitKVStoreForTest returns a RouterCommitKVStore wrapping a fresh
+// TestInMemoryRouter with a constant version. Tests that need a different
+// inner router or a non-constant versionProvider construct the store
+// directly.
+func newRouterCommitKVStoreForTest(t *testing.T, version int64) (*RouterCommitKVStore, *TestInMemoryRouter) {
+	t.Helper()
+	inner := NewTestInMemoryRouter()
+	store := NewRouterCommitKVStore(inner, testRouterStoreName, func() int64 { return version })
+	return store, inner
+}
+
+func TestRouterCommitKVStore_GetReturnsValueWrittenViaRouter(t *testing.T) {
+	store, inner := newRouterCommitKVStoreForTest(t, 0)
+	require.NoError(t, inner.ApplyChangeSets(context.Background(), []*proto.NamedChangeSet{{
+		Name: testRouterStoreName,
+		Changeset: proto.ChangeSet{Pairs: []*proto.KVPair{
+			{Key: []byte("k"), Value: []byte("v")},
+		}},
+	}}))
+
+	require.Equal(t, []byte("v"), store.Get([]byte("k")))
+	require.True(t, store.Has([]byte("k")))
+}
+
+func TestRouterCommitKVStore_GetMissingKeyReturnsNil(t *testing.T) {
+	store, _ := newRouterCommitKVStoreForTest(t, 0)
+	require.Nil(t, store.Get([]byte("missing")))
+	require.False(t, store.Has([]byte("missing")))
+}
+
+func TestRouterCommitKVStore_SetWritesViaRouter(t *testing.T) {
+	store, inner := newRouterCommitKVStoreForTest(t, 0)
+	store.Set([]byte("k"), []byte("v"))
+
+	val, found, err := inner.Read(testRouterStoreName, []byte("k"))
+	require.NoError(t, err)
+	require.True(t, found)
+	require.Equal(t, []byte("v"), val)
+}
+
+func TestRouterCommitKVStore_RemoveDeletesViaRouter(t *testing.T) {
+	store, inner := newRouterCommitKVStoreForTest(t, 0)
+	store.Set([]byte("k"), []byte("v"))
+	store.Remove([]byte("k"))
+
+	val, found, err := inner.Read(testRouterStoreName, []byte("k"))
+	require.NoError(t, err)
+	require.False(t, found)
+	require.Nil(t, val)
+}
+
+// TestRouterCommitKVStore_BindsToSingleStoreName confirms that two wrappers
+// pointed at the same router but bound to different module names see only
+// their own data, and writes land under the configured store name in the
+// underlying router.
+func TestRouterCommitKVStore_BindsToSingleStoreName(t *testing.T) {
+	inner := NewTestInMemoryRouter()
+	bankStore := NewRouterCommitKVStore(inner, "bank", func() int64 { return 0 })
+	evmStore := NewRouterCommitKVStore(inner, "evm", func() int64 { return 0 })
+
+	bankStore.Set([]byte("k"), []byte("from-bank"))
+	evmStore.Set([]byte("k"), []byte("from-evm"))
+
+	require.Equal(t, []byte("from-bank"), bankStore.Get([]byte("k")))
+	require.Equal(t, []byte("from-evm"), evmStore.Get([]byte("k")))
+
+	val, found, err := inner.Read("bank", []byte("k"))
+	require.NoError(t, err)
+	require.True(t, found)
+	require.Equal(t, []byte("from-bank"), val)
+
+	val, found, err = inner.Read("evm", []byte("k"))
+	require.NoError(t, err)
+	require.True(t, found)
+	require.Equal(t, []byte("from-evm"), val)
+}
+
+// TestRouterCommitKVStore_VersionInvokesProviderEachCall confirms that the
+// versionProvider lambda is consulted on every Version() call rather than
+// captured once at construction time, so callers can swap the value at
+// runtime.
+func TestRouterCommitKVStore_VersionInvokesProviderEachCall(t *testing.T) {
+	current := int64(7)
+	store := NewRouterCommitKVStore(NewTestInMemoryRouter(), testRouterStoreName, func() int64 { return current })
+
+	require.Equal(t, int64(7), store.Version())
+	current = 42
+	require.Equal(t, int64(42), store.Version())
+}
+
+// TestRouterCommitKVStore_RootHashIs32ZeroBytes locks in the placeholder
+// contract: 32 bytes, all zero, freshly allocated on every call so that a
+// caller mutating the returned slice cannot corrupt subsequent reads.
+func TestRouterCommitKVStore_RootHashIs32ZeroBytes(t *testing.T) {
+	store, _ := newRouterCommitKVStoreForTest(t, 0)
+	hash := store.RootHash()
+	require.Len(t, hash, 32)
+	require.Equal(t, make([]byte, 32), hash)
+
+	hash[0] = 0xFF
+	hash2 := store.RootHash()
+	require.Len(t, hash2, 32)
+	require.Equal(t, make([]byte, 32), hash2)
+}
+
+// TestRouterCommitKVStore_CloseReturnsError locks in that Close is illegal
+// during the standard lifecycle: the wrapped Router is owned by the caller
+// and must outlive this view, so Close surfaces a non-nil error rather than
+// performing any teardown.
+func TestRouterCommitKVStore_CloseReturnsError(t *testing.T) {
+	store, _ := newRouterCommitKVStoreForTest(t, 0)
+	err := store.Close()
+	require.EqualError(t, err, "RouterCommitKVStore.Close: illegal during standard lifecycle")
+}
+
+func TestRouterCommitKVStore_IteratorPanicsOnRouterError(t *testing.T) {
+	// TestInMemoryRouter.Iterator always returns an error; the wrapper must
+	// surface that as a panic.
+	store, _ := newRouterCommitKVStoreForTest(t, 0)
+	require.Panics(t, func() { _ = store.Iterator(nil, nil, true) })
+}
+
+func TestRouterCommitKVStore_GetProofPanicsOnRouterError(t *testing.T) {
+	// TestInMemoryRouter.GetProof always returns an error; the wrapper must
+	// surface that as a panic.
+	store, _ := newRouterCommitKVStoreForTest(t, 0)
+	require.Panics(t, func() { _ = store.GetProof([]byte("k")) })
+}
+
+// failingRouter is a Router whose Read and ApplyChangeSets return injected
+// sentinel errors. It exists so we can exercise the panic-on-error path for
+// the methods that TestInMemoryRouter implements without errors. Iterator and
+// GetProof always return a not-implemented error and are not used by these
+// tests; TestInMemoryRouter already covers their panic paths.
+type failingRouter struct {
+	readErr  error
+	writeErr error
+}
+
+var _ Router = (*failingRouter)(nil)
+
+func (f *failingRouter) Read(string, []byte) ([]byte, bool, error) {
+	return nil, false, f.readErr
+}
+
+func (f *failingRouter) ApplyChangeSets(context.Context, []*proto.NamedChangeSet) error {
+	return f.writeErr
+}
+
+func (f *failingRouter) Iterator(string, []byte, []byte, bool) (dbm.Iterator, error) {
+	return nil, errors.New("failingRouter.Iterator: not used by these tests")
+}
+
+func (f *failingRouter) GetProof(string, []byte) (*ics23.CommitmentProof, error) {
+	return nil, errors.New("failingRouter.GetProof: not used by these tests")
+}
+
+func TestRouterCommitKVStore_GetPanicsOnRouterError(t *testing.T) {
+	store := NewRouterCommitKVStore(
+		&failingRouter{readErr: errors.New("boom")},
+		testRouterStoreName,
+		func() int64 { return 0 },
+	)
+	require.PanicsWithError(t, `RouterCommitKVStore.Get(store="bank"): boom`, func() {
+		_ = store.Get([]byte("k"))
+	})
+}
+
+func TestRouterCommitKVStore_HasPanicsOnRouterError(t *testing.T) {
+	store := NewRouterCommitKVStore(
+		&failingRouter{readErr: errors.New("boom")},
+		testRouterStoreName,
+		func() int64 { return 0 },
+	)
+	require.PanicsWithError(t, `RouterCommitKVStore.Has(store="bank"): boom`, func() {
+		_ = store.Has([]byte("k"))
+	})
+}
+
+func TestRouterCommitKVStore_SetPanicsOnRouterError(t *testing.T) {
+	store := NewRouterCommitKVStore(
+		&failingRouter{writeErr: errors.New("boom")},
+		testRouterStoreName,
+		func() int64 { return 0 },
+	)
+	require.PanicsWithError(t, `RouterCommitKVStore.ApplyChangeSets(store="bank"): boom`, func() {
+		store.Set([]byte("k"), []byte("v"))
+	})
+}
+
+func TestRouterCommitKVStore_RemovePanicsOnRouterError(t *testing.T) {
+	store := NewRouterCommitKVStore(
+		&failingRouter{writeErr: errors.New("boom")},
+		testRouterStoreName,
+		func() int64 { return 0 },
+	)
+	require.PanicsWithError(t, `RouterCommitKVStore.ApplyChangeSets(store="bank"): boom`, func() {
+		store.Remove([]byte("k"))
+	})
+}

sei-db/state_db/sc/types/types.go

@@ -43,23 +43,36 @@ type Committer interface {
 	io.Closer
 }
 
+// Planed for future deprecation but not yet deprecated.
 type CommitKVStore interface {
+	// Planed for future deprecation but not yet deprecated.
+	// This is the proper method to call to get a value from the store.
 	Get(key []byte) []byte
 
+	// Planed for future deprecation but not yet deprecated. This is the proper method to call to check
+	// if a key exists in the store.
 	Has(key []byte) bool
 
+	// Deprected: do not call in production code, use CommitKVStore.ApplyChangeSets() instead.
 	Set(key, value []byte)
 
+	// Deprected: do not call in production code, use CommitKVStore.ApplyChangeSets() instead.
 	Remove(key []byte)
 
+	// Deprected but safe to call
 	Version() int64
 
+	// Deprected: do not call in production code, may panic on stores backed by flatKV
 	RootHash() []byte
 
+	// Partially deprecated: may panic if called on a store that does not support iteration (e.g. evm/ after migration)
 	Iterator(start, end []byte, ascending bool) dbm.Iterator
 
+	// Partially deprecated: may panic if called on a store that does not support proofs (e.g. evm/ after migration)
 	GetProof(key []byte) *ics23.CommitmentProof
 
+	// deprecated: some implementations always return errors, and ones that don't mean you are closing something
+	// that shouldn't be closed directly
 	io.Closer
 }