Skip to content
This repository was archived by the owner on Jan 20, 2026. It is now read-only.

Fix Panic: Check For Error Before Defer#482

Merged
Kbhat1 merged 1 commit intoseiv2from
CheckErrBeforeDefer
Apr 10, 2024
Merged

Fix Panic: Check For Error Before Defer#482
Kbhat1 merged 1 commit intoseiv2from
CheckErrBeforeDefer

Conversation

@Kbhat1
Copy link
Contributor

@Kbhat1 Kbhat1 commented Apr 9, 2024

Describe your changes and provide context

  • Checks for err before defer
  • Without this, if there is a err, the node will panic on the defer because scStore is nil

Testing performed to validate your change

  • Verified on node

@Kbhat1 Kbhat1 changed the title Check err before defer Check For Error Before Defer Apr 9, 2024
@Kbhat1 Kbhat1 requested a review from yzang2019 April 9, 2024 22:49
@Kbhat1 Kbhat1 changed the title Check For Error Before Defer Fix Panic: Check For Error Before Defer Apr 9, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 55.35%. Comparing base (35e6b74) to head (b8bd97d).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##            seiv2     #482      +/-   ##
==========================================
- Coverage   55.35%   55.35%   -0.01%     
==========================================
  Files         629      629              
  Lines       53963    53963              
==========================================
- Hits        29872    29871       -1     
- Misses      21957    21958       +1     
  Partials     2134     2134              
Files Coverage Δ
storev2/rootmulti/store.go 3.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

@Kbhat1 Kbhat1 merged commit 7fd82bd into seiv2 Apr 10, 2024
@Kbhat1 Kbhat1 deleted the CheckErrBeforeDefer branch April 10, 2024 17:29
udpatil pushed a commit that referenced this pull request Apr 16, 2024
## Describe your changes and provide context
- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

## Testing performed to validate your change
- Verified on node
udpatil pushed a commit that referenced this pull request Apr 19, 2024
## Describe your changes and provide context
- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

## Testing performed to validate your change
- Verified on node
udpatil pushed a commit that referenced this pull request Apr 19, 2024
## Describe your changes and provide context
- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

## Testing performed to validate your change
- Verified on node
yzang2019 added a commit that referenced this pull request Mar 26, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
yzang2019 added a commit that referenced this pull request Mar 26, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Kbhat1 pushed a commit that referenced this pull request Mar 30, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Kbhat1 pushed a commit that referenced this pull request Apr 16, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
yzang2019 added a commit that referenced this pull request Apr 21, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes

Remove noVersioning and add packages
Kbhat1 pushed a commit that referenced this pull request Apr 22, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Kbhat1 pushed a commit that referenced this pull request Apr 23, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Kbhat1 pushed a commit that referenced this pull request May 30, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Kbhat1 pushed a commit that referenced this pull request Jun 9, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Kbhat1 pushed a commit that referenced this pull request Jun 13, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Kbhat1 pushed a commit that referenced this pull request Jun 16, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Kbhat1 pushed a commit that referenced this pull request Jun 20, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Kbhat1 pushed a commit that referenced this pull request Jun 30, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Kbhat1 pushed a commit that referenced this pull request Jul 7, 2025
[SeiDB] Part2:Add io.closer interface for commit multistore (#374)

Problem:
With SeiDB, we need to support closing the database when self
remediation is triggered, otherwise the chain will halt due to not able
to reopen the database.

Changes:
- Add an interface for CMS (commit multistore)
- Add implementation for existing cms
- Add close logic for self remediation to close the commit store and
state store

Tested with local docker chain

[SeiDB] Part3: Adjust Snapshot and Pruning logic for storeV2 (#376)

Changes:
- Adjust and fix pruning and snapshot logic which could break seidb
- Add some logging for snapshot manager

Covered by unit test and local docker cluster

SeiDB part 4

SeiDB part 5

[SeiDB] Fix concurrent map access (#411)

This should fix the concurrent map access for storeV2 root multistore
over (rs.ckvStores). The problem is that it is currently not protected
by a read lock so when other goroutine modify the map, it will throw
panic

[SeiDB] Fix various issues from ottersec audit (#418)

Fix a bunch of minor issues from audit:
- LoadVersionAndUpgrade should not ignore version
- Simplify CacheWrap to avoid wrapping twice
- SS store query is not setting height correctly in the query response
- Add panic logic for bumpversion if it is false in commit
- Use the correct commit info in query

[SeiDB] Fix SS apply changeset version off by 1 (#424)

Problem:
When apply changeset to SS store, currently we are using the
lastCommitInfo version, which is the previous block height. This means
the version is off by 1. The correct way is to use the current block
height for the changeset version
Will add integration test for this test case

SeiDB fixes

Add metrics for storeV2 (#456)

Add two new metrics for StoreV2:
- Add metric to monitor SC commit latency
- Add metric to monitor SS store last commit version

Tested locally and see metrics appear

Fix Panic: Check For Error Before Defer (#482)

- Checks for err before `defer`
- Without this, if there is a err, the node will panic on the `defer`
because `scStore` is `nil`

- Verified on node

Add validation to prevent data corruption due to SS store misoperation (#503)

Problem:
Currently if a node did state sync with SS disabled, and then we turn on
SS, it will behave wrong due to SS not having initial version for most
keys and cause data corruption. And if that node tries to create a
snapshot, it will produce a bad snapshot which could cause other nodes
to app hash.

Solution:
In this PR, we simply add a validation check to make sure if SS is
suddenly enabled, and SS doesn't have any data yet, SC must also not
have any data, otherwise, we will panic during startup.

Tested on arctic-1:
<img width="887" alt="image"
src="https://github.com/sei-protocol/sei-cosmos/assets/50607998/9bc1a67b-7b11-4254-96a2-9e155239bb85">

Enabled SS first, hit panic, and then disable SS again, node was running
fine.

Bump seidb version

SeiDB fixes
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants