tests: superfluid delegator voting #1586

czarcas7ic · 2022-05-25T20:15:04Z

What is the purpose of the change

Changes genesis creation to be closer to production (uses osmo instead of stake as staking token as well as all other occurrences of using stake as default token)
Adds a test to ensure a delegator can override their share of the validator's voting power.

Testing and Verifying

This change is already covered by existing e2e test framework. Was also run locally on arm64 arch.

Documentation and Release Note

Does this pull request introduce a new feature or user-facing behavior changes? not
Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
How is the feature or change documented? not applicable

Closes: #XXX This PR refactors e2e test setup to have an abstraction `chainConfig` that has information about which validators should not be run during initialization. This is a first step toward testing state-sync as we want to postpone running some nodes so that we can test nodes "catching up". Additionally, this PR extracts a separate package `docker` for managing and storing all information related to docker images. - create a `chainConfig` struct to encapsulate all configurations related to chains in a single abstraction * remove global variables `propHeightA`, `propHeightB`, `votingPeriodA`, `votingPeriodB`. Instead, these are part of the `chainConfig` struct * `skipRunValidatorIndexes` - this is needed to skip certain validators from being run during setup so that we can test state-sync post-initialization and upgrade - remove code duplication for every chain. Instead, always loop over `chainConfig`s so that if we add more chains later, the setup would work out of the box - tested running both with and without upgrade - Does this pull request introduce a new feature or user-facing behavior changes? no - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? no - How is the feature or change documented? not applicable

codecov-commenter · 2022-05-25T20:20:59Z

Codecov Report

Merging #1586 (ff4935c) into main (0857e87) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1586   +/-   ##
=======================================
  Coverage   19.60%   19.60%           
=======================================
  Files         242      242           
  Lines       32279    32279           
=======================================
  Hits         6329     6329           
  Misses      24792    24792           
  Partials     1158     1158

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0857e87...ff4935c. Read the comment docs.

tests/e2e/e2e_util_test.go

tests/e2e/chain/chain.go

tests/e2e/chain/export.go

tests/e2e/e2e_setup_test.go

tests/e2e/chain/chain.go

tests/e2e/chain/config.go

tests/e2e/chain/export.go

tests/e2e/e2e_setup_test.go

tests/e2e/e2e_test.go

tests/e2e/e2e_util_test.go

tests/e2e/chain/export.go

ValarDragon · 2022-05-27T17:05:43Z

tests/e2e/chain/config.go

+	var stakeGenState staketypes.GenesisState
+	if err := util.Cdc.UnmarshalJSON(appGenState[staketypes.ModuleName], &stakeGenState); err != nil {
+		return err
+	}
+
+	stakeGenState.Params = staketypes.Params{
+		BondDenom:         OsmoDenom,
+		MaxValidators:     100,
+		MaxEntries:        7,
+		HistoricalEntries: 10000,
+		UnbondingTime:     240000000000,
+		MinCommissionRate: sdk.ZeroDec(),
+	}
+
+	sz, err := util.Cdc.MarshalJSON(&stakeGenState)
+	if err != nil {
+		return err
+	}
+	appGenState[staketypes.ModuleName] = sz


(For a future PR) I think we should move these to be independent functions, and think of ways to lower the boilerplate involved with them. (e.g. Go Generics)

tests/e2e/e2e_test.go

…osis into adam/sf-tests-2

czarcas7ic · 2022-05-27T21:18:45Z

This should be good to go now, unsure of why linter is failing though, looks like its having trouble pulling the linter docker image suddenly

p0mvn

Really great job on minimizing code duplication and extracting these reusable methods!

A few minor questions and comments from me

Dockerfile

tests/e2e/scripts/pool2A.json

p0mvn · 2022-05-27T21:45:35Z

tests/e2e/chain/export.go

