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.

Sign up for GitHub

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

Jump to bottom

test: fix e2e tests + PSS e2e tests use permissionless #2192

Merged
merged 9 commits into from
Sep 3, 2024
Merged

test: fix e2e tests + PSS e2e tests use permissionless #2192

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9119460
Fix e2e tests + PSS e2e tests use permissionless
bermuell Aug 29, 2024
23b0929
cleanup
bermuell Aug 30, 2024
904c087
addressed comments
bermuell Aug 30, 2024
72510bf
address more comments
bermuell Aug 30, 2024
0ea4988
cleanup
bermuell Sep 2, 2024
c3cb9a9
adapt PSS modify tests
bermuell Sep 2, 2024
bc8b6f0
address new comments
bermuell Sep 2, 2024
61c853e
removed ConsumerUpdateProposal
bermuell Sep 3, 2024
d10b610
addressed more review comments
bermuell Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
655 changes: 449 additions & 206 deletions tests/e2e/actions.go
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions tests/e2e/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ var hermesTemplates = map[string]string{
// type aliases for shared types from e2e package
type (
ChainID = e2e.ChainID
ConsumerID = e2e.ConsumerID
ValidatorID = e2e.ValidatorID
ValidatorConfig = e2e.ValidatorConfig
ChainConfig = e2e.ChainConfig
Expand Down Expand Up @@ -122,6 +123,7 @@ type TestConfig struct {
timeOffset time.Duration
transformGenesis bool
name string
Consumer2ChainID map[ConsumerID]ChainID // dynamic mapping of
bermuell marked this conversation as resolved.
Show resolved Hide resolved
}

// Initialize initializes the TestConfig instance by setting the runningChains field to an empty map.
Expand Down
8 changes: 8 additions & 0 deletions tests/e2e/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,13 @@ var stepChoices = map[string]StepChoice{
description: "test minting without inactive validators as a sanity check",
testConfig: MintTestCfg,
},
// TODO PERMISSIONLESS: ADD NEW E2E TEST
/* "permissionless-ics": {
name: "permissionless-ics",
steps: stepsPermissionlessICS(),
bermuell marked this conversation as resolved.
Show resolved Hide resolved
description: "test permissionless ics",
testConfig: DefaultTestCfg,
}, */
"inactive-vals-outside-max-validators": {
name: "inactive-vals-outside-max-validators",
steps: stepsInactiveValsTopNReproduce(),
Expand Down Expand Up @@ -596,6 +603,7 @@ func main() {
log.Fatalf("Error parsing command arguments %s\n", err)
}

//SetupLogger()
bermuell marked this conversation as resolved.
Show resolved Hide resolved
testCases := getTestCases(selectedTests, selectedTestfiles, providerVersions, consumerVersions)
testRunners := createTestRunners(testCases)
defer deleteTargets(testRunners)
Expand Down
74 changes: 57 additions & 17 deletions tests/e2e/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
e2e "github.com/cosmos/interchain-security/v5/tests/e2e/testlib"
"github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
"github.com/kylelemons/godebug/pretty"
"github.com/tidwall/gjson"
"gopkg.in/yaml.v2"
Expand All @@ -38,7 +39,7 @@

type Chain struct {
target e2e.TargetDriver
testConfig TestConfig
testConfig *TestConfig
bermuell marked this conversation as resolved.
Show resolved Hide resolved
}

func (tr Chain) GetChainState(chain ChainID, modelState ChainState) ChainState {
Expand Down Expand Up @@ -335,7 +336,7 @@
}

type Commands struct {
containerConfig ContainerConfig // FIXME only needed for 'Now' time tracking
containerConfig *ContainerConfig
validatorConfigs map[ValidatorID]ValidatorConfig
chainConfigs map[ChainID]ChainConfig
target e2e.PlatformDriver
Expand Down Expand Up @@ -465,6 +466,26 @@
Title: title,
Description: description,
}
case "/interchain_security.ccv.provider.v1.MsgUpdateConsumer":
spawnTime := rawContent.Get("initialization_parameters.spawn_time").Time().Sub(tr.containerConfig.Now)
consumerId := rawContent.Get("consumer_id").String()
consumerChainId := ChainID("")
for _, chainCfg := range tr.chainConfigs {
if chainCfg.ConsumerId == e2e.ConsumerID(consumerId) {
consumerChainId = chainCfg.ChainId
bermuell marked this conversation as resolved.
Show resolved Hide resolved
}
}
Dismissed Show dismissed Hide dismissed
return e2e.ConsumerAdditionProposal{
Deposit: uint(deposit),
Chain: consumerChainId,
Status: status,
SpawnTime: int(spawnTime.Milliseconds()),
InitialHeight: clienttypes.Height{
RevisionNumber: rawContent.Get("initialization_parameters.initial_height.revision_number").Uint(),
RevisionHeight: rawContent.Get("initialization_parameters.initial_height.revision_height").Uint(),
},
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Approved: Enhanced handling of consumer-related messages in GetProposal.

The method now correctly handles MsgUpdateConsumer and MsgRemoveConsumer, aligning with the enhanced consumer chain management. Consider adding unit tests to cover these new scenarios to ensure they are handled correctly.

Would you like me to help in writing these unit tests or should I open a GitHub issue to track this task?

Also applies to: 526-532

Tools
GitHub Check: CodeQL

[warning] 477-481: Iteration over map
Iteration over map may be a possible source of non-determinism

case "/interchain_security.ccv.provider.v1.MsgConsumerAddition":
chainId := rawContent.Get("chain_id").String()
spawnTime := rawContent.Get("spawn_time").Time().Sub(tr.containerConfig.Now)
Expand Down Expand Up @@ -498,13 +519,13 @@
Title: title,
Type: "/cosmos.upgrade.v1beta1.SoftwareUpgradeProposal",
}
case "/interchain_security.ccv.provider.v1.MsgConsumerRemoval":
chainId := rawContent.Get("chain_id").String()
case "/interchain_security.ccv.provider.v1.MsgRemoveConsumer":
consumerID := rawContent.Get("consumer_id").String()
stopTime := rawContent.Get("stop_time").Time().Sub(tr.containerConfig.Now)

var chain ChainID
for i, conf := range tr.chainConfigs {
if string(conf.ChainId) == chainId {
if string(conf.ConsumerId) == consumerID {
chain = i
break
}
Expand Down Expand Up @@ -746,19 +767,25 @@
arr := gjson.Get(string(bz), "chains").Array()
chains := make(map[ChainID]bool)
for _, c := range arr {
id := c.Get("chain_id").String()
chains[ChainID(id)] = true
phase := c.Get("phase").String()
if phase == types.ConsumerPhase_name[int32(types.ConsumerPhase_CONSUMER_PHASE_INITIALIZED)] ||
insumity marked this conversation as resolved.
Show resolved Hide resolved
phase == types.ConsumerPhase_name[int32(types.ConsumerPhase_CONSUMER_PHASE_REGISTERED)] ||
phase == types.ConsumerPhase_name[int32(types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED)] {
id := c.Get("chain_id").String()
chains[ChainID(id)] = true
}
}

return chains
}

func (tr Commands) GetConsumerAddress(consumerChain ChainID, validator ValidatorID) string {
binaryName := tr.chainConfigs[ChainID("provi")].BinaryName
consumer_id := tr.chainConfigs[ChainID(consumerChain)].ConsumerId
bermuell marked this conversation as resolved.
Show resolved Hide resolved
cmd := tr.target.ExecCommand(binaryName,

"query", "provider", "validator-consumer-key",
string(consumerChain), tr.validatorConfigs[validator].ValconsAddress,
string(consumer_id), tr.validatorConfigs[validator].ValconsAddress,
`--node`, tr.GetQueryNode(ChainID("provi")),
`-o`, `json`,
)
Expand All @@ -773,10 +800,12 @@

func (tr Commands) GetProviderAddressFromConsumer(consumerChain ChainID, validator ValidatorID) string {
binaryName := tr.chainConfigs[ChainID("provi")].BinaryName
consumer_id := tr.chainConfigs[ChainID(consumerChain)].ConsumerId

cmd := tr.target.ExecCommand(binaryName,

"query", "provider", "validator-provider-key",
string(consumerChain), tr.validatorConfigs[validator].ConsumerValconsAddressOnProvider,
string(consumer_id), tr.validatorConfigs[validator].ConsumerValconsAddressOnProvider,
`--node`, tr.GetQueryNode(ChainID("provi")),
`-o`, `json`,
)
Expand Down Expand Up @@ -898,7 +927,11 @@
arr := gjson.Get(string(bz), "consumer_chain_ids").Array()
chains := []ChainID{}
for _, c := range arr {
chains = append(chains, ChainID(c.String()))
for _, chain := range tr.chainConfigs {
if chain.ConsumerId == ConsumerID(c.String()) {
chains = append(chains, chain.ChainId)
bermuell marked this conversation as resolved.
Show resolved Hide resolved
}
}
Dismissed Show dismissed Hide dismissed
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential issue: Non-deterministic iteration over map.

The iteration over maps in GetHasToValidate does not guarantee order, which could lead to non-deterministic behavior. If the order of validation is important, consider refactoring to use a slice or other data structure that maintains order.

Tools
GitHub Check: CodeQL

[warning] 934-938: Iteration over map
Iteration over map may be a possible source of non-determinism

}

return chains
Expand Down Expand Up @@ -969,20 +1002,25 @@

func (tr Commands) GetProposedConsumerChains(chain ChainID) []string {
binaryName := tr.chainConfigs[chain].BinaryName
bz, err := tr.target.ExecCommand(binaryName,
"query", "provider", "list-proposed-consumer-chains",
cmd := tr.target.ExecCommand(binaryName,
"query", "provider", "list-consumer-chains",
`--node`, tr.GetQueryNode(chain),
`-o`, `json`,
).CombinedOutput()
)
bz, err := cmd.CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}

arr := gjson.Get(string(bz), "proposedChains").Array()
arr := gjson.Get(string(bz), "chains").Array()
chains := []string{}
for _, c := range arr {
cid := c.Get("chainID").String()
chains = append(chains, cid)
cid := c.Get("chain_id").String()
phase := c.Get("phase").String()
if phase == types.ConsumerPhase_name[int32(types.ConsumerPhase_CONSUMER_PHASE_INITIALIZED)] ||
phase == types.ConsumerPhase_name[int32(types.ConsumerPhase_CONSUMER_PHASE_REGISTERED)] {
chains = append(chains, cid)
}
}

return chains
Expand Down Expand Up @@ -1013,9 +1051,11 @@
// GetConsumerCommissionRate returns the commission rate of the given validator on the given consumerChain
func (tr Commands) GetConsumerCommissionRate(consumerChain ChainID, validator ValidatorID) float64 {
binaryName := tr.chainConfigs[ChainID("provi")].BinaryName
consumerID := tr.chainConfigs[consumerChain].ConsumerId

cmd := tr.target.ExecCommand(binaryName,
"query", "provider", "validator-consumer-commission-rate",
string(consumerChain), tr.validatorConfigs[validator].ValconsAddress,
string(consumerID), tr.validatorConfigs[validator].ValconsAddress,
`--node`, tr.GetQueryNode(ChainID("provi")),
`-o`, `json`,
)
Expand Down
Loading
Loading