Skip to content

Canonical snapshots #1664

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

Jump to bottom
Open
wants to merge 1 commit into
base: js/fix-double-read-lock-forward-port
Choose a base branch
from
Open

Canonical snapshots #1664

wants to merge 1 commit into from
+558 −459

Conversation

@jasagredo
Copy link
Contributor

@jasagredo jasagredo commented Sep 4, 2025 •
edited
Loading

Closes #1574

This was referenced Sep 4, 2025
Merged
Merged
Merged
Merged
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from e8490ea to c70a787 Compare September 4, 2025 13:29
@jasagredo
Copy link
Contributor Author

jasagredo commented Sep 4, 2025 •
edited
Loading

I plan on merging this as a PR stack:

@jasagredo jasagredo changed the title Non-native snapshots Canonical snapshots Sep 4, 2025
@jasagredo jasagredo changed the base branch from main to js/extract-2 September 4, 2025 16:44
@jasagredo jasagredo force-pushed the js/extract-2 branch 2 times, most recently from 7ccb173 to 8f5216b Compare September 5, 2025 09:57
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from c70a787 to 996abce Compare September 5, 2025 10:01
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from 996abce to c9d27ba Compare September 16, 2025 13:37
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from c9d27ba to aefce0a Compare September 17, 2025 07:44
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from aefce0a to 7fbfc44 Compare September 17, 2025 13:31
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from 7fbfc44 to 7d1bed6 Compare September 18, 2025 08:31
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from 7d1bed6 to 575d1fd Compare September 18, 2025 12:32
@jasagredo jasagredo self-assigned this Sep 19, 2025
@jasagredo jasagredo moved this to 🏗 In progress in Consensus Team Backlog Sep 19, 2025
@jasagredo jasagredo mentioned this pull request Sep 22, 2025
Open
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from 575d1fd to 900b804 Compare September 25, 2025 09:49
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from 900b804 to 0a5ade1 Compare September 30, 2025 14:01
@jasagredo jasagredo changed the base branch from js/extract-2 to js/snap-docs October 1, 2025 11:02
@jasagredo jasagredo force-pushed the js/snap-docs branch 3 times, most recently from 6221551 to 1931d2e Compare October 13, 2025 08:07
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch 2 times, most recently from 31b79ee to 2616a5a Compare October 13, 2025 08:40
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from 2616a5a to 8571d76 Compare October 14, 2025 08:26
Base automatically changed from js/snap-docs to main October 14, 2025 10:24
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from 8571d76 to b937b78 Compare October 14, 2025 15:02
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch 3 times, most recently from ee0160d to 104f18b Compare October 29, 2025 12:10
@jasagredo jasagredo changed the base branch from main to js/expose-for-node October 29, 2025 12:11
@jasagredo jasagredo marked this pull request as ready for review October 29, 2025 12:11
@jasagredo jasagredo moved this from 🏗 In progress to 👀 In review in Consensus Team Backlog Oct 29, 2025
@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from 104f18b to 28dd9ce Compare October 29, 2025 12:23
dnadales
dnadales reviewed Oct 29, 2025
View reviewed changes
...-consensus-cardano/src/snapshot-conversion/Ouroboros/Consensus/Cardano/SnapshotConversion.hs
-- ^ The backend used for the output snapshot, to write it in the metadata
}

data SomeBackend c where
Copy link
Member

@dnadales dnadales Oct 29, 2025

Choose a reason for hiding this comment

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

I wonder if c is the best name. We also used to use c for the Crypto parameter.

...-consensus-cardano/src/snapshot-conversion/Ouroboros/Consensus/Cardano/SnapshotConversion.hs
, outDeleteExtra :: Maybe FilePath
-- ^ In case some other directory needs to be wiped out
, outProgressMsg :: String
-- ^ A progress message (just for displaying)
Copy link
Member

@dnadales dnadales Oct 29, 2025

Choose a reason for hiding this comment

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

This might require further elaboration: how would this be displayed and for which purpose.

...-consensus-cardano/src/snapshot-conversion/Ouroboros/Consensus/Cardano/SnapshotConversion.hs
IO (SomeBackend SinkArgs)
-- ^ Sink arguments for consuming a stream of TxOuts
, outDeleteExtra :: Maybe FilePath
-- ^ In case some other directory needs to be wiped out
Copy link
Member

@dnadales dnadales Oct 29, 2025

Choose a reason for hiding this comment

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

This sound scary :) In which situations will this option be used?

@jasagredo jasagredo force-pushed the js/non-native-snapshots branch from 28dd9ce to d8ed028 Compare October 31, 2025 10:59
@jasagredo jasagredo changed the base branch from js/expose-for-node to js/fix-double-read-lock-forward-port October 31, 2025 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dnadales dnadales dnadales approved these changes

@nfrisby nfrisby Awaiting requested review from nfrisby nfrisby is a code owner

@amesgen amesgen Awaiting requested review from amesgen amesgen is a code owner

@fraser-iohk fraser-iohk Awaiting requested review from fraser-iohk fraser-iohk is a code owner

@geo2a geo2a Awaiting requested review from geo2a geo2a is a code owner

@bladyjoker bladyjoker Awaiting requested review from bladyjoker bladyjoker is a code owner

Assignees

@jasagredo jasagredo

Labels

non-native-snapshots

Projects

Consensus Team Backlog
Status: 👀 In review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow to create snapshot in non-native format

3 participants

@jasagredo @dnadales