-
Notifications
You must be signed in to change notification settings - Fork 37
Implement retry/resume functionality for [create] & [setup] steps #399
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
base: main
Are you sure you want to change the base?
Conversation
- Add get_named_txs, get_setup_progress, and update_setup_progress methods to DbOps trait for resumable deployments and setup progress tracking - Implement these methods with SQLite queries in SqliteDb, including progress persistence - Create setup_progress table to record last completed setup step per scenario hash - Modify TestScenario to check existing named transactions and skip deployment if already done - Add logic to track and persist setup step progress, skipping completed steps on reruns - Introduce SetupStepFailed error for failed setup steps to halt execution and preserve progress - Enhance mock database implementation with stub methods for resumable functionality - Update rusqlite dependency to use bundled features - Add entries to .gitignore for Foundry binaries and macOS DS_Store files - Include unit tests verifying resumable methods and progress updates in SqliteDb implementation
zeroXbrock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a great first pass, thank you @anuragchvn-blip!
Just a few notes:
DB_VERSION needs to be incremented -- when users update their contender client, their DB will still be on the old version. Updating this value makes sure that updated clients with the old DB schema know to update their DB.
The simulation stage improperly skips setup steps. I noticed that when I tried to simulate a failure (by shutting down my node during setup) and retried, the simulation stage skipped all the setup steps (presumably because we ran them before in anvil). We should still be able to skip steps in the sim (if we've finished that step onchain), because the sim chain is a fork of the target chain, but first we need to properly identify the target chain for resumption (next point).
I noticed that when I spun up a new chain and ran the same setup again, the setup steps were skipped. I attached some comments to the code that suggest a solution to fix this.
Looking forward to your next commits, super excited about this feature!
| Ok(res) | ||
| } | ||
|
|
||
| fn get_setup_progress(&self, scenario_hash: &str) -> Result<Option<u64>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think scenario_hash should be a FixedBytes<32>, then we can convert it to a string for the DB here -- this trait shouldn't have to worry about improper hashes being given as input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scenario_hash should also be compounded with genesis_hash so we can identify the setup per-chain. Right now, setup steps are skipped for chains that have never setup the scenario.
My solution for this would be to add genesis_hash to the args (for get_setup_progress and update_setup_progress), then concat the hashes and re-hash to get the scenario_id we actually use as the DB index.
Something like this:
let scenario_id = keccak256([
scenario_hash.as_slice(),
genesis_hash.as_slice()
].concat());also do this in update_setup_progress
PR Description for Issue #71 Implementation
Title: Implement retry/resume functionality for [[create]] & [[setup]] steps
Description:
This PR implements the retry and resume functionality for both
[[create]]and[[setup]]steps as requested in issue #71.Changes Made:
Create Steps Resume:
named_txstable before deploying contractsSetup Steps Resume:
setup_progresstable withscenario_hashandlast_step_indexfieldsget_setup_progressandupdate_setup_progressdatabase methodsDatabase Schema:
setup_progresstable:(scenario_hash TEXT PRIMARY KEY, last_step_index INTEGER NOT NULL)Error Handling:
Technical Details:
named_txsto determine which deployments to skipTesting:
This resolves issue #71 by allowing users to resume failed scenario deployments without losing progress.