Skip to content

Reject legacy snapshots #1774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: js/tracing-forkers
Choose a base branch
from
+35 −3
Open

Reject legacy snapshots #1774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions ouroboros-consensus/changelog.d/20251124_165114_javier.sagredo_reject_legacy.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!--
A new scriv changelog fragment.

Uncomment the section that is right (remove the HTML comment wrapper).
For top level release notes, leave all the headers commented out.
-->

<!--
### Patch

- A bullet item for the Patch category.

-->
<!--
### Non-Breaking

- A bullet item for the Non-Breaking category.

-->
### Breaking

- Legacy snapshots will be rejected and deleted, instead of crashing consensus.
Copy link
Contributor

@geo2a geo2a Nov 26, 2025

Choose a reason for hiding this comment

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

From the code, I see that we check if the snapshot is a file and, if so, infer that it is in the legacy format and throw and exception. Is the deletion already done when we handle this exception? Where can I find this code, if it exists?

Copy link
Contributor

@geo2a geo2a Nov 26, 2025

Choose a reason for hiding this comment

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

Ah I see, it's not in Consensus, the exception need to be handled in the node. In that case, I think this changelog entry needs to be reformulated:

Suggested change
- Legacy snapshots will be rejected and deleted, instead of crashing consensus.
- Legacy snapshots will be rejected instead of crashing Consensus, and an error will be signaled upstream, so that `cardano-node` could choose what to do with the incompatible snapshot.

Copy link
Contributor Author

@jasagredo jasagredo Nov 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The logic is here:
https://github.com/IntersectMBO/ouroboros-consensus/blob/main/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/LedgerDB/API.hs#L596-L601

We delete the snapshot if we get an InitFailureRead as we do here.

Copy link
Contributor

@geo2a geo2a Nov 26, 2025

Choose a reason for hiding this comment

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

Right. I think I like the approach of emitting a warning and suggesting the convert the snapshot, as you've described in #1708, but if you think that deleting is fine, then let's delete.

4 changes: 3 additions & 1 deletion ...oros-consensus/src/ouroboros-consensus-lsm/Ouroboros/Consensus/Storage/LedgerDB/V2/LSM.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,10 @@ loadSnapshot ::
Session m ->
DiskSnapshot ->
ExceptT (SnapshotFailure blk) m (LedgerSeq' m blk, RealPoint blk)
loadSnapshot tracer rr ccfg fs session ds =
loadSnapshot tracer rr ccfg fs@(SomeHasFS hfs) session ds =
do
fileEx <- lift $ doesFileExist hfs (snapshotToDirPath ds)
Monad.when fileEx $ throwE $ InitFailureRead ReadSnapshotIsLegacy
snapshotMeta <-
withExceptT (InitFailureRead . ReadMetadataError (snapshotToMetadataPath ds)) $
loadSnapshotMetadata fs ds
Expand Down
2 changes: 2 additions & 0 deletions ...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/LedgerDB/Snapshots.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ data ReadSnapshotErr
ReadSnapshotDataCorruption
| -- | An error occurred while reading the snapshot metadata file
ReadMetadataError FsPath MetadataErr
| -- | We were given a legacy snapshot
ReadSnapshotIsLegacy
deriving (Eq, Show)

data TablesCodecVersion = TablesCodecVersion1
Expand Down
5 changes: 4 additions & 1 deletion ...os-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/LedgerDB/V1/Snapshots.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ import Ouroboros.Consensus.Util.Args (Complete)
import Ouroboros.Consensus.Util.Enclose
import Ouroboros.Consensus.Util.IOLike
import System.FS.API
import qualified System.FS.API as FS

snapshotManager ::
( IOLike m
Expand Down Expand Up @@ -293,7 +294,9 @@ loadSnapshot ::
(SnapshotFailure blk)
m
((DbChangelog' blk, ResourceKey m, LedgerBackingStore m (ExtLedgerState blk)), RealPoint blk)
loadSnapshot tracer bArgs@(SomeBackendArgs bss) ccfg fs@(SnapshotsFS fs') reg s = do
loadSnapshot tracer bArgs@(SomeBackendArgs bss) ccfg fs@(SnapshotsFS fs'@(SomeHasFS hfs)) reg s = do
fileEx <- Trans.lift $ FS.doesFileExist hfs (snapshotToDirPath s)
Monad.when fileEx $ throwError $ InitFailureRead ReadSnapshotIsLegacy
(extLedgerSt, checksumAsRead) <-
withExceptT (InitFailureRead . ReadSnapshotFailed) $
readExtLedgerState fs' (decodeDiskExtLedgerState ccfg) decode (snapshotToStatePath s)
Expand Down
5 changes: 4 additions & 1 deletion ...ros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/LedgerDB/V2/InMemory.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,10 @@ loadSnapshot ::
SomeHasFS m ->
DiskSnapshot ->
ExceptT (SnapshotFailure blk) m (LedgerSeq' m blk, RealPoint blk)
loadSnapshot tracer _rr ccfg fs ds = do
loadSnapshot tracer _rr ccfg fs@(SomeHasFS hfs) ds = do
fileEx <- lift $ doesFileExist hfs (snapshotToDirPath ds)
Monad.when fileEx $ throwE $ InitFailureRead ReadSnapshotIsLegacy

snapshotMeta <-
withExceptT (InitFailureRead . ReadMetadataError (snapshotToMetadataPath ds)) $
loadSnapshotMetadata fs ds
Expand Down