Skip to content
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.

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

[Feat] Introduce a test storage mode #12

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ljedrz
Copy link
Collaborator

@ljedrz ljedrz commented Jan 14, 2025

This PR proposes a new StorageMode::Test variant which may contain a TempDir (with feature = "rocks" in snarkOS/VM tests). It is intended to replace all current uses of impl From<Option<u16>> for StorageMode, which returns StorageMode::Production even in tests (which, without additional setup, would cause the dev's production database to be overwritten), which complicates their setup.

With these changes, we could simplify and unify the test storage setup in snarkOS, reducing the LoC related to storage by over 200.

@ljedrz
Copy link
Collaborator Author

ljedrz commented Jan 14, 2025

Note: semver-wise, the new aleo-std version should be at least 0.2, but I've followed the convention followed by this crate thus far. Happy to adjust it if we want to be compliant.

Copy link
Contributor

@vicsn vicsn left a comment

Choose a reason for hiding this comment

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

LGTM

@vicsn vicsn requested a review from d0cd February 12, 2025 16:14
@ljedrz ljedrz requested a review from vicsn February 13, 2025 15:06
pub enum StorageMode {
/// The production mode is used for running a node on the Aleo mainnet.
Production,
/// The development mode is used for running a node on a local network.
Development(u16),
/// The custom mode is used for running a node on custom configurations.
Custom(PathBuf),
/// Test-only ephemeral storage which self-destructs afterwards.
Test(Option<Arc<TempDir>>),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: why is it wrapped in an Option?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

while I don't recall this being used, we might want to reuse an instance of storage in some fine-grained tests, which this would allow

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.

2 participants