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 all 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
663 changes: 454 additions & 209 deletions tests/e2e/actions.go
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 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
7 changes: 7 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
79 changes: 62 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,30 @@
Title: title,
Description: description,
}
case "/interchain_security.ccv.provider.v1.MsgUpdateConsumer":
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
break
}
}
Comment on lines +472 to +477
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 chainConfigs map to find the matching consumerChainId does not guarantee order, which could lead to non-deterministic behavior. If the order of chains is important, consider refactoring to use a slice or other data structure that maintains order.

Tools
GitHub Check: CodeQL

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


updateProposal := ConsumerAdditionProposal{
Deposit: uint(deposit),
Chain: consumerChainId,
Status: status,
}
if rawContent.Get("initialization_parameters").Exists() {
spawnTime := rawContent.Get("initialization_parameters.spawn_time").Time().Sub(tr.containerConfig.Now)
updateProposal.SpawnTime = int(spawnTime.Milliseconds())
updateProposal.InitialHeight = clienttypes.Height{
RevisionNumber: rawContent.Get("initialization_parameters.initial_height.revision_number").Uint(),
RevisionHeight: rawContent.Get("initialization_parameters.initial_height.revision_height").Uint(),
}
}
return updateProposal
Comment on lines +469 to +492
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.

Suggestion: Refactor the code to reduce duplication and improve readability.

Consider extracting the common logic for finding the consumerChainId and chain into a separate function. This will reduce duplication and make the code more readable.

Also applies to: 526-539

Tools
GitHub Check: CodeQL

[warning] 472-477: 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 +523,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 +771,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
consumerId := tr.chainConfigs[ChainID(consumerChain)].ConsumerId
cmd := tr.target.ExecCommand(binaryName,

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

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

cmd := tr.target.ExecCommand(binaryName,

"query", "provider", "validator-provider-key",
string(consumerChain), tr.validatorConfigs[validator].ConsumerValconsAddressOnProvider,
string(consumerId), tr.validatorConfigs[validator].ConsumerValconsAddressOnProvider,
`--node`, tr.GetQueryNode(ChainID("provi")),
`-o`, `json`,
)
Expand Down Expand Up @@ -898,7 +931,12 @@
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
break
}
}
}

return chains
Expand Down Expand Up @@ -969,20 +1007,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 +1056,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