-
Notifications
You must be signed in to change notification settings - Fork 3
feat: ledger replay with 'fake' validator keypair #301
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
Conversation
* master: chore: improve/fix ledger tooling (#294)
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.
3 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
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.
Testing replay on a realistic ledger with a different keypair fails on the first transaction that requires a clone.
OK so we can improve on the idea, but there are other important changes in here which are needed to successfully simulate ledger replay locally, namely handling a huge amount of accounts. The only piece that is not desired at this point is the ledger keypair overwrite. However that is already a separate command, I just need to improve the docs on that. So once I did the docs part we should merge this anyways (I'll put a note about the limitations with the docs). |
about = "Prepares the ledger for testing", | ||
long_about = "(Over)writes the validator keypair" | ||
)] | ||
LedgerReplayTest { ledger_path: PathBuf }, |
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 would rename it to OverrideKeypair
(same for the command name), since is not preparing for tests
@@ -45,5 +53,8 @@ fn main() { | |||
config, | |||
) | |||
} | |||
Commands::LedgerReplayTest { ledger_path } => { |
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.
Ditto, I would rename to OverrideKeypair
I'm not against merging the PR, it adds a functionality to override the keypair. |
The idea of the name was that in the future this command would do any extra preparation of the ledger to make it usable in tests. But I can rename for now if you prefer |
Closing as it doesn't fix the issue. We will re-open with a proper fix for the replay |
Summary
Adding ability to prepare a ledger for replay in a diagnostics/test scenario.
Originally @GabrielePicco requested a way to basically ignore the keypair until after ledger
replay. However from looking at the way that the validator is initialized I realized this would
turn out to be a heavy handed solution to the problem.
Instead I decided to add a new command to the genx CLI that would allow for the keypair to be
overwritten right before we want to replay a specific legger. This is even more secure since
we're sure to wipe the original validator keypair from disk before starting the replay.
This also allows downloading the ledger without the keypair in the first place.
It is also safe to not cause any changes on chain (as a result of replay or txs after) since
the validator keypair is incorrect and cannot sign any on-chain transactions.
FIXES: #299
NOTE: I'm basing this off the previous PR (#300) since builds up on it. Once that is merged we
need to rebase this PR on master to merge.
Details
Here is an excerpt from the genx README explaining how to overwrite the validator keypair with a
random one when replaying the ledger to diagnose/test.
Greptile Summary
This PR adds a new
test-ledger
command to the genx CLI tool that enables ledger replay testing without requiring the original validator keypair, improving testing and diagnostic capabilities.test-ledger
command intools/genx/src/main.rs
that overwrites validator keypair with a random one for testingledger_replay_test.rs
module to handle keypair generation and file management for test scenarios