Skip to content

Conversation

@yzang2019
Copy link
Collaborator

Describe your changes and provide context

This PR add an in memory latestVersion for each database, change interface to handle getLatestVersion always from in memory to avoid any I/O read.

Add logic to retrieve latestVersion during database initialization.

Testing performed to validate your change

Added and fix unit test for latestVersion

@yzang2019 yzang2019 requested a review from blindchaser October 8, 2025 16:21
@codecov
Copy link

codecov bot commented Oct 8, 2025

Codecov Report

❌ Patch coverage is 38.19444% with 89 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.06%. Comparing base (c3ee918) to head (10e313a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
ss/test/storage_test_suite.go 0.00% 47 Missing ⚠️
ss/rocksdb/db.go 54.00% 19 Missing and 4 partials ⚠️
ss/pebbledb/db.go 59.09% 14 Missing and 4 partials ⚠️
ss/pruning/manager.go 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (38.19%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project status has failed because the head coverage (40.06%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
+ Coverage   40.00%   40.06%   +0.06%     
==========================================
  Files          59       59              
  Lines        7510     7533      +23     
==========================================
+ Hits         3004     3018      +14     
- Misses       4161     4168       +7     
- Partials      345      347       +2     
Files with missing lines Coverage Δ
ss/store.go 50.00% <100.00%> (+2.11%) ⬆️
ss/types/store.go 0.00% <ø> (ø)
ss/pruning/manager.go 0.00% <0.00%> (ø)
ss/pebbledb/db.go 56.32% <59.09%> (+0.87%) ⬆️
ss/rocksdb/db.go 48.27% <54.00%> (+3.26%) ⬆️
ss/test/storage_test_suite.go 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.


for _, kvPair := range cs.Changeset.Pairs {
if kvPair.Value == nil {
if err := b.Delete(cs.Name, kvPair.Key); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a general practice i recommend against overriding error like this, specially in cases like this since the scope of err is tighly limited to the if block. Ditto for next if statement.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks for pointing that out!


return b.Write()
// Update latest version
err = b.Write()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto re keeping the scope of err variable tight.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

func (db *Database) GetLatestMigratedModule() (string, error) {
//TODO implement me
panic("implement me")
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking out loud/unrelated to this PR: when was the last time we used the sqlite backing store? If heavily unused, my vote is to remove it entirely.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea agreed, we don’t use it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna do that in a separate PR

@yzang2019 yzang2019 merged commit 3c38a45 into main Oct 8, 2025
8 of 10 checks passed
@yzang2019 yzang2019 deleted the yzang/add-watermark branch October 8, 2025 22:40
jewei1997 added a commit to sei-protocol/sei-chain that referenced this pull request Oct 27, 2025
## Describe your changes and provide context

A common workflow that users do is to query the latest height and then
query information about that height such as events/logs. However, since
we have an option to do async writes for receipts, the events/logs may
not be available yet for that height, causing the query to fail.

In general, we want all of our heights outputted by our RPC endpoints to
be aligned with each other. We have introduced the concept of a
watermark (sei-protocol/sei-db#116) across
Tendermint block store, SS Store, and receipt store. Pulling this
watermark across all 3 dbs should allow the evmrpc to output the minimum
height available across these dbs for a latest height. This will ensure
that all data for a given height outputted by our RPCs is available
across all our endpoints.

This PR introduces a WatermarkManager that manages the heights of these
DBs, which the evmrpc endpoints can use to get the latest heights that
are ready.

## Testing performed to validate your change
unit tests + manual testing
jewei1997 pushed a commit that referenced this pull request Nov 6, 2025
## Describe your changes and provide context
This PR add an in memory latestVersion for each database, change
interface to handle getLatestVersion always from in memory to avoid any
I/O read.

Add logic to retrieve latestVersion during database initialization.

## Testing performed to validate your change
Added and fix unit test for latestVersion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants