From 217066df51e09fca7b12fe5296487970fd076b2a Mon Sep 17 00:00:00 2001 From: kbhat1 Date: Tue, 2 Sep 2025 10:08:55 -0400 Subject: [PATCH 1/5] Surface Read Errors --- ss/pebbledb/db.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ss/pebbledb/db.go b/ss/pebbledb/db.go index b1fe4916..804cf6e8 100644 --- a/ss/pebbledb/db.go +++ b/ss/pebbledb/db.go @@ -307,14 +307,11 @@ func (db *Database) Has(storeKey string, version int64, key []byte) (bool, error func (db *Database) Get(storeKey string, targetVersion int64, key []byte) ([]byte, error) { if targetVersion < db.earliestVersion { - return nil, nil + return nil, errorutils.ErrRecordNotFound } prefixedVal, err := getMVCCSlice(db.storage, storeKey, key, targetVersion) if err != nil { - if errors.Is(err, errorutils.ErrRecordNotFound) { - return nil, nil - } return nil, fmt.Errorf("failed to perform PebbleDB read: %w", err) } @@ -342,7 +339,7 @@ func (db *Database) Get(storeKey string, targetVersion int64, key []byte) ([]byt } // the value is considered deleted - return nil, nil + return nil, errorutils.ErrRecordNotFound } func (db *Database) ApplyChangeset(version int64, cs *proto.NamedChangeSet) error { From 712f10a65d8a77090754156508274fc1bc4f5b9f Mon Sep 17 00:00:00 2001 From: kbhat1 Date: Tue, 2 Sep 2025 10:17:47 -0400 Subject: [PATCH 2/5] Add errors to other backends --- ss/pebbledb/db.go | 1 - ss/rocksdb/db.go | 3 ++- ss/sqlite/db.go | 4 ---- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/ss/pebbledb/db.go b/ss/pebbledb/db.go index 804cf6e8..6f4c0187 100644 --- a/ss/pebbledb/db.go +++ b/ss/pebbledb/db.go @@ -312,7 +312,6 @@ func (db *Database) Get(storeKey string, targetVersion int64, key []byte) ([]byt prefixedVal, err := getMVCCSlice(db.storage, storeKey, key, targetVersion) if err != nil { - return nil, fmt.Errorf("failed to perform PebbleDB read: %w", err) } diff --git a/ss/rocksdb/db.go b/ss/rocksdb/db.go index e4375679..60230e68 100644 --- a/ss/rocksdb/db.go +++ b/ss/rocksdb/db.go @@ -11,6 +11,7 @@ import ( "github.com/linxGnu/grocksdb" "github.com/sei-protocol/sei-db/common/errors" + errorutils "github.com/sei-protocol/sei-db/common/errors" "github.com/sei-protocol/sei-db/config" "github.com/sei-protocol/sei-db/proto" "github.com/sei-protocol/sei-db/ss/types" @@ -150,7 +151,7 @@ func (db *Database) Has(storeKey string, version int64, key []byte) (bool, error func (db *Database) Get(storeKey string, version int64, key []byte) ([]byte, error) { if version < db.tsLow { - return nil, nil + return nil, errorutils.ErrRecordNotFound } slice, err := db.getSlice(storeKey, version, key) diff --git a/ss/sqlite/db.go b/ss/sqlite/db.go index 92e7959c..b3123396 100644 --- a/ss/sqlite/db.go +++ b/ss/sqlite/db.go @@ -172,10 +172,6 @@ func (db *Database) Get(storeKey string, targetVersion int64, key []byte) ([]byt tomb int64 ) if err := stmt.QueryRow(storeKey, key, targetVersion).Scan(&value, &tomb); err != nil { - if errors.Is(err, sql.ErrNoRows) { - return nil, nil - } - return nil, fmt.Errorf("failed to query row: %w", err) } From d8ab7682648366ed09b9a5a632fdab6e354083cd Mon Sep 17 00:00:00 2001 From: kbhat1 Date: Tue, 2 Sep 2025 10:30:20 -0400 Subject: [PATCH 3/5] Update unit tests --- ss/pebbledb/db.go | 2 +- ss/rocksdb/db.go | 2 +- ss/sqlite/db.go | 2 +- ss/test/storage_test_suite.go | 18 ++++++++++-------- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/ss/pebbledb/db.go b/ss/pebbledb/db.go index 6f4c0187..965d0488 100644 --- a/ss/pebbledb/db.go +++ b/ss/pebbledb/db.go @@ -298,7 +298,7 @@ func (db *Database) Has(storeKey string, version int64, key []byte) (bool, error } val, err := db.Get(storeKey, version, key) - if err != nil { + if err != nil && err != errorutils.ErrRecordNotFound { return false, err } diff --git a/ss/rocksdb/db.go b/ss/rocksdb/db.go index 60230e68..210543ec 100644 --- a/ss/rocksdb/db.go +++ b/ss/rocksdb/db.go @@ -142,7 +142,7 @@ func (db *Database) Has(storeKey string, version int64, key []byte) (bool, error } slice, err := db.getSlice(storeKey, version, key) - if err != nil { + if err != nil && err != errorutils.ErrRecordNotFound { return false, err } diff --git a/ss/sqlite/db.go b/ss/sqlite/db.go index b3123396..01e21206 100644 --- a/ss/sqlite/db.go +++ b/ss/sqlite/db.go @@ -148,7 +148,7 @@ func (db *Database) SetEarliestVersion(version int64, ignoreVersion bool) error func (db *Database) Has(storeKey string, version int64, key []byte) (bool, error) { val, err := db.Get(storeKey, version, key) - if err != nil { + if err != nil && err != errorutils.ErrRecordNotFound { return false, err } diff --git a/ss/test/storage_test_suite.go b/ss/test/storage_test_suite.go index ba731a4f..3f77beae 100644 --- a/ss/test/storage_test_suite.go +++ b/ss/test/storage_test_suite.go @@ -141,7 +141,7 @@ func (s *StorageTestSuite) TestDatabaseGetVersionedKey() { // all queries after version 15 should return nil for i := int64(15); i <= 17; i++ { bz, err = db.Get(storeKey1, i, key) - s.Require().NoError(err) + s.Require().Error(err) s.Require().Nil(bz) ok, err = db.Has(storeKey1, i, key) @@ -618,10 +618,11 @@ func (s *StorageTestSuite) TestDatabasePrune() { val := fmt.Sprintf("val%03d-%03d", i, v) bz, err := db.Get(storeKey1, v, []byte(key)) - s.Require().NoError(err) if v <= 25 { + s.Require().Error(err) s.Require().Nil(bz) } else { + s.Require().NoError(err) s.Require().Equal([]byte(val), bz) } } @@ -644,7 +645,7 @@ func (s *StorageTestSuite) TestDatabasePrune() { key := fmt.Sprintf("key%03d", i) bz, err := db.Get(storeKey1, v, []byte(key)) - s.Require().NoError(err) + s.Require().Error(err) s.Require().Nil(bz) } } @@ -688,7 +689,7 @@ func (s *StorageTestSuite) TestDatabasePruneKeepRecent() { // ensure queries for versions 50 and older return nil bz, err := db.Get(storeKey1, 49, key) - s.Require().Nil(err) + s.Require().Error(err) s.Require().Nil(bz) itr, err := db.Iterator(storeKey1, 49, nil, nil) @@ -733,11 +734,11 @@ func (s *StorageTestSuite) TestDatabasePruneKeepLastVersion() { // Verify that all keys before prune height are deleted bz, err := db.Get(storeKey1, 100, []byte("key000")) - s.Require().NoError(err) + s.Require().Error(err) s.Require().Nil(bz) bz, err = db.Get(storeKey1, 160, []byte("key001")) - s.Require().NoError(err) + s.Require().Error(err) s.Require().Nil(bz) // Verify keys after prune height can be retrieved @@ -926,7 +927,7 @@ func (s *StorageTestSuite) TestParallelWriteAndPruning() { // check if the data is pruned version := int64(latestVersion - prunePeriod) val, err := db.Get(storeKey1, version, []byte(fmt.Sprintf("key-%d-%03d", version-1, 0))) - s.Require().Nil(err) + s.Require().Error(err) s.Require().Nil(val) version = int64(latestVersion) @@ -1136,10 +1137,11 @@ func (s *StorageTestSuite) TestParallelIterationAndPruning() { val := fmt.Sprintf("val%03d-%03d", i, v) bz, err := db.Get(storeKey1, v, []byte(key)) - s.Require().NoError(err) if v <= int64(latestVersion-numHeightsPruned) { + s.Require().Error(err) s.Require().Nil(bz) } else { + s.Require().NoError(err) s.Require().Equal([]byte(val), bz) } } From 26ae8603a9fffdaddcfaa7fa058855fcad500d08 Mon Sep 17 00:00:00 2001 From: kbhat1 Date: Tue, 2 Sep 2025 10:50:02 -0400 Subject: [PATCH 4/5] errors.Is --- ss/pebbledb/db.go | 2 +- ss/rocksdb/db.go | 12 ++++++------ ss/sqlite/db.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ss/pebbledb/db.go b/ss/pebbledb/db.go index 965d0488..14dd454a 100644 --- a/ss/pebbledb/db.go +++ b/ss/pebbledb/db.go @@ -298,7 +298,7 @@ func (db *Database) Has(storeKey string, version int64, key []byte) (bool, error } val, err := db.Get(storeKey, version, key) - if err != nil && err != errorutils.ErrRecordNotFound { + if err != nil && !errors.Is(err, errorutils.ErrRecordNotFound) { return false, err } diff --git a/ss/rocksdb/db.go b/ss/rocksdb/db.go index 210543ec..c047c6aa 100644 --- a/ss/rocksdb/db.go +++ b/ss/rocksdb/db.go @@ -6,11 +6,11 @@ package rocksdb import ( "bytes" "encoding/binary" + "errors" "fmt" "sync" "github.com/linxGnu/grocksdb" - "github.com/sei-protocol/sei-db/common/errors" errorutils "github.com/sei-protocol/sei-db/common/errors" "github.com/sei-protocol/sei-db/config" "github.com/sei-protocol/sei-db/proto" @@ -142,7 +142,7 @@ func (db *Database) Has(storeKey string, version int64, key []byte) (bool, error } slice, err := db.getSlice(storeKey, version, key) - if err != nil && err != errorutils.ErrRecordNotFound { + if err != nil && !errors.Is(err, errorutils.ErrRecordNotFound) { return false, err } @@ -208,11 +208,11 @@ func (db *Database) Prune(version int64) error { func (db *Database) Iterator(storeKey string, version int64, start, end []byte) (types.DBIterator, error) { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { - return nil, errors.ErrKeyEmpty + return nil, errorutils.ErrKeyEmpty } if start != nil && end != nil && bytes.Compare(start, end) > 0 { - return nil, errors.ErrStartAfterEnd + return nil, errorutils.ErrStartAfterEnd } prefix := storePrefix(storeKey) @@ -224,11 +224,11 @@ func (db *Database) Iterator(storeKey string, version int64, start, end []byte) func (db *Database) ReverseIterator(storeKey string, version int64, start, end []byte) (types.DBIterator, error) { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { - return nil, errors.ErrKeyEmpty + return nil, errorutils.ErrKeyEmpty } if start != nil && end != nil && bytes.Compare(start, end) > 0 { - return nil, errors.ErrStartAfterEnd + return nil, errorutils.ErrStartAfterEnd } prefix := storePrefix(storeKey) diff --git a/ss/sqlite/db.go b/ss/sqlite/db.go index 01e21206..11dd0fd2 100644 --- a/ss/sqlite/db.go +++ b/ss/sqlite/db.go @@ -148,7 +148,7 @@ func (db *Database) SetEarliestVersion(version int64, ignoreVersion bool) error func (db *Database) Has(storeKey string, version int64, key []byte) (bool, error) { val, err := db.Get(storeKey, version, key) - if err != nil && err != errorutils.ErrRecordNotFound { + if err != nil && !errors.Is(err, errorutils.ErrRecordNotFound) { return false, err } From 542619ff6776cd93001897f94d673b0b25ce63e1 Mon Sep 17 00:00:00 2001 From: kbhat1 Date: Wed, 3 Sep 2025 09:09:33 -0400 Subject: [PATCH 5/5] Assert exact error --- ss/test/storage_test_suite.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/ss/test/storage_test_suite.go b/ss/test/storage_test_suite.go index 3f77beae..bda45da5 100644 --- a/ss/test/storage_test_suite.go +++ b/ss/test/storage_test_suite.go @@ -5,6 +5,7 @@ import ( "sync" "github.com/cosmos/iavl" + errorutils "github.com/sei-protocol/sei-db/common/errors" "github.com/sei-protocol/sei-db/config" "github.com/sei-protocol/sei-db/ss/types" "github.com/stretchr/testify/suite" @@ -141,7 +142,7 @@ func (s *StorageTestSuite) TestDatabaseGetVersionedKey() { // all queries after version 15 should return nil for i := int64(15); i <= 17; i++ { bz, err = db.Get(storeKey1, i, key) - s.Require().Error(err) + s.Require().ErrorIs(err, errorutils.ErrRecordNotFound) s.Require().Nil(bz) ok, err = db.Has(storeKey1, i, key) @@ -619,7 +620,7 @@ func (s *StorageTestSuite) TestDatabasePrune() { bz, err := db.Get(storeKey1, v, []byte(key)) if v <= 25 { - s.Require().Error(err) + s.Require().ErrorIs(err, errorutils.ErrRecordNotFound) s.Require().Nil(bz) } else { s.Require().NoError(err) @@ -645,7 +646,7 @@ func (s *StorageTestSuite) TestDatabasePrune() { key := fmt.Sprintf("key%03d", i) bz, err := db.Get(storeKey1, v, []byte(key)) - s.Require().Error(err) + s.Require().ErrorIs(err, errorutils.ErrRecordNotFound) s.Require().Nil(bz) } } @@ -689,7 +690,7 @@ func (s *StorageTestSuite) TestDatabasePruneKeepRecent() { // ensure queries for versions 50 and older return nil bz, err := db.Get(storeKey1, 49, key) - s.Require().Error(err) + s.Require().ErrorIs(err, errorutils.ErrRecordNotFound) s.Require().Nil(bz) itr, err := db.Iterator(storeKey1, 49, nil, nil) @@ -734,11 +735,11 @@ func (s *StorageTestSuite) TestDatabasePruneKeepLastVersion() { // Verify that all keys before prune height are deleted bz, err := db.Get(storeKey1, 100, []byte("key000")) - s.Require().Error(err) + s.Require().ErrorIs(err, errorutils.ErrRecordNotFound) s.Require().Nil(bz) bz, err = db.Get(storeKey1, 160, []byte("key001")) - s.Require().Error(err) + s.Require().ErrorIs(err, errorutils.ErrRecordNotFound) s.Require().Nil(bz) // Verify keys after prune height can be retrieved @@ -927,7 +928,7 @@ func (s *StorageTestSuite) TestParallelWriteAndPruning() { // check if the data is pruned version := int64(latestVersion - prunePeriod) val, err := db.Get(storeKey1, version, []byte(fmt.Sprintf("key-%d-%03d", version-1, 0))) - s.Require().Error(err) + s.Require().ErrorIs(err, errorutils.ErrRecordNotFound) s.Require().Nil(val) version = int64(latestVersion) @@ -1138,7 +1139,7 @@ func (s *StorageTestSuite) TestParallelIterationAndPruning() { bz, err := db.Get(storeKey1, v, []byte(key)) if v <= int64(latestVersion-numHeightsPruned) { - s.Require().Error(err) + s.Require().ErrorIs(err, errorutils.ErrRecordNotFound) s.Require().Nil(bz) } else { s.Require().NoError(err)