feat: simd runs in-process testnet by default #9246

clevinson · 2021-04-30T21:53:20Z

Description

After some more recent conversations w/ @aaronc, I decided to go back to his original proposal of setting up a subcommand for running in-process testnets.

This PR splits the simd testnet command into two subcommands:

simd testnet start which starts an in-process n-node testnet
simd testnet init-files which sets up configuration & genesis files for an n-node testnet to be run as separate processes (one per node, most likely via Docker Compose)

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards. - n/a
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/) - see docs: simd testnet commands #9411
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

clevinson · 2021-04-30T21:53:32Z

I'd like some preliminary feedback on this draft PR, to get validation that others are aligned with this approach, and then i'd proceed with the following updates to clean this up:

Allow node0's ports to be set explicitly (defaulting to 26657, 1317, 9090, etc. etc.) as opposed to random free ports
add better documentation of new interfaces, functions, cli methods
changelog

Also, looking for feedback on naming :)

In a follow-up PR, we should figure out how to expose keys to users for manual testing if we run these testnets in CI (with Heroku Review Apps or similar). Making a full on faucet seems out of scope for this work. Two alternatives I can think of (neither are secure at all, but maybe that's not a concern):

storing persistent validator keys in github and explicitly marking them as insecure keys used for validators in CI
- this would require us to pass in a --keyring-dir flag into simd testnet command
outputing the node0 validator's mnemonic to stdout as part of the simd testnets command, so we could grab this mnemonic for testing by looking at github action / heroku logs ?

simapp/simd/cmd/testnet.go

…stnet setup

…nson/ci-devnets

testutil/network/network.go

amaury1093

Concept ACK.

I feel like this is the software developer's answer ("just code it") to a devops problem. A better solution might be some cloud orchestration config and deployment. But maybe this PR is fine for now, since the maintainers of the SDK are mainly software devs, and this setup is something we know how to debug easily 👍

Makefile

testutil/network/network.go

aaronc · 2021-05-26T19:40:49Z

This is just CLI breaking for tests, thus doesn't merit a ! in the title

aaronc · 2021-05-26T19:42:59Z

Also I think it's feat and not refactor as it changes existing behavior for end users, right?

ryanchristo · 2021-05-26T20:44:55Z

CHANGELOG.md

@@ -112,6 +113,7 @@ if input key is empty, or input data contains empty key.
  * The `RegisterCustomTypeURL` function and the `cosmos.base.v1beta1.ServiceMsg` interface have been removed from the interface registry.
 * (codec) [\#9251](https://github.com/cosmos/cosmos-sdk/pull/9251) Rename `clientCtx.JSONMarshaler` to `clientCtx.JSONCodec` as per #9226.
 * (x/bank) [\#9271](https://github.com/cosmos/cosmos-sdk/pull/9271) SendEnabledCoin(s) renamed to IsSendEnabledCoin(s) to better reflect its functionality.
+* [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) The `New` method for the network package now returns an error.


cc @aaronc should this be an API breaking change?

technically yes

ryanchristo · 2021-05-26T21:58:29Z

Maybe we could improve the UX a bit down the road. The CLI shows at the end started test network; press the Enter Key to terminate which is a bit confusing unless you know what you're doing. Maybe a link to some docs would be good at some point.

I agree. Maybe simapp readme could be expanded upon or we could add another document to Running a Node. It looks like the testnet command is not documented anywhere at the moment.

#9014 might be relevant here. I've assigned myself to the issue. I'll work on adding some documentation.

blushi

Tested ACK.

amaury1093

tACK, thanks @ryanchristo

my test:

./build/simd testnet start --v 10
./build/simd q staking validators // make sure there's 10 validators

amaury1093 · 2021-05-28T11:34:02Z

This is just CLI breaking for tests, thus doesn't merit a ! in the title

I believe we actually want the ! in the title, right? because of #9246 (comment). anyways, @ryanchristo i'll let you decide on the title and put automerge on at will

amaury1093

Oh wait, no, I forgot about #9246 (comment). let's wait

amaury1093 · 2021-06-28T08:48:17Z

@clevinson or @ryanchristo can one of you fix the changelog conflicts?

amaury1093

tACK: ./build/simd testnet start

## Description  This pull request is a follow-up pull request for #9246 that adds documentation for the `simd testnet` command. --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [x] Review `Codecov Report` in the comment section below once CI passes

@aaronc

## Description ref: cosmos#9183 After some more recent conversations w/ @aaronc, I decided to go back to his original proposal of setting up a subcommand for running in-process testnets. This PR splits the `simd testnet` command into two subcommands: - `simd testnet start` which starts an in-process n-node testnet - `simd testnet init-files` which sets up configuration & genesis files for an n-node testnet to be run as separate processes (one per node, most likely via Docker Compose) --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - **n/a** - [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - **see cosmos#9411** - [x] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [x] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [ ] Review `Codecov Report` in the comment section below once CI passes

update testnet subcommand to run in-process testnets by default

e1fd374

clevinson requested review from aaronc, amaury1093 and robert-zaremba April 30, 2021 21:53

clevinson commented Apr 30, 2021

View reviewed changes

simapp/simd/cmd/testnet.go Outdated Show resolved Hide resolved

clevinson added 4 commits April 30, 2021 14:55

Update simapp/simd/cmd/testnet.go

15803e3

update flag names, update make localnet-start task with new simd te…

21a0961

…stnet setup

Merge remote-tracking branch 'origin/clevinson/ci-devnets' into clevi…

80704f2

…nson/ci-devnets

Merge remote-tracking branch 'origin/master' into clevinson/ci-devnets

082d317

github-actions bot added the Type: Build label Apr 30, 2021

fix tests

837e58d

github-actions bot added C:CLI C:x/auth C:x/authz C:x/bank C:x/crisis C:x/distribution distribution module related C:x/evidence C:x/feegrant C:x/genutil genutil module issues C:x/gov C:x/mint C:x/params C:x/slashing C:x/staking labels Apr 30, 2021

clevinson commented Apr 30, 2021

View reviewed changes

testutil/network/network.go Outdated Show resolved Hide resolved

clevinson commented Apr 30, 2021

View reviewed changes

testutil/network/network.go Outdated Show resolved Hide resolved

make lint-fix

12c7283

amaury1093 reviewed May 3, 2021

View reviewed changes

Makefile Show resolved Hide resolved

testutil/network/network.go Outdated Show resolved Hide resolved

testutil/network/network.go Outdated Show resolved Hide resolved

testutil/network/network.go Outdated Show resolved Hide resolved

ryanchristo changed the title ~~refactor!: simd runs in-process testnet by default~~ feat: simd runs in-process testnet by default May 26, 2021

ryanchristo and others added 4 commits May 26, 2021 13:32

use private functions

65c6410

consolidate input params

2725ac7

Merge branch 'master' into clevinson/ci-devnets

aee829f

fix struct name

0b9aaf1

ryanchristo reviewed May 26, 2021

View reviewed changes

ryanchristo mentioned this pull request May 28, 2021

docs: simd testnet commands #9411

Merged

9 tasks

blushi approved these changes May 28, 2021

View reviewed changes

amaury1093 approved these changes May 28, 2021

View reviewed changes

amaury1093 suggested changes May 28, 2021

View reviewed changes

clevinson added the S: DO NOT MERGE Status: DO NOT MERGE label Jun 8, 2021

amaury1093 removed the S: DO NOT MERGE Status: DO NOT MERGE label Jun 28, 2021

ryanchristo and others added 4 commits June 28, 2021 11:47

Merge branch 'master' into clevinson/ci-devnets

f2210bd

amend changelog

e479ff1

fix new testsuite

b86cdcf

fix test command

4717b85

amaury1093 approved these changes Jun 29, 2021

View reviewed changes

amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Jun 29, 2021

amaury1093 added 2 commits June 29, 2021 12:17

Merge branch 'master' into clevinson/ci-devnets

61ac80b

Merge branch 'master' into clevinson/ci-devnets

6892453

mergify bot merged commit aef416f into master Jun 29, 2021

mergify bot deleted the clevinson/ci-devnets branch June 29, 2021 10:41

aleem1314 mentioned this pull request Nov 1, 2021

feat!: add testnet cmd regen-network/regen-ledger#607

Merged

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: simd runs in-process testnet by default #9246

feat: simd runs in-process testnet by default #9246

clevinson commented Apr 30, 2021 •

edited by ryanchristo

Loading

clevinson commented Apr 30, 2021 •

edited

Loading

amaury1093 left a comment

aaronc commented May 26, 2021

aaronc commented May 26, 2021

ryanchristo May 26, 2021

aaronc May 26, 2021

ryanchristo commented May 26, 2021 •

edited

Loading

blushi left a comment

amaury1093 left a comment

amaury1093 commented May 28, 2021

amaury1093 left a comment

amaury1093 commented Jun 28, 2021

amaury1093 left a comment

feat: simd runs in-process testnet by default #9246

feat: simd runs in-process testnet by default #9246

Conversation

clevinson commented Apr 30, 2021 • edited by ryanchristo Loading

Description

clevinson commented Apr 30, 2021 • edited Loading

amaury1093 left a comment

Choose a reason for hiding this comment

aaronc commented May 26, 2021

aaronc commented May 26, 2021

ryanchristo May 26, 2021

Choose a reason for hiding this comment

aaronc May 26, 2021

Choose a reason for hiding this comment

ryanchristo commented May 26, 2021 • edited Loading

blushi left a comment

Choose a reason for hiding this comment

amaury1093 left a comment

Choose a reason for hiding this comment

amaury1093 commented May 28, 2021

amaury1093 left a comment

Choose a reason for hiding this comment

amaury1093 commented Jun 28, 2021

amaury1093 left a comment

Choose a reason for hiding this comment

clevinson commented Apr 30, 2021 •

edited by ryanchristo

Loading

clevinson commented Apr 30, 2021 •

edited

Loading

ryanchristo commented May 26, 2021 •

edited

Loading