@@ -13,6 +13,8 @@ type Validator struct {
 	Index         int    `json:"index"`
 	Mnemonic      string `json:"mnemonic"`
 	PublicAddress string `json:"publicAddress"`
+	PublicKey     string `json:"publicKey"`
+	OperAddress   string `json:"operAddress"`


This can be removed as it is not used

We still need to store this value for each validator so we are able to SFS to their operator address

I was hoping to keep these Validator and Chain structs only used for transferring state between the "chain-initialization" part (that happens inside a Docker container) and the test runner part (in e2e package). The goal of this is to decouple the chain initialization and test runner components as much as possible.

I know there is still some coupling present right now but the goal is to eventually get to completely independent components with incremental refactoring.

So this OperAddress is never used during chain initialization. I think if we want to keep adding more state to Validator or Chain structs, we should wrap them into structs that are local to the e2e package:

type validatorConfig struct { validator chain.Validator operatorAddress string }

We already do something similar with chain.Chain:

osmosis/tests/e2e/e2e_setup_test.go

Lines 40 to 52 in 360abcf

type chainConfig struct {

// voting period is number of blocks it takes to deposit, 1.2 seconds per validator to vote on the prop, and a buffer.

votingPeriod float32

// upgrade proposal height for chain.

propHeight int

// Indexes of the validators to skip from running during initialization.

// This is needed for testing functionality like state-sync where we would

// like to start a node during tests post-initialization.

skipRunValidatorIndexes map[int]struct{}

latestProposalNumber int

latestLockNumber int

chain *chain.Chain

}

Here, instead of adding the fields directly to chain.Chain we wrap it into the chainConfig struct

Please let me know what you think

Should we move this to an issue?

I can get to it now, if it proves to be a difficult change (it shouldn't be) then I will make an issue instead and merge

tests/e2e/e2e_setup_test.go

tests/localosmosis/Dockerfile

tests/e2e/e2e_util_test.go

tests/e2e/scripts/.bash_history

tests/e2e/e2e_util_test.go

Co-authored-by: Roman <[email protected]>

czarcas7ic · 2022-05-29T01:52:25Z

Changes made, let me know if we anything further done here. Once we merge this, CI will be ~10 minute for this until I merge in Roman's changes which should make non PRs to main much faster

ValarDragon

LGTM! We should chat about some more steps to help eliminate boilerplate / overhead further in a followup issue/PR

czarcas7ic and others added 9 commits May 23, 2022 09:38

change to osmo no stake

2c87381

currently working

f416f6f

superfluid vote overwrite test

6d7c5d4

change docker file

9f0ff0a

add extract oper address

12d4a57

extract val addr

cac90ee

new line

6904461

delete no longer needed script

ee095d0

github-actions bot added the Type:build label May 25, 2022

czarcas7ic and others added 2 commits May 25, 2022 15:22

Merge branch 'main' into adam/sf-tests-2

e4ec2fb

Delete .bash_history

6a84b6a

czarcas7ic marked this pull request as ready for review May 25, 2022 20:44

czarcas7ic requested a review from a team May 25, 2022 20:44

ValarDragon reviewed May 25, 2022

View reviewed changes

tests/e2e/e2e_util_test.go Outdated Show resolved Hide resolved

ValarDragon reviewed May 25, 2022

View reviewed changes

tests/e2e/e2e_util_test.go Outdated Show resolved Hide resolved

alexanderbez reviewed May 25, 2022

View reviewed changes

p0mvn reviewed May 25, 2022

View reviewed changes

tests/e2e/chain/export.go Outdated Show resolved Hide resolved

add helper functions to declutter

966d361

czarcas7ic mentioned this pull request May 27, 2022

[v10] E2E Test Expansion #1605

Closed

3 tasks

ValarDragon reviewed May 27, 2022

View reviewed changes

remove majority boilerplate

02bbe8c

p0mvn reviewed May 27, 2022

View reviewed changes

tests/e2e/e2e_test.go Outdated Show resolved Hide resolved

czarcas7ic and others added 5 commits May 27, 2022 12:50

move docker file back to distroless

9767416

Merge branch 'main' into adam/sf-tests-2

eb16e72

no longer use bash to add account

a4cabdd

Merge branch 'main' into adam/sf-tests-2

1e7ee23

remove debugging print statements

c3ea676

czarcas7ic and others added 3 commits May 27, 2022 15:44

Merge branch 'adam/sf-tests-2' of https://github.com/osmosis-labs/osm…

0a8d28d

…osis into adam/sf-tests-2

Delete .bash_history

6c0e355

remove extra line in docker file

4c4358f

github-actions bot removed the Type:build label May 27, 2022

czarcas7ic requested a review from a team May 27, 2022 21:17

p0mvn reviewed May 27, 2022

View reviewed changes

ValarDragon reviewed May 28, 2022

View reviewed changes

tests/e2e/scripts/.bash_history Outdated Show resolved Hide resolved

ValarDragon reviewed May 28, 2022

View reviewed changes

tests/e2e/e2e_util_test.go Outdated Show resolved Hide resolved

czarcas7ic and others added 2 commits May 28, 2022 19:47

Apply suggestions from code review

e209efb

Co-authored-by: Roman <[email protected]>

code review suggestions

360abcf

czarcas7ic requested a review from a team May 29, 2022 01:52

ValarDragon approved these changes May 29, 2022

View reviewed changes

alexanderbez approved these changes May 30, 2022

View reviewed changes

czarcas7ic and others added 2 commits May 30, 2022 19:49

further validator abstractions

b9b14ef

Merge branch 'main' into adam/sf-tests-2

ff4935c

czarcas7ic merged commit 2fdfeae into main May 30, 2022

czarcas7ic deleted the adam/sf-tests-2 branch May 30, 2022 20:41

czarcas7ic mentioned this pull request May 31, 2022

test: e2e test superfluid gov voting #1556

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: superfluid delegator voting #1586

tests: superfluid delegator voting #1586

czarcas7ic commented May 25, 2022

codecov-commenter commented May 25, 2022 •

edited

Loading

ValarDragon May 27, 2022

czarcas7ic commented May 27, 2022

p0mvn left a comment

p0mvn May 27, 2022

czarcas7ic May 29, 2022

p0mvn May 29, 2022 •

edited

Loading

ValarDragon May 30, 2022

czarcas7ic May 30, 2022

czarcas7ic commented May 29, 2022

ValarDragon left a comment

	type chainConfig struct {
	// voting period is number of blocks it takes to deposit, 1.2 seconds per validator to vote on the prop, and a buffer.
	votingPeriod float32
	// upgrade proposal height for chain.
	propHeight int
	// Indexes of the validators to skip from running during initialization.
	// This is needed for testing functionality like state-sync where we would
	// like to start a node during tests post-initialization.
	skipRunValidatorIndexes map[int]struct{}
	latestProposalNumber int
	latestLockNumber int
	chain *chain.Chain
	}

tests: superfluid delegator voting #1586

tests: superfluid delegator voting #1586

Conversation

czarcas7ic commented May 25, 2022

What is the purpose of the change

Testing and Verifying

Documentation and Release Note

codecov-commenter commented May 25, 2022 • edited Loading

Codecov Report

ValarDragon May 27, 2022

Choose a reason for hiding this comment

czarcas7ic commented May 27, 2022

p0mvn left a comment

Choose a reason for hiding this comment

p0mvn May 27, 2022

Choose a reason for hiding this comment

czarcas7ic May 29, 2022

Choose a reason for hiding this comment

p0mvn May 29, 2022 • edited Loading

Choose a reason for hiding this comment

ValarDragon May 30, 2022

Choose a reason for hiding this comment

czarcas7ic May 30, 2022

Choose a reason for hiding this comment

czarcas7ic commented May 29, 2022

ValarDragon left a comment

Choose a reason for hiding this comment

codecov-commenter commented May 25, 2022 •

edited

Loading

p0mvn May 29, 2022 •

edited

Loading