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

Local testnet simulator implementation #4

Merged
merged 14 commits into from
Sep 11, 2023
Merged

Conversation

KuphJr
Copy link
Contributor

@KuphJr KuphJr commented Sep 7, 2023

TODO:

  • README entry
  • changeset

@KuphJr KuphJr requested a review from zeuslawyer September 7, 2023 23:33
zeuslawyer
zeuslawyer previously approved these changes Sep 8, 2023
@KuphJr KuphJr requested a review from zeuslawyer September 8, 2023 20:00
@KuphJr KuphJr marked this pull request as ready for review September 8, 2023 20:03
const simulations = [...Array(numberOfSimulatedNodeExecutions)].map(() =>
simulateScript({
source: requestData.source,
secrets: simulationConfig.secrets, // Secrets are taken from simulationConfig, not request data included in transaction
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems a bit odd that the secrets that accompany a request would be in the sim's config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this will be more clear shortly when I get this integrated into the HH starter kit. If it still doesn't make sense then, maybe we can revisit. I will have the starter kit PR up later today.


const req = await reqTx.wait(1)
const requestId = req.events[0].topics[1]
const response = await functionsListener.listenForResponse(requestId)
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it possible for the request response to be fulfilled before the listener gets mounted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't because it takes 1 block for the response transaction to be mined on the testnet, which is plenty of time to initialize the response listener. Also, we need to initiate the request transaction so we can get the requestId (however, maybe I could switch reqTx.wait(1) -> reqTx.wait(0) to further eliminate any chance of a race condition.

zeuslawyer
zeuslawyer previously approved these changes Sep 11, 2023
Copy link
Collaborator

@zeuslawyer zeuslawyer left a comment

Choose a reason for hiding this comment

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

LGTM. Few comments for you to consider.

@KuphJr KuphJr merged commit aa7299b into main Sep 11, 2023
4 checks passed
@github-actions github-actions bot mentioned this pull request Sep 11, 2023
ernest-nowacki pushed a commit that referenced this pull request Nov 2, 2023
* implemented SubscriptionManager class and createSubscription method

* regenerated package-lock.json

* added recompilation in separate command

* regenerated package-lock.json

* added env var during hardhat compilation

* ran prettier

* fixed corrupted package-lock.json
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