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

chore: add a way to start a local testnet #901

Closed

Conversation

p-offtermatt
Copy link
Contributor

@p-offtermatt p-offtermatt commented Apr 27, 2023

Description

Closes: #896

The PR adds a file to start a simple, local testnet
with 3 validators, a provider chain, and a consumer chain.

This serves to help new or existing developers to manually test changes
and get quick feedback, to play with a small chain, and so on.

The script is far from perfect, but it's working as-is.
This is not production code, and merely supposed to help with some manual tests, so I think it brings value in its current state.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • Included the correct type prefix in the PR title
  • Targeted the correct branch (see PR Targeting)
  • Provided a link to the relevant issue or specification
  • Updated the relevant documentation or specification
  • Reviewed "Files changed" and left comments if necessary
  • Confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage

@p-offtermatt p-offtermatt linked an issue Apr 27, 2023 that may be closed by this pull request
@p-offtermatt p-offtermatt marked this pull request as ready for review April 27, 2023 17:39
@mpoke mpoke requested a review from ancazamfir April 28, 2023 07:25
@mpoke mpoke changed the title feat: add a way to start a local testnet chore: add a way to start a local testnet Apr 28, 2023
Copy link
Contributor

@shaspitz shaspitz left a comment

Choose a reason for hiding this comment

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

In general I'm not opposed to these changes and they look solid! Few questions:

  • Is this script intended to be run in a docker container and/or should it be dockerized?
  • How is this setup different from the e2e test setup?
  • Is it possible to reuse existing .sh code from tests/e2e/testnet-scripts to get the testnet up and running in a container, and have a way for a user to bring up this testnet without running any e2e "steps" against the two chains? Then provide instructions for how the user can shell into said container and execute txs as they please

@p-offtermatt
Copy link
Contributor Author

Thanks for the comments Shawn! Doing my best to address them below.

In general I'm not opposed to these changes and they look solid! Few questions:

* Is this script intended to be run in a docker container and/or should it be dockerized?

No, it wasn't my intention. Ideally I would like something that can be run without Docker here, to minimize dependencies and not have the added complexity of Docker. I can also see an argument for using Docker specifically to keep dependencies inside the Docker image and away from the host, however.

* How is this setup different from the e2e test setup?

The e2e test setup achieves a similar goal. Differences are: 1) e2e runs completely in Docker. 2) e2e relies on Go code and the e2e framework, since e.g. starting chains is a step in the e2e framework. To reuse anything major from the e2e framework, we'd have to modify Go code and add a new "manual-test" setup in the e2e framework.

* Is it possible to reuse existing .sh code from `tests/e2e/testnet-scripts` to get the testnet up and running in a container, and have a way for a user to bring up this testnet without running any e2e "steps" against the two chains? Then provide instructions for how the user can shell into said container and execute txs as they please

A bit redundant from the answer to the last comment: Yes it's possible, but it would probably involve modifying the e2e Go code. Starting a chain and adding a consumer chain are all done as steps in the e2e framework, so from the outside one would need to not interact with the testnet-script, but with the e2e Go entrypoint. It seems like a plausible way as well, it might not be worth the effort vs having this single script (though I am not very certain either way here, having the e2e framework provide this functionality might be desirable and easier to maintain long-term).

@shaspitz
Copy link
Contributor

shaspitz commented May 4, 2023

@p-offtermatt thanks for the responses! Yea, the main reason why I'm leaning toward using the e2e entrypoint for starting a local testnet is that this script you have here repeats a lot of the functionality that already exists in the tests/e2e/testnet-scripts directory.

I'm sure there's some differences, but my bias is always to minimize the amount of code committed to the repo to achieve a goal. Happy to chat about this PR over a call tho!

@jtremback
Copy link
Contributor

jtremback commented May 9, 2023

I agree with Shawn here. Building your own shell script to start everything from scratch is an awesome exercise, and almost everyone on the team has done it at some point, but this is all stuff that the e2e tests are already doing. In fact, this e2e framework and many other e2e frameworks in Cosmos started with someone making a script like this. For fun, here is the first commit of the script that is a distant ancestor of both our e2e testnet framework, and Strangelove's IBC-test (although they ported to Go).

I actually already use the e2e tests as a janky way to mess around on the command line by inserting a state check that I know will fail on one of steps. Then I go into the Docker and mess around. Seems to me that this sort of accomplishes what is being done here (correct me if I'm wrong on that).

Some of the pain points with my method are:

  • Obviously it's sort of janky to get into the environment by making a test fail at a certain point. Maybe we can set up a test run that gets to a point that is convenient for manual testing and then hangs. If a test run completes, it tears down the Docker, so it needs to hang to prevent that.
  • It's not documented how to get into the e2e Docker, I just do it because I know how. It's not really an "official" technique. We could set up a documented command that uses stuff from the e2e framework to set up an environment that has a consumer and provider, and maybe the command could even shell into the docker.
  • It's tough to remember how to query and and send txs in the e2e environment because you have to remember the right home folder and node ip, and keychain stuff, etc etc. Maybe we could write a file to the root of the docker using the config of the test run that would have reference commands for querying and sending txs from each node on each chain.

Not saying you should start on that project or anything right now, just my thoughts.

@p-offtermatt
Copy link
Contributor Author

I agree with the points made here.
In particular, I take the point that adding this in the repo adds an extra maintenance burden that we might not want.
There are a couple of ways that I prefer a local script for to a Docker image, e.g. easily including extra tools that are installed locally, without having to reproduce their installation in the Dockerfile.
However, I do agree that we should reuse the existing code as much as possible. This script was written because we needed it to test a specific thing that was hard to test otherwise, so I thought it would be valuable to preserve, but It's probably not worth the maintenance burden.

In hindsight, I agree that the ICS repo should probably not commit to maintaining this script, so I'll close this PR.
If it's ever useful, the script exists as a Gist here https://gist.github.com/p-offtermatt/553aa20cef0ea4ab6bbd95c67857c2c3

Reusing the e2e setup to also allow manual testing could be discussed more in the related issue if necessary, it's likely a decent idea but seems out of the scope of this PR.

@p-offtermatt p-offtermatt deleted the ph/896-feature-add-a-way-to-add-a-local-testnet branch September 12, 2023 09:33
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.

feature: add a way to start a local testnet
3 participants