-
Notifications
You must be signed in to change notification settings - Fork 27
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
[@kadena-clli] Add devnet simulate command (Feat) #1469
Conversation
…ena-cli/devnet-simulation
🦋 Changeset detectedLatest commit: b9da194 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 5 Ignored Deployments
|
…moved sender00 references
…ena-cli/devnet-simulation
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.
lgtm except the minor change request
prompt: simulationPrompts.transferIntervalPrompt, | ||
validation: z.number(), | ||
option: new Option( | ||
'-ti, --interval <interval>', |
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.
simulationNumberOfAccounts: createOption({
key: 'accounts' as const,
prompt: simulationPrompts.numberOfAccountsPrompt,
validation: z.number(),
option: new Option(
'-na, --accounts <accounts>',
'Amount of accounts to be created in the simulation.',
),
}),
please uphold this format:
simulationNumberOfAccounts: createOption({
key: 'simulationNumberOfAccounts' as const,
prompt: simulationPrompts.numberOfAccountsPrompt,
validation: z.number(),
option: new Option(
'-na, --simulation-number-of-accounts <simulationNumberOfAccounts>',
'Amount of accounts to be created in the simulation.',
),
}),
For this and all prompts below
packages/tools/kadena-cli/src/devnet/utils/simulation/coin/simulate.ts
Outdated
Show resolved
Hide resolved
let seededRandomNo = seedRandom(seed); | ||
let counter: number = 0; | ||
|
||
while (true) { |
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.
There is a lot of complexity within this while loop, making it hard for me to see how exactly it will break the loop, or if it is possible that it will run forever. This code would really benefit from moving some logic to separate methods
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.
Everything inside the loop is a little bit hard to refactor since I would have to pass a bunch of common variables inside each method and return objects with multiple properties due to the necessary logging in addition to some conditions that validate if the iteration is valid or if it should be skipped.
I can add some more comments to add clarity to what is happening and check if I can refactor it into smaller methods.
Answering your question: the loop could go for infinity. This loop's goal is the simulate random traffic in the devnet (with the possibility of replicating it via the seed number). The only way to break the loop automatically is with a maximum time flag which is being developed on another PR, otherwise you'd have to shut it yourself.
prompt: simulationPrompts.numberOfAccountsPrompt, | ||
validation: z.number(), | ||
option: new Option( | ||
'-na, --simulation-number-of-accounts <simulationNumberOfAccounts>', |
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.
@alber70g there was some confusion about this
but this should be
-n or only --simulation-number-of-accounts and removal of the -na
(same for all below if so)
correct | lets get this confusion out :)
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.
So since this is a specifically for devnet simulate, should we move it to the simulate
directory?
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.
Moved to simulation specific directory; updated the flags to single characters
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.
Approved but please let Albert confirm my remark. ;)
In the PR, we find the simulate feature integrated in
kadena-cli
under thedevnet
command.The simulate feature is responsible for simulating traffic in the devnet. Alongside the simulation, some auxiliary files implement the different transfer types and utils functions.
The simulate feature also produces a log file.
Note: some additional files are included in the PR due to lint and style fixes