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

Starship integration #56

Merged
merged 35 commits into from
Sep 21, 2023
Merged

Starship integration #56

merged 35 commits into from
Sep 21, 2023

Conversation

Anmol1696
Copy link
Contributor

@Anmol1696 Anmol1696 commented Jul 16, 2023

Overview

In this PR we add starship integration and tests.

Implementation

  • Add tests/starship dir
  • Move tests/e2e/testdata to tests/testdata, so that the same contracts and be used for e2e tests as well as starship tests
  • Run starship tests in the CI
  • Test case:
    • 2 way mesh (making both chains and consumer and producers)
    • Move assets around
    • Qeury contracts/states
  • Created tests/starship/setup pkg to initialize chains with the mesh contracts and perform e2e handshakes

CI

CI has been tested here: https://github.com/osmosis-labs/mesh-security-sdk/actions/workflows/starship-e2e-tests.yml

Why

With starship we are able to test against

  • actual chain binaries (currently still running against demo app, but when ready can be run against actual chain binaries)
  • actual relayers (using the hermes relayer, with config to pull)
  • ability to create a full mesh (for tests and also internal devnets)

Starship also gives a nice playground and a good start to be able to create an integrated qa environment.

Since Starship acts like a complete blackbox testing framework (we only interact via RPC endpoints), work needed to adapt tests/e2e tests where all functions and steps to initialize new chains with mesh-contracts gets codified and tested. We make little to no assumptions about the infra, so can easily create a util to initialize contracts.

Starship integration will also give chains that want to adopt mesh-security a way to quickly test out compatibility via and e2e testing setup.

Future work

  • Make tests/starship/setup pkg integrated with tests/e2e package
  • Add more scenarios with multiple chains, creating an actual mesh
  • Cloud k8s integration for running a bigger and longer devnet
  • Add more tests scenarios

Limitations

  • Contract setup takes long time: around 8 mins (this can be sped up)
  • Transient errors: Since there are alot of moving parts, there are still some cases of transient errors (we are working on eleminating them completely, one at a time). System will get more robust with time

tests/starship/setup/contract.go Dismissed Show dismissed Hide dismissed
tests/starship/setup/setup.go Dismissed Show dismissed Hide dismissed
tests/starship/setup/main.go Dismissed Show dismissed Hide dismissed
Copy link
Contributor

@alpe alpe left a comment

Choose a reason for hiding this comment

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

Very nice start! Thanks for driving this forward!
I have not looked at the test details. yet but I was wondering how this PR relates to #55 and what the bigger picture is?
With starship tests in tests/ folder, I would expect them to run in CI. Is this what you want to do?

x/meshsecurity/types/params.go Outdated Show resolved Hide resolved
demo/Dockerfile Outdated Show resolved Hide resolved
demo/Makefile Outdated Show resolved Hide resolved
demo/Makefile Outdated Show resolved Hide resolved
demo/Makefile Outdated Show resolved Hide resolved
tests/starship/DEVNET.md Outdated Show resolved Hide resolved
tests/starship/README.md Outdated Show resolved Hide resolved
@Anmol1696
Copy link
Contributor Author

@alpe sorry you had to review this, but this is still a draft PR and is not fully ready to be merged.
I will take your points into account when I open this PR up properly.

@Anmol1696
Copy link
Contributor Author

As for what the bigger picture is with Starship, we can make an issue or a design doc of sorts needed.

But yes we will run these tests in the CI as well, as well as for setup for anyone who wants to use Starship, for let's say frontend, backend or even contract development. I think we have to figure out where best to place the starship dir, but it is self contained and can be placed anywhere.

@Anmol1696 Anmol1696 changed the title starship 2 way contracts WIP: starship 2 way contracts Jul 20, 2023
@Anmol1696 Anmol1696 changed the title WIP: starship 2 way contracts WIP: starship integration Sep 1, 2023
@Anmol1696 Anmol1696 changed the title WIP: starship integration Starship integration Sep 1, 2023
@Anmol1696 Anmol1696 requested a review from alpe September 1, 2023 13:08
@Anmol1696 Anmol1696 marked this pull request as ready for review September 1, 2023 13:08
push:
branches:
- main
pull_request:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No sure if we want to run this on every PR, takes around 15mins to run the CI

@Anmol1696 Anmol1696 requested a review from a team September 2, 2023 11:37
Copy link

@JakeHartnell JakeHartnell left a comment

Choose a reason for hiding this comment

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

Thank you for this work, the demo we showed at Osmocon would not be possible without it. 🙏

@alpe
Copy link
Contributor

alpe commented Sep 21, 2023

This has been pending for some time due to my other responsibilities. Sorry for this!
I think it is good to get this rolling and see how this adds value. I have not looked deeper into the code as it is not touching the MS code. But I assume @JakeHartnell did, so let's merge and let CI do some work. ;-)

@alpe alpe merged commit ce6a286 into main Sep 21, 2023
5 checks passed
@alpe alpe deleted the anmol/starship-2-way-contracts branch September 21, 2023 10:28
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.

3 participants