test: fix e2e tests + PSS e2e tests use permissionless #2192
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
bermuell
force-pushed
the
bernd/permissionless_ics
branch
2 times, most recently
from
e1a7681
to
603fdbd
Compare
bermuell
force-pushed
the
bernd/permissionless_ics
branch
from
f761c84
to
c6671fc
Compare
WalkthroughWalkthroughThe changes introduce enhancements to consumer chain management within an interchain security context. New action types and methods for creating, updating, and managing consumer chains are added, along with improvements to logging, configuration handling, and governance proposal management. The overall structure of the codebase is refined to enhance clarity, maintainability, and functionality related to consumer chains and their interactions. Changes
Tip We have updated our review workflow to use the Anthropic's Claude family of models. Please share any feedback in the discussion post on our Discord. Recent review detailsConfiguration used: .coderabbit.yml Files selected for processing (1)
Additional context usedPath-based instructions (1)
Gitleaks
golangci-lint
GitHub Check: CodeQL
Additional comments not posted (7)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (8)
tests/e2e/state.go (1)
477-481: Address static analysis warnings about map iteration.The static analysis tool has flagged potential issues with non-determinism due to map iteration. This is a common issue in Go when the order of iteration over maps, which is random, can lead to non-deterministic behavior.
Consider whether the order of elements is significant in these iterations. If the order is important, you might need to sort the keys before iterating or use a slice if applicable.
// Example of sorting map keys before iteration keys := make([]string, 0, len(map)) for k := range map { keys = append(keys, k) } sort.Strings(keys) for _, k := range keys { // process map[k] }Also applies to: 934-938
Tools
GitHub Check: CodeQL
[warning] 477-481: Iteration over map
Iteration over map may be a possible source of non-determinismtests/e2e/steps_partial_set_security.go (5)
36-36: Consider handling known errors more robustly.The
SetConsumerCommissionRateActionexpects an error with the message "unknown consumer chain". It's good practice to handle known errors more explicitly in test scenarios to ensure they are not just logged but acted upon, perhaps by retrying or correcting the test setup.
45-55: Review the use of future spawn times in test setups.The
CreateConsumerChainActionsets a spawn time far in the future (10 minutes). While this may be intended to test delayed chain starts, it's important to ensure that this setup aligns with the test's objectives. If the delay is not part of the test requirements, consider adjusting the spawn time to a more immediate setting to reduce test execution time.
1471-1481: Check future spawn times in the context of allowlisted validators.The
CreateConsumerChainActionsets a future spawn time in a scenario involving allowlisted validators. This setup should be carefully reviewed to ensure it matches the intended test behavior, especially regarding how allowlisting interacts with delayed chain starts.
1654-1665: Evaluate the setup of future spawn times with validator set caps.The
CreateConsumerChainActionuses a future spawn time in conjunction with a validator set cap. This combination should be scrutinized to ensure it serves the test's purpose, particularly how the cap affects chain initialization when delayed.
1835-1845: Review the configuration of future spawn times in modification scenarios.The
CreateConsumerChainActionsets a spawn time far in the future within a modification scenario. This setting should be evaluated to ensure it aligns with the testing goals, particularly in terms of how modifications are applied during the delay.tests/e2e/actions.go (2)
Line range hint
552-656: Potential improvements in submitConsumerAdditionProposal method.The method is crucial for submitting governance proposals related to consumer chains. It appears to handle the creation and submission of proposals adequately. However, consider adding more detailed comments explaining each step for better maintainability and readability.
Add detailed comments explaining the logic behind each major step in the
submitConsumerAdditionProposalmethod.Tools
Gitleaks
582-582: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
GitHub Check: CodeQL
[failure] 462-462: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
[failure] 621-621: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
Line range hint
889-1062: Potential security issue with unsafe quoting in JSON operations.Similar to previous methods, this method uses potentially unsafe quoting which could lead to security vulnerabilities if the JSON content contains single quotes. Consider using parameterized or properly escaped queries to mitigate injection risks.
Apply this diff to fix the unsafe quoting issue:
- "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, proposalFile), + "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, json.EscapeString(jsonStr), proposalFile),Tools
Gitleaks
894-894: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| // GenerateGovProposalContent creates proposal content ready to be used by `gov submit-proposal` command | ||
| func GenerateGovProposalContent(title, summary, metadata, deposit, description string, expedited bool, msgs ...sdk.Msg) string { | ||
|
|
||
| // Register the messages. Needed for correct type annotation in the resulting json | ||
| modcodec := codec.NewProtoCodec(codectypes.NewInterfaceRegistry()) | ||
| modcodec.InterfaceRegistry().RegisterImplementations( | ||
| (*sdk.Msg)(nil), | ||
| msgs..., | ||
| ) | ||
|
|
||
| proposal := GovernanceProposal{ | ||
| Metadata: metadata, | ||
| Deposit: deposit, | ||
| Title: title, | ||
| Summary: summary, | ||
| Expedited: expedited, | ||
| } | ||
|
|
||
| for _, msg := range msgs { | ||
| msgJson, err := modcodec.MarshalInterfaceJSON(msg) | ||
| if err != nil { | ||
| panic(fmt.Errorf("failed marshalling message '%v' for gov proposal: err=%s", msg, err)) | ||
| } | ||
| proposal.Messages = append(proposal.Messages, msgJson) | ||
| } | ||
| raw, err := json.MarshalIndent(proposal, "", " ") | ||
| if err != nil { | ||
| panic(fmt.Errorf("failed to marshal proposal: %w", err)) | ||
| } | ||
|
|
||
| return string(raw) | ||
| } |
There was a problem hiding this comment.
Improve Error Handling in GenerateGovProposalContent
The function uses panic for error handling, which might not be ideal for a utility function as it can terminate the program unexpectedly. Consider returning an error instead, allowing the caller to decide how to handle it.
Consider refactoring the error handling:
- panic(fmt.Errorf("failed marshalling message '%v' for gov proposal: err=%s", msg, err))
+ return "", fmt.Errorf("failed marshalling message '%v' for gov proposal: err=%s", msg, err)
- panic(fmt.Errorf("failed to marshal proposal: %w", err))
+ return "", fmt.Errorf("failed to marshal proposal: %w", err)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // GenerateGovProposalContent creates proposal content ready to be used by `gov submit-proposal` command | |
| func GenerateGovProposalContent(title, summary, metadata, deposit, description string, expedited bool, msgs ...sdk.Msg) string { | |
| // Register the messages. Needed for correct type annotation in the resulting json | |
| modcodec := codec.NewProtoCodec(codectypes.NewInterfaceRegistry()) | |
| modcodec.InterfaceRegistry().RegisterImplementations( | |
| (*sdk.Msg)(nil), | |
| msgs..., | |
| ) | |
| proposal := GovernanceProposal{ | |
| Metadata: metadata, | |
| Deposit: deposit, | |
| Title: title, | |
| Summary: summary, | |
| Expedited: expedited, | |
| } | |
| for _, msg := range msgs { | |
| msgJson, err := modcodec.MarshalInterfaceJSON(msg) | |
| if err != nil { | |
| panic(fmt.Errorf("failed marshalling message '%v' for gov proposal: err=%s", msg, err)) | |
| } | |
| proposal.Messages = append(proposal.Messages, msgJson) | |
| } | |
| raw, err := json.MarshalIndent(proposal, "", " ") | |
| if err != nil { | |
| panic(fmt.Errorf("failed to marshal proposal: %w", err)) | |
| } | |
| return string(raw) | |
| } | |
| // GenerateGovProposalContent creates proposal content ready to be used by `gov submit-proposal` command | |
| func GenerateGovProposalContent(title, summary, metadata, deposit, description string, expedited bool, msgs ...sdk.Msg) (string, error) { | |
| // Register the messages. Needed for correct type annotation in the resulting json | |
| modcodec := codec.NewProtoCodec(codectypes.NewInterfaceRegistry()) | |
| modcodec.InterfaceRegistry().RegisterImplementations( | |
| (*sdk.Msg)(nil), | |
| msgs..., | |
| ) | |
| proposal := GovernanceProposal{ | |
| Metadata: metadata, | |
| Deposit: deposit, | |
| Title: title, | |
| Summary: summary, | |
| Expedited: expedited, | |
| } | |
| for _, msg := range msgs { | |
| msgJson, err := modcodec.MarshalInterfaceJSON(msg) | |
| if err != nil { | |
| return "", fmt.Errorf("failed marshalling message '%v' for gov proposal: err=%s", msg, err) | |
| } | |
| proposal.Messages = append(proposal.Messages, msgJson) | |
| } | |
| raw, err := json.MarshalIndent(proposal, "", " ") | |
| if err != nil { | |
| return "", fmt.Errorf("failed to marshal proposal: %w", err) | |
| } | |
| return string(raw), nil | |
| } |
tests/e2e/main.go
Outdated
| "permissionless-ics": { | ||
| name: "permissionless-ics", | ||
| steps: stepsPermissionlessICS(), | ||
| description: "test permissionless ics", | ||
| testConfig: DefaultTestCfg, | ||
| }, |
There was a problem hiding this comment.
Addition of the permissionless-ics test case.
The new test case permissionless-ics is added to the stepChoices map. However, there are static analysis failures indicating that the function stepsPermissionlessICS is undefined.
Ensure that the function stepsPermissionlessICS is defined and correctly implemented. This function is crucial for the execution of the permissionless-ics test case.
#!/bin/bash
# Description: Verify the definition of `stepsPermissionlessICS`.
# Test: Search for the function definition. Expect: Function definition should exist.
rg --type go -A 5 $'func stepsPermissionlessICS'Tools
GitHub Check: test-e2e
[failure] 253-253:
undefined: stepsPermissionlessICS
GitHub Check: test-e2e-compatibility
[failure] 253-253:
undefined: stepsPermissionlessICS
tests/e2e/actions.go
Outdated
| func (tr Chain) UpdateConsumer(providerChain ChainID, validator ValidatorID, update types.MsgUpdateConsumer) { | ||
|
|
||
| fmt.Println("Update proposal for consumer_id=", update.ConsumerId) | ||
| content, err := json.Marshal(update) | ||
| if err != nil { | ||
| log.Fatalf("failed marshalling MsgUpdate", err.Error()) | ||
| } | ||
| jsonFile := "/update_consumer.json" | ||
| bz, err := tr.target.ExecCommand( | ||
| "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile), | ||
| ).CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal(err, "\n", string(bz)) | ||
| } | ||
|
|
||
| // Send consumer chain update | ||
| cmd := tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "tx", "provider", "update-consumer", jsonFile, | ||
| `--from`, `validator`+fmt.Sprint(validator), | ||
| `--chain-id`, string(tr.testConfig.chainConfigs[providerChain].ChainId), | ||
| `--home`, tr.getValidatorHome(providerChain, validator), | ||
| `--gas`, `900000`, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| `--keyring-backend`, `test`, | ||
| "--output", "json", | ||
| `-y`, | ||
| ) | ||
|
|
||
| fmt.Println("Update Consumer", "cmd:", cmd.String(), "content:\n", string(content)) | ||
|
|
||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal("update consumer failed ", "error: ", err, "output: ", string(bz)) | ||
| } | ||
|
|
||
| fmt.Println("update consumer", "output", string(bz)) | ||
|
|
||
| // Check transaction | ||
| txResponse := &TxResponse{} | ||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx response on update-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| if txResponse.Code != 0 { | ||
| log.Fatalf("sending update-consumer transaction failed with error code %d, Log:'%s'", txResponse.Code, txResponse.RawLog) | ||
| } | ||
| tr.waitBlocks(ChainID("provi"), 2, 10*time.Second) | ||
| } |
There was a problem hiding this comment.
Potential security issue with unsafe quoting in JSON operations.
The method uses potentially unsafe quoting which could lead to security vulnerabilities if the JSON content contains single quotes. Consider using parameterized or properly escaped queries to mitigate injection risks.
Apply this diff to fix the unsafe quoting issue:
- "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile),
+ "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, json.EscapeString(content), jsonFile),Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (4)
tests/e2e/state.go (2)
469-488: Approved: Enhanced handling of consumer-related messages inGetProposal.The method now correctly handles
MsgUpdateConsumerandMsgConsumerAddition, aligning with the enhanced consumer chain management. The logic for extracting parameters from the proposal's raw content and constructing theConsumerAdditionProposalstruct looks good.Consider adding unit tests to cover these new scenarios and ensure they are handled correctly. Let me know if you would like assistance in writing these tests or if I should open a GitHub issue to track this task.
Tools
GitHub Check: CodeQL
[warning] 473-477: Iteration over map
Iteration over map may be a possible source of non-determinism
930-934: Potential non-determinism in map iteration, but acceptable in this case.The code segment iterates over the
chainConfigsmap to find the chain ID corresponding to eachconsumer_chain_idreturned by the query. While iterating over a map does not guarantee a consistent order, which could lead to non-deterministic behavior, in this specific case, the order of iteration does not appear to affect the correctness of the result.The goal here is to find and append the matching chain IDs to the
chainsslice, and the order in which they are appended does not seem to matter.If strict ordering of the resulting
chainsslice is required in the future, consider using an alternative data structure that preserves insertion order, such as a slice of structs containing both theConsumerIDandChainID.Tools
GitHub Check: CodeQL
[warning] 930-934: Iteration over map
Iteration over map may be a possible source of non-determinismtests/e2e/actions.go (2)
278-315: Code looks good, with a minor nitpick.The
updateConsumerChainfunction correctly constructs anMsgUpdateConsumermessage and executes the update.Nitpick: Consider reordering the fields in
UpdateConsumerChainActionstruct to match the order inConsumerInitializationParametersfor better consistency, as suggested in a previous comment.
491-491: Clarify the scope of the TODO comment.The TODO comment should specify whether introducing the
waitForTxfunction is related to permissionless functionality or general improvements.Update the TODO comment to reflect its specific context or link it to an issue tracker for better tracking and management:
// TODO (PERMISSIONLESS): introduce waitForTx // See GitHub issue: https://github.com/cosmos/interchain-security/issues/123
tests/e2e/actions.go
Outdated
| func (tr Chain) UpdateConsumer(providerChain ChainID, validator ValidatorID, update types.MsgUpdateConsumer) { | ||
| content, err := json.Marshal(update) | ||
| if err != nil { | ||
| log.Fatal("failed marshalling MsgUpdate: ", err.Error()) | ||
| } | ||
| jsonFile := "/update_consumer.json" | ||
| bz, err := tr.target.ExecCommand( | ||
| "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile), | ||
| ).CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal(err, "\n", string(bz)) | ||
| } | ||
|
|
||
| // Send consumer chain update | ||
| cmd := tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "tx", "provider", "update-consumer", jsonFile, | ||
| `--from`, `validator`+fmt.Sprint(validator), | ||
| `--chain-id`, string(tr.testConfig.chainConfigs[providerChain].ChainId), | ||
| `--home`, tr.getValidatorHome(providerChain, validator), | ||
| `--gas`, `900000`, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| `--keyring-backend`, `test`, | ||
| "--output", "json", | ||
| `-y`, | ||
| ) | ||
|
|
||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal("update consumer failed ", "error: ", err, "output: ", string(bz)) | ||
| } | ||
|
|
||
| // Check transaction | ||
| txResponse := &TxResponse{} | ||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx response on update-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| if txResponse.Code != 0 { | ||
| log.Fatalf("sending update-consumer transaction failed with error code %d, Log:'%s'", txResponse.Code, txResponse.RawLog) | ||
| } | ||
| tr.waitBlocks(ChainID("provi"), 2, 10*time.Second) | ||
| } |
There was a problem hiding this comment.
Fix potentially unsafe JSON quoting.
The UpdateConsumer method looks good overall, but there is an issue with potentially unsafe JSON quoting on line 401. If the content variable contains single quotes, it could break out of the enclosing single quotes and lead to injection vulnerabilities.
To fix this, consider using strconv.Quote to safely escape the content string before embedding it in the JSON:
"/bin/bash", "-c", fmt.Sprintf(`echo %s > %s`, strconv.Quote(string(content)), jsonFile),| } | ||
| jsonFile := "/create_consumer.json" | ||
| bz, err := tr.target.ExecCommand( | ||
| "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile), |
There was a problem hiding this comment.
Fix potentially unsafe JSON quoting.
Similar to the previous comment, the JSON quoting on line 456 is potentially unsafe. If the content variable contains single quotes, it could break out of the enclosing single quotes and lead to injection vulnerabilities.
Use strconv.Quote to safely escape the content string before embedding it in the JSON:
"/bin/bash", "-c", fmt.Sprintf(`echo %s > %s`, strconv.Quote(string(content)), jsonFile),Tools
GitHub Check: CodeQL
[failure] 456-456: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (2)
tests/e2e/steps_partial_set_security.go (1)
Line range hint
448-685: Is the Top N feature supported in the core logic of permissionless?The test assumes that the Top N feature is not supported in the core logic of permissionless at its first implementation, which is causing the test to fail.
If this feature is supported, please add a TODO to make this test pass. Let me know if you need any clarification or assistance.
tests/e2e/actions.go (1)
261-276: Looks good!The
UpdateConsumerChainActionstruct definition is approved. It can be useful for updating consumer chains.Consider reordering some fields to match the order in
ConsumerInitializationParametersfor consistency:type UpdateConsumerChainAction struct { Chain ChainID From ValidatorID ConsumerChain ChainID NewOwner string SpawnTime uint InitialHeight clienttypes.Height DistributionChannel string TopN uint32 ValidatorsPowerCap uint32 ValidatorSetCap uint32 Allowlist []string Denylist []string MinStake uint64 AllowInactiveVals bool }
tests/e2e/actions.go
Outdated
| func (tr Chain) UpdateConsumer(providerChain ChainID, validator ValidatorID, update types.MsgUpdateConsumer) { | ||
| content, err := json.Marshal(update) | ||
| if err != nil { | ||
| log.Fatal("failed marshalling MsgUpdateConsumer: ", err.Error()) | ||
| } | ||
| jsonFile := "/update_consumer.json" | ||
| bz, err := tr.target.ExecCommand( | ||
| "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile), | ||
| ).CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal(err, "\n", string(bz)) | ||
| } | ||
|
|
||
| // Send consumer chain update | ||
| cmd := tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "tx", "provider", "update-consumer", jsonFile, | ||
| `--from`, `validator`+fmt.Sprint(validator), | ||
| `--chain-id`, string(tr.testConfig.chainConfigs[providerChain].ChainId), | ||
| `--home`, tr.getValidatorHome(providerChain, validator), | ||
| `--gas`, `900000`, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| `--keyring-backend`, `test`, | ||
| "--output", "json", | ||
| `-y`, | ||
| ) | ||
|
|
||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal("update consumer failed ", "error: ", err, "output: ", string(bz)) | ||
| } | ||
|
|
||
| // Check transaction | ||
| txResponse := &TxResponse{} | ||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx response on update-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| if txResponse.Code != 0 { | ||
| log.Fatalf("sending update-consumer transaction failed with error code %d, Log:'%s'", txResponse.Code, txResponse.RawLog) | ||
| } | ||
| tr.waitBlocks(ChainID("provi"), 2, 10*time.Second) | ||
| } |
There was a problem hiding this comment.
Looks good overall, but fix the potentially unsafe JSON quoting.
The UpdateConsumer method is approved. It properly marshals the update message, sends it to the provider chain, and checks the transaction response.
However, there is a potentially unsafe JSON quoting issue on line 401. If the content variable contains single quotes, it could break out of the enclosing single quotes and lead to injection vulnerabilities.
To fix this, consider using strconv.Quote to safely escape the content string before embedding it in the JSON:
"/bin/bash", "-c", fmt.Sprintf(`echo %s > %s`, strconv.Quote(string(content)), jsonFile),
tests/e2e/actions.go
Outdated
| func (tr Chain) CreateConsumer(providerChain, consumerChain ChainID, validator ValidatorID, metadata types.ConsumerMetadata, initParams *types.ConsumerInitializationParameters, powerShapingParams *types.PowerShapingParameters) ConsumerID { | ||
|
|
||
| chainID := string(tr.testConfig.chainConfigs[consumerChain].ChainId) | ||
| rec := types.MsgCreateConsumer{ | ||
| ChainId: chainID, | ||
| Metadata: metadata, | ||
| InitializationParameters: initParams, | ||
| PowerShapingParameters: powerShapingParams, | ||
| } | ||
|
|
||
| content, err := json.Marshal(rec) | ||
| if err != nil { | ||
| log.Fatalf("failed marshalling MsgCreateConsumer: %s", err.Error()) | ||
| } | ||
| jsonFile := "/create_consumer.json" | ||
| bz, err := tr.target.ExecCommand( | ||
| "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile), | ||
| ).CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal(err, "\n", string(bz)) | ||
| } | ||
|
|
||
| // Send consumer chain creation | ||
| cmd := tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "tx", "provider", "create-consumer", jsonFile, | ||
| `--from`, `validator`+fmt.Sprint(validator), | ||
| `--chain-id`, string(tr.testConfig.chainConfigs[providerChain].ChainId), | ||
| `--home`, tr.getValidatorHome(providerChain, validator), | ||
| `--gas`, `900000`, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| `--keyring-backend`, `test`, | ||
| "--output", "json", | ||
| `-y`, | ||
| ) | ||
|
|
||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal("create consumer failed ", "error: ", err, "output: ", string(bz)) | ||
| } | ||
|
|
||
| txResponse := &TxResponse{} | ||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx response on create-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| if txResponse.Code != 0 { | ||
| log.Fatalf("sending transaction failed with error code %d, Log:'%s'", txResponse.Code, txResponse.RawLog) | ||
| } | ||
|
|
||
| // TODO: introduce waitForTx | ||
| tr.waitBlocks(providerChain, 2, 10*time.Second) | ||
|
|
||
| // Get Consumer ID from transaction | ||
| cmd = tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "query", "tx", txResponse.TxHash, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| "--output", "json", | ||
| ) | ||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal("not able to query tx containing creation-consumer: cmd:", cmd, "err:", err.Error(), "out:", string(bz)) | ||
| } | ||
|
|
||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx containing create-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| consumerId := "" | ||
| for _, event := range txResponse.Events { | ||
| if event.Type != "consumer_creation" { | ||
| continue | ||
| } | ||
| attr, exists := event.GetAttribute("consumer_id") | ||
| if !exists { | ||
| log.Fatalf("no event with consumer_id found in tx content of create-consumer: %v", event) | ||
| } | ||
| consumerId = attr.Value | ||
| } | ||
| if consumerId == "" { | ||
| log.Fatalf("no consumer-id found in consumer creation transaction events for chain '%s'. events: %v", consumerChain, txResponse.Events) | ||
| } | ||
|
|
||
| cfg, exists := tr.testConfig.chainConfigs[e2e.ChainID(chainID)] | ||
| if !exists { | ||
| log.Fatal("no chain config found for consumer chain", chainID) | ||
| } | ||
| if cfg.ConsumerId != "" && cfg.ConsumerId != e2e.ConsumerID(consumerId) { | ||
| log.Fatal("chain ", chainID, " registered already with a different consumer ID", consumerId) | ||
| } | ||
|
|
||
| // Set the new created consumer-id on the chain's config | ||
| cfg.ConsumerId = e2e.ConsumerID(consumerId) | ||
| tr.testConfig.chainConfigs[e2e.ChainID(chainID)] = cfg | ||
|
|
||
| return e2e.ConsumerID(consumerId) | ||
| } |
There was a problem hiding this comment.
Looks good overall, but fix the potentially unsafe JSON quoting.
The CreateConsumer method is approved. It properly constructs the MsgCreateConsumer message, sends it to the provider chain, retrieves the consumer ID from the transaction events, and updates the chain config with the new consumer ID.
However, there is a potentially unsafe JSON quoting issue on line 456, similar to the previous method. If the content variable contains single quotes, it could break out of the enclosing single quotes and lead to injection vulnerabilities.
To fix this, consider using strconv.Quote to safely escape the content string before embedding it in the JSON:
"/bin/bash", "-c", fmt.Sprintf(`echo %s > %s`, strconv.Quote(string(content)), jsonFile),Tools
GitHub Check: CodeQL
[failure] 456-456: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
tests/e2e/actions.go
Outdated
| func (tr Chain) UpdateConsumer(providerChain ChainID, validator ValidatorID, update types.MsgUpdateConsumer) { | ||
| content, err := json.Marshal(update) | ||
| if err != nil { | ||
| log.Fatal("failed marshalling MsgUpdateConsumer: ", err.Error()) | ||
| } | ||
| jsonFile := "/update-consumer.json" | ||
| bz, err := tr.target.ExecCommand( | ||
| "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile), | ||
| ).CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal(err, "\n", string(bz)) | ||
| } | ||
|
|
||
| // Send consumer chain update | ||
| cmd := tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "tx", "provider", "update-consumer", jsonFile, | ||
| `--from`, `validator`+fmt.Sprint(validator), | ||
| `--chain-id`, string(tr.testConfig.chainConfigs[providerChain].ChainId), | ||
| `--home`, tr.getValidatorHome(providerChain, validator), | ||
| `--gas`, `900000`, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| `--keyring-backend`, `test`, | ||
| "--output", "json", | ||
| `-y`, | ||
| ) | ||
|
|
||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal("update consumer failed ", "error: ", err, "output: ", string(bz)) | ||
| } | ||
|
|
||
| // Check transaction | ||
| txResponse := &TxResponse{} | ||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx response on update-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| if txResponse.Code != 0 { | ||
| log.Fatalf("sending update-consumer transaction failed with error code %d, Log:'%s'", txResponse.Code, txResponse.RawLog) | ||
| } | ||
| tr.waitBlocks(ChainID("provi"), 2, 10*time.Second) | ||
| } |
There was a problem hiding this comment.
Fix potentially unsafe JSON quoting.
The UpdateConsumer method looks good overall, but there is an issue with potentially unsafe JSON quoting on line 401. If the content variable contains single quotes, it could break out of the enclosing single quotes and lead to injection vulnerabilities.
To fix this, consider using strconv.Quote to safely escape the content string before embedding it in the JSON:
"/bin/bash", "-c", fmt.Sprintf(`echo %s > %s`, strconv.Quote(string(content)), jsonFile),
tests/e2e/actions.go
Outdated
|
|
||
| chainID := string(tr.testConfig.chainConfigs[consumerChain].ChainId) | ||
| msg := types.MsgCreateConsumer{ | ||
| ChainId: chainID, | ||
| Metadata: metadata, | ||
| InitializationParameters: initParams, | ||
| PowerShapingParameters: powerShapingParams, | ||
| } | ||
|
|
||
| content, err := json.Marshal(msg) | ||
| if err != nil { | ||
| log.Fatalf("failed marshalling MsgCreateConsumer: %s", err.Error()) | ||
| } | ||
| jsonFile := "/create-consumer.json" | ||
| bz, err := tr.target.ExecCommand( | ||
| "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile), | ||
| ).CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal(err, "\n", string(bz)) | ||
| } | ||
|
|
||
| // Send consumer chain creation | ||
| cmd := tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "tx", "provider", "create-consumer", jsonFile, | ||
| `--from`, `validator`+fmt.Sprint(validator), | ||
| `--chain-id`, string(tr.testConfig.chainConfigs[providerChain].ChainId), | ||
| `--home`, tr.getValidatorHome(providerChain, validator), | ||
| `--gas`, `900000`, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| `--keyring-backend`, `test`, | ||
| "--output", "json", | ||
| `-y`, | ||
| ) | ||
|
|
||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal("create consumer failed ", "error: ", err, "output: ", string(bz)) | ||
| } | ||
|
|
||
| txResponse := &TxResponse{} | ||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx response on create-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| if txResponse.Code != 0 { | ||
| log.Fatalf("sending transaction failed with error code %d, Log:'%s'", txResponse.Code, txResponse.RawLog) | ||
| } | ||
|
|
||
| // TODO: introduce waitForTx | ||
| tr.waitBlocks(providerChain, 2, 10*time.Second) | ||
|
|
||
| // Get Consumer ID from transaction | ||
| cmd = tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "query", "tx", txResponse.TxHash, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| "--output", "json", | ||
| ) | ||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatalf("not able to query tx containing creation-consumer: tx: %s, err: %s, out: %s", | ||
| txResponse.TxHash, err.Error(), string(bz)) | ||
| } | ||
|
|
||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx containing create-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| consumerId := "" | ||
| for _, event := range txResponse.Events { | ||
| if event.Type != "consumer_creation" { | ||
| continue | ||
| } | ||
| attr, exists := event.GetAttribute("consumer_id") | ||
| if !exists { | ||
| log.Fatalf("no event with consumer_id found in tx content of create-consumer: %v", event) | ||
| } | ||
| consumerId = attr.Value | ||
| } | ||
| if consumerId == "" { | ||
| log.Fatalf("no consumer-id found in consumer creation transaction events for chain '%s'. events: %v", consumerChain, txResponse.Events) | ||
| } | ||
|
|
||
| cfg, exists := tr.testConfig.chainConfigs[e2e.ChainID(chainID)] | ||
| if !exists { | ||
| log.Fatal("no chain config found for consumer chain", chainID) | ||
| } | ||
| if cfg.ConsumerId != "" && cfg.ConsumerId != e2e.ConsumerID(consumerId) { | ||
| log.Fatalf("chain '%s'registered already with a different consumer ID '%s'", chainID, consumerId) | ||
| } | ||
|
|
||
| // Set the new created consumer-id on the chain's config | ||
| cfg.ConsumerId = e2e.ConsumerID(consumerId) | ||
| tr.testConfig.chainConfigs[e2e.ChainID(chainID)] = cfg | ||
|
|
||
| return e2e.ConsumerID(consumerId) | ||
| } |
There was a problem hiding this comment.
Fix potentially unsafe JSON quoting.
The CreateConsumer method looks good overall. It properly constructs the MsgCreateConsumer message, sends it to the provider chain, retrieves the consumer ID from the transaction events, and updates the chain config with the new consumer ID.
However, there is a potentially unsafe JSON quoting issue on line 456, similar to the previous method. If the content variable contains single quotes, it could break out of the enclosing single quotes and lead to injection vulnerabilities.
To fix this, consider using strconv.Quote to safely escape the content string before embedding it in the JSON:
"/bin/bash", "-c", fmt.Sprintf(`echo %s > %s`, strconv.Quote(string(content)), jsonFile),Tools
GitHub Check: CodeQL
[failure] 456-456: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
There was a problem hiding this comment.
I left some additional comments (e.g., if we do not use Consumer2ChainID, can we remove it?).
Also, this PR's description has that happy-path has passed but in the PR tests it seems to fail. Is this expected?
It makes sense that the consumer-double-sign and consumer-misbehaviour fail because Hermes does not yet support consumerIds.
In this PR's description, it is written that the inactive-vals-topN is not passing. Can we have this passing as well before we merge this PR? At least, on a first sight, it should not fail due to permissionless.
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (5)
tests/e2e/steps_partial_set_security.go (1)
Line range hint
415-646: TODO PERMISSIONLESS: ThestepsTopNChaintest is failing.The
stepsTopNChaintest is marked as "TODO PERMISSIONLESS" and is currently failing. This indicates that the test needs to be updated to work with the permissionless functionality.Do you want me to update the test to work with the permissionless functionality? I can open a GitHub issue to track this task.
tests/e2e/actions.go (4)
261-276: Consider reordering some fields for consistency.Suggest reordering the following fields to match the order in related types:
- Move
SpawnTimebeforeInitialHeightto match the order inConsumerInitializationParameters.Apply this diff to reorder the fields:
type UpdateConsumerChainAction struct { Chain ChainID From ValidatorID ConsumerChain ChainID NewOwner string + SpawnTime uint InitialHeight clienttypes.Height - SpawnTime uint DistributionChannel string TopN uint32 ValidatorsPowerCap uint32 ValidatorSetCap uint32 Allowlist []string Denylist []string MinStake uint64 AllowInactiveVals bool }
278-315: Use consistent naming conventions.Suggest using camel case consistently for variable names:
- Rename
consumerIdtoconsumerIDto match the naming convention used in other places.
Line range hint
543-660: Use consistent naming conventions.The refactoring of the
submitConsumerAdditionProposalmethod looks good. It improves clarity and maintainability by using the new message structures and proposal content generation.Suggest renaming the following variables to start with lowercase letters for consistency with Go naming conventions:
- Rename
Metadatatometadata.- Rename
InitializationParameterstoinitializationParameters.Tools
Gitleaks
573-573: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
GitHub Check: CodeQL
[failure] 456-456: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
[failure] 613-613: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
Line range hint
879-939: Use consistent naming conventions.The refactoring of the
submitConsumerModificationProposalmethod looks good. It improves clarity and maintainability by using the new message structures and proposal content generation.Suggest using camel case consistently for variable names:
- Rename
consumerIdtoconsumerIDto match the naming convention used in other places.Tools
Gitleaks
886-886: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
GitHub Check: CodeQL
[failure] 904-904: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
| for _, chainCfg := range tr.chainConfigs { | ||
| if chainCfg.ConsumerId == e2e.ConsumerID(consumerId) { | ||
| consumerChainId = chainCfg.ChainId | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
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
bermuell
force-pushed
the
bernd/permissionless_ics
branch
from
694f560
to
61c853e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (1)
tests/e2e/steps_partial_set_security.go (1)
Line range hint
437-642: Update the function to use the new consumer chain creation and update pattern.The function still uses the old
SubmitConsumerAdditionProposalActionandVoteGovProposalActionpattern. Please update it to use the newCreateConsumerChainActionandUpdateConsumerChainActionactions, similar to the changes made in thestepsOptInChainfunction.
| 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 | ||
| break | ||
| } | ||
| } | ||
|
|
||
| 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 |
There was a problem hiding this comment.
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
tests/e2e/actions.go
Outdated
| chainID := string(tr.testConfig.chainConfigs[consumerChain].ChainId) | ||
| msg := types.MsgCreateConsumer{ | ||
| ChainId: chainID, | ||
| Metadata: metadata, | ||
| InitializationParameters: initParams, | ||
| PowerShapingParameters: powerShapingParams, | ||
| } | ||
|
|
||
| content, err := json.Marshal(msg) | ||
| if err != nil { | ||
| log.Fatalf("failed marshalling MsgCreateConsumer: %s", err.Error()) | ||
| } | ||
| jsonFile := "/create-consumer.json" | ||
| bz, err := tr.target.ExecCommand( | ||
| "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile), | ||
| ).CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal(err, "\n", string(bz)) | ||
| } | ||
|
|
||
| // Send consumer chain creation | ||
| cmd := tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "tx", "provider", "create-consumer", jsonFile, | ||
| `--from`, `validator`+fmt.Sprint(validator), | ||
| `--chain-id`, string(tr.testConfig.chainConfigs[providerChain].ChainId), | ||
| `--home`, tr.getValidatorHome(providerChain, validator), | ||
| `--gas`, `900000`, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| `--keyring-backend`, `test`, | ||
| "--output", "json", | ||
| `-y`, | ||
| ) | ||
|
|
||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal("create consumer failed ", "error: ", err, "output: ", string(bz)) | ||
| } | ||
|
|
||
| txResponse := &TxResponse{} | ||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx response on create-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| if txResponse.Code != 0 { | ||
| log.Fatalf("sending transaction failed with error code %d, Log:'%s'", txResponse.Code, txResponse.RawLog) | ||
| } | ||
|
|
||
| // TODO: introduce waitForTx | ||
| tr.waitBlocks(providerChain, 2, 10*time.Second) | ||
|
|
||
| // Get Consumer ID from transaction | ||
| cmd = tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "query", "tx", txResponse.TxHash, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| "--output", "json", | ||
| ) | ||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatalf("not able to query tx containing creation-consumer: tx: %s, err: %s, out: %s", | ||
| txResponse.TxHash, err.Error(), string(bz)) | ||
| } | ||
| fmt.Println("@@@@ created consumer chain tx=", txResponse.TxHash) | ||
|
|
||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx containing create-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| consumerId := "" | ||
| for _, event := range txResponse.Events { | ||
| if event.Type != "consumer_creation" { | ||
| continue | ||
| } | ||
| attr, exists := event.GetAttribute("consumer_id") | ||
| if !exists { | ||
| log.Fatalf("no event with consumer_id found in tx content of create-consumer: %v", event) | ||
| } | ||
| consumerId = attr.Value | ||
| } | ||
| if consumerId == "" { | ||
| log.Fatalf("no consumer-id found in consumer creation transaction events for chain '%s'. events: %v", consumerChain, txResponse.Events) | ||
| } | ||
|
|
||
| cfg, exists := tr.testConfig.chainConfigs[e2e.ChainID(chainID)] | ||
| if !exists { | ||
| log.Fatal("no chain config found for consumer chain", chainID) | ||
| } | ||
| if cfg.ConsumerId != "" && cfg.ConsumerId != e2e.ConsumerID(consumerId) { | ||
| log.Fatalf("chain '%s'registered already with a different consumer ID '%s'", chainID, consumerId) | ||
| } | ||
|
|
||
| // Set the new created consumer-id on the chain's config | ||
| cfg.ConsumerId = e2e.ConsumerID(consumerId) | ||
| tr.testConfig.chainConfigs[e2e.ChainID(chainID)] = cfg | ||
|
|
||
| return e2e.ConsumerID(consumerId) | ||
| } |
There was a problem hiding this comment.
Looks good overall, but fix the potentially unsafe JSON quoting.
The CreateConsumer method is approved. It properly constructs the MsgCreateConsumer message, sends it to the provider chain, retrieves the consumer ID from the transaction events, and updates the chain config with the new consumer ID.
However, there is a potentially unsafe JSON quoting issue on line 456, similar to the previous method. If the content variable contains single quotes, it could break out of the enclosing single quotes and lead to injection vulnerabilities.
To fix this, consider using strconv.Quote to safely escape the content string before embedding it in the JSON:
"/bin/bash", "-c", fmt.Sprintf(`echo %s > %s`, strconv.Quote(string(content)), jsonFile),Tools
GitHub Check: CodeQL
[failure] 456-456: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
Tools
GitHub Check: CodeQL
[failure] 456-456: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
| func (tr Chain) UpdateConsumer(providerChain ChainID, validator ValidatorID, update types.MsgUpdateConsumer) { | ||
| content, err := json.Marshal(update) | ||
| if err != nil { | ||
| log.Fatal("failed marshalling MsgUpdateConsumer: ", err.Error()) | ||
| } | ||
| jsonFile := "/update-consumer.json" | ||
| bz, err := tr.target.ExecCommand( | ||
| "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile), | ||
| ).CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal(err, "\n", string(bz)) | ||
| } | ||
|
|
||
| // Send consumer chain update | ||
| cmd := tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "tx", "provider", "update-consumer", jsonFile, | ||
| `--from`, `validator`+fmt.Sprint(validator), | ||
| `--chain-id`, string(tr.testConfig.chainConfigs[providerChain].ChainId), | ||
| `--home`, tr.getValidatorHome(providerChain, validator), | ||
| `--gas`, `900000`, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| `--keyring-backend`, `test`, | ||
| "--output", "json", | ||
| `-y`, | ||
| ) | ||
|
|
||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal("update consumer failed ", "error: ", err, "output: ", string(bz)) | ||
| } | ||
|
|
||
| // Check transaction | ||
| txResponse := &TxResponse{} | ||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx response on update-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| if txResponse.Code != 0 { | ||
| log.Fatalf("sending update-consumer transaction failed with error code %d, Log:'%s'", txResponse.Code, txResponse.RawLog) | ||
| } | ||
| tr.waitBlocks(providerChain, 2, 10*time.Second) | ||
| } |
There was a problem hiding this comment.
Fix potentially unsafe JSON quoting.
The UpdateConsumer method looks good overall, but there is an issue with potentially unsafe JSON quoting on line 401. If the content variable contains single quotes, it could break out of the enclosing single quotes and lead to injection vulnerabilities.
To fix this, consider using strconv.Quote to safely escape the content string before embedding it in the JSON:
"/bin/bash", "-c", fmt.Sprintf(`echo %s > %s`, strconv.Quote(string(content)), jsonFile),|
|
||
| chainID := string(tr.testConfig.chainConfigs[consumerChain].ChainId) | ||
| msg := types.MsgCreateConsumer{ | ||
| ChainId: chainID, | ||
| Metadata: metadata, | ||
| InitializationParameters: initParams, | ||
| PowerShapingParameters: powerShapingParams, | ||
| } | ||
|
|
||
| content, err := json.Marshal(msg) | ||
| if err != nil { | ||
| log.Fatalf("failed marshalling MsgCreateConsumer: %s", err.Error()) | ||
| } | ||
| jsonFile := "/create-consumer.json" | ||
| bz, err := tr.target.ExecCommand( | ||
| "/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, content, jsonFile), | ||
| ).CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal(err, "\n", string(bz)) | ||
| } | ||
|
|
||
| // Send consumer chain creation | ||
| cmd := tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "tx", "provider", "create-consumer", jsonFile, | ||
| `--from`, `validator`+fmt.Sprint(validator), | ||
| `--chain-id`, string(tr.testConfig.chainConfigs[providerChain].ChainId), | ||
| `--home`, tr.getValidatorHome(providerChain, validator), | ||
| `--gas`, `900000`, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| `--keyring-backend`, `test`, | ||
| "--output", "json", | ||
| `-y`, | ||
| ) | ||
|
|
||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatal("create consumer failed ", "error: ", err, "output: ", string(bz)) | ||
| } | ||
|
|
||
| txResponse := &TxResponse{} | ||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx response on create-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| if txResponse.Code != 0 { | ||
| log.Fatalf("sending transaction failed with error code %d, Log:'%s'", txResponse.Code, txResponse.RawLog) | ||
| } | ||
|
|
||
| // TODO: introduce waitForTx (see issue #2198) | ||
| tr.waitBlocks(providerChain, 2, 10*time.Second) | ||
|
|
||
| // Get Consumer ID from transaction | ||
| cmd = tr.target.ExecCommand( | ||
| tr.testConfig.chainConfigs[providerChain].BinaryName, | ||
| "query", "tx", txResponse.TxHash, | ||
| `--node`, tr.getValidatorNode(providerChain, validator), | ||
| "--output", "json", | ||
| ) | ||
| bz, err = cmd.CombinedOutput() | ||
| if err != nil { | ||
| log.Fatalf("not able to query tx containing creation-consumer: tx: %s, err: %s, out: %s", | ||
| txResponse.TxHash, err.Error(), string(bz)) | ||
| } | ||
|
|
||
| err = json.Unmarshal(bz, txResponse) | ||
| if err != nil { | ||
| log.Fatalf("unmarshalling tx containing create-consumer: %s, json: %s", err.Error(), string(bz)) | ||
| } | ||
|
|
||
| consumerId := "" | ||
| for _, event := range txResponse.Events { | ||
| if event.Type != "consumer_creation" { | ||
| continue | ||
| } | ||
| attr, exists := event.GetAttribute("consumer_id") | ||
| if !exists { | ||
| log.Fatalf("no event with consumer_id found in tx content of create-consumer: %v", event) | ||
| } | ||
| consumerId = attr.Value | ||
| } | ||
| if consumerId == "" { | ||
| log.Fatalf("no consumer-id found in consumer creation transaction events for chain '%s'. events: %v", consumerChain, txResponse.Events) | ||
| } | ||
|
|
||
| cfg, exists := tr.testConfig.chainConfigs[e2e.ChainID(chainID)] | ||
| if !exists { | ||
| log.Fatal("no chain config found for consumer chain", chainID) | ||
| } | ||
| if cfg.ConsumerId != "" && cfg.ConsumerId != e2e.ConsumerID(consumerId) { | ||
| log.Fatalf("chain '%s'registered already with a different consumer ID '%s'", chainID, consumerId) | ||
| } | ||
|
|
||
| // Set the new created consumer-id on the chain's config | ||
| cfg.ConsumerId = e2e.ConsumerID(consumerId) | ||
| tr.testConfig.chainConfigs[e2e.ChainID(chainID)] = cfg | ||
|
|
||
| return e2e.ConsumerID(consumerId) | ||
| } |
There was a problem hiding this comment.
Fix potentially unsafe JSON quoting.
The CreateConsumer method looks good overall. It properly constructs the MsgCreateConsumer message, sends it to the provider chain, retrieves the consumer ID from the transaction events, and updates the chain config with the new consumer ID.
However, there is a potentially unsafe JSON quoting issue on line 456, similar to the previous method. If the content variable contains single quotes, it could break out of the enclosing single quotes and lead to injection vulnerabilities.
To fix this, consider using strconv.Quote to safely escape the content string before embedding it in the JSON:
"/bin/bash", "-c", fmt.Sprintf(`echo %s > %s`, strconv.Quote(string(content)), jsonFile),Tools
GitHub Check: CodeQL
[failure] 456-456: Potentially unsafe quoting
If this JSON value contains a single quote, it could break out of the enclosing quotes.
* feat: first iteration on Permissionless ICS (#2117) * (partially) renamed chain ids to consumer ids * renamed proposal messages * removed global slash entry * fixed unit tests * added new messages * introduced new state * added functionality for the register and initialize messages * renamed (partially) chainIds to consumerIds * set consumerId to chainId association during registration * added extra check in the initialization so unknokwn, launched, or stopped chains cannot re-initialize * added initial work on traversing initialized chains that are to-be-launched * fixed rebase issues after bringing the VSCMaturedPackets work in * made it so we traverse initialization records instead of addition proposals (+ additional changes so the unit tests pass) * renamed more chainIDs to consumerIds * removed ClientIdToChainId state because chainId already resides on the registration record * nit fixes in go docs * removed MsgConsumerAddition * added CLI commands for new messages * removed consumer modification proposal * removed (partially) consumer removal proposal * rebased to pick up the inactive-validators work (PR #2079) * introduced consumerId in the equivocation messages (and a useful query for Hermes to get the consumerId) * added safeguard so that a validator cannot opt-in to two different chains with the same chain id * renamed some chainIDs to consumerIds * updated based on comments Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> * fixed integration tests * rebased to pick up the removal of legacy proposals (#2130) and re-introduced old messages so that existing proposals can deserialize * changes messages to only have MsgCreateConsumer and MsgUpdateConsumer and modified protos so that we are backward-compatible * cleaned up slightly a few things (mostly committing & pushing) so people can pick up the latest changes * fixed the CreateConsumer and UpdateConsumer logic and made most of the fields optional * fixed hooks and the code around proposalId to consumerId * feat: extend consumer validator query to return commission rate (backport #2162) (#2165) * adapt #2162 changes for permissionless ICS * nits --------- Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * renamed some chainIds to consumerIds * took into account comments and also added safeguard to reject new proposals that still use deprecated messages (e.g., MsgConsumerAddition, etc.) * Update x/ccv/provider/types/msg.go Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> * removed double-gas charge on MsgCreateConsumer and imroved the logic of MsgUpdateConsumer * added PopulateMinimumPowerInTopN tested * took into account comments (using protos for marshalling string slice, fixed issues in the UpdateConsumer logic, added extra check to abort spurious proposals) * feat: add fields to consumer validators query (#2167) * extend consumer validators query * nit * nits * fix msg order * deprecate power for consumer_power * modified the way we verify the new owner address, as well as nit refactoring on the ConsumerIds * fixed some rebase issues and changed a proto to be backward-compatible --------- Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * fixed bug on removing previous spawn time & added tests * added some additional tests * added tests on the hooks * removed check that spawn time is in the future * feat: refactor consumer validator set computation (#2175) * add UT * nits * address comments * Update x/ccv/provider/keeper/partial_set_security.go Co-authored-by: insumity <karolos@informal.systems> * fix tests --------- Co-authored-by: insumity <karolos@informal.systems> * fix!: ConsumerModificationProposal is needed in Gaia upgrade handler (#2176) ConsumerModificationProposal is needed in Gaia upgrade handler * fix: allow the owner of a chain to remove the chain (#2178) fixed access in RemoveConsumer * refactor: add proto enum for ConsumerPhase (#2182) * feat: extend `consumer_validators` query to return consumer valset before launch (#2164) * (partially) renamed chain ids to consumer ids * renamed proposal messages * removed global slash entry * fixed unit tests * added new messages * introduced new state * added functionality for the register and initialize messages * renamed (partially) chainIds to consumerIds * set consumerId to chainId association during registration * added extra check in the initialization so unknokwn, launched, or stopped chains cannot re-initialize * added initial work on traversing initialized chains that are to-be-launched * fixed rebase issues after bringing the VSCMaturedPackets work in * made it so we traverse initialization records instead of addition proposals (+ additional changes so the unit tests pass) * renamed more chainIDs to consumerIds * removed ClientIdToChainId state because chainId already resides on the registration record * nit fixes in go docs * removed MsgConsumerAddition * added CLI commands for new messages * removed consumer modification proposal * removed (partially) consumer removal proposal * rebased to pick up the inactive-validators work (PR #2079) * introduced consumerId in the equivocation messages (and a useful query for Hermes to get the consumerId) * added safeguard so that a validator cannot opt-in to two different chains with the same chain id * renamed some chainIDs to consumerIds * updated based on comments Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> * fixed integration tests * rebased to pick up the removal of legacy proposals (#2130) and re-introduced old messages so that existing proposals can deserialize * changes messages to only have MsgCreateConsumer and MsgUpdateConsumer and modified protos so that we are backward-compatible * cleaned up slightly a few things (mostly committing & pushing) so people can pick up the latest changes * fixed the CreateConsumer and UpdateConsumer logic and made most of the fields optional * fixed hooks and the code around proposalId to consumerId * pre-spawn query * rebase * nits * feat: extend consumer validator query to return commission rate (backport #2162) (#2165) * adapt #2162 changes for permissionless ICS * nits --------- Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * remove panics * renamed some chainIds to consumerIds * took into account comments and also added safeguard to reject new proposals that still use deprecated messages (e.g., MsgConsumerAddition, etc.) * Update x/ccv/provider/types/msg.go Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> * removed double-gas charge on MsgCreateConsumer and imroved the logic of MsgUpdateConsumer * added PopulateMinimumPowerInTopN tested * took into account comments (using protos for marshalling string slice, fixed issues in the UpdateConsumer logic, added extra check to abort spurious proposals) * update logic * took into account comments (using protos for marshalling string slice, fixed issues in the UpdateConsumer logic, added extra check to abort spurious proposals) * feat: add fields to consumer validators query (#2167) * extend consumer validators query * nit * nits * fix msg order * deprecate power for consumer_power * nits * feat: first iteration on Permissionless ICS (#2117) * (partially) renamed chain ids to consumer ids * renamed proposal messages * removed global slash entry * fixed unit tests * added new messages * introduced new state * added functionality for the register and initialize messages * renamed (partially) chainIds to consumerIds * set consumerId to chainId association during registration * added extra check in the initialization so unknokwn, launched, or stopped chains cannot re-initialize * added initial work on traversing initialized chains that are to-be-launched * fixed rebase issues after bringing the VSCMaturedPackets work in * made it so we traverse initialization records instead of addition proposals (+ additional changes so the unit tests pass) * renamed more chainIDs to consumerIds * removed ClientIdToChainId state because chainId already resides on the registration record * nit fixes in go docs * removed MsgConsumerAddition * added CLI commands for new messages * removed consumer modification proposal * removed (partially) consumer removal proposal * rebased to pick up the inactive-validators work (PR #2079) * introduced consumerId in the equivocation messages (and a useful query for Hermes to get the consumerId) * added safeguard so that a validator cannot opt-in to two different chains with the same chain id * renamed some chainIDs to consumerIds * updated based on comments Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> * fixed integration tests * rebased to pick up the removal of legacy proposals (#2130) and re-introduced old messages so that existing proposals can deserialize * changes messages to only have MsgCreateConsumer and MsgUpdateConsumer and modified protos so that we are backward-compatible * cleaned up slightly a few things (mostly committing & pushing) so people can pick up the latest changes * fixed the CreateConsumer and UpdateConsumer logic and made most of the fields optional * fixed hooks and the code around proposalId to consumerId * feat: extend consumer validator query to return commission rate (backport #2162) (#2165) * adapt #2162 changes for permissionless ICS * nits --------- Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * renamed some chainIds to consumerIds * took into account comments and also added safeguard to reject new proposals that still use deprecated messages (e.g., MsgConsumerAddition, etc.) * Update x/ccv/provider/types/msg.go Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> * removed double-gas charge on MsgCreateConsumer and imroved the logic of MsgUpdateConsumer * added PopulateMinimumPowerInTopN tested * took into account comments (using protos for marshalling string slice, fixed issues in the UpdateConsumer logic, added extra check to abort spurious proposals) * feat: add fields to consumer validators query (#2167) * extend consumer validators query * nit * nits * fix msg order * deprecate power for consumer_power * modified the way we verify the new owner address, as well as nit refactoring on the ConsumerIds * fixed some rebase issues and changed a proto to be backward-compatible --------- Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * cover stopped phase case * fixed bug on removing previous spawn time & added tests * added some additional tests * added tests on the hooks * removed check that spawn time is in the future * feat: refactor consumer validator set computation (#2175) * add UT * nits * address comments * Update x/ccv/provider/keeper/partial_set_security.go Co-authored-by: insumity <karolos@informal.systems> * fix tests --------- Co-authored-by: insumity <karolos@informal.systems> * nit * nits * nit --------- Co-authored-by: insumity <karolos@informal.systems> Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * fix build * tests: fix integration test setup (#2183) * fix integration test setup * fix integration tests * feat: add consumer chain query (#2179) * feat: first iteration on Permissionless ICS (#2117) * (partially) renamed chain ids to consumer ids * renamed proposal messages * removed global slash entry * fixed unit tests * added new messages * introduced new state * added functionality for the register and initialize messages * renamed (partially) chainIds to consumerIds * set consumerId to chainId association during registration * added extra check in the initialization so unknokwn, launched, or stopped chains cannot re-initialize * added initial work on traversing initialized chains that are to-be-launched * fixed rebase issues after bringing the VSCMaturedPackets work in * made it so we traverse initialization records instead of addition proposals (+ additional changes so the unit tests pass) * renamed more chainIDs to consumerIds * removed ClientIdToChainId state because chainId already resides on the registration record * nit fixes in go docs * removed MsgConsumerAddition * added CLI commands for new messages * removed consumer modification proposal * removed (partially) consumer removal proposal * rebased to pick up the inactive-validators work (PR #2079) * introduced consumerId in the equivocation messages (and a useful query for Hermes to get the consumerId) * added safeguard so that a validator cannot opt-in to two different chains with the same chain id * renamed some chainIDs to consumerIds * updated based on comments Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> * fixed integration tests * rebased to pick up the removal of legacy proposals (#2130) and re-introduced old messages so that existing proposals can deserialize * changes messages to only have MsgCreateConsumer and MsgUpdateConsumer and modified protos so that we are backward-compatible * cleaned up slightly a few things (mostly committing & pushing) so people can pick up the latest changes * fixed the CreateConsumer and UpdateConsumer logic and made most of the fields optional * fixed hooks and the code around proposalId to consumerId * feat: extend consumer validator query to return commission rate (backport #2162) (#2165) * adapt #2162 changes for permissionless ICS * nits --------- Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * renamed some chainIds to consumerIds * took into account comments and also added safeguard to reject new proposals that still use deprecated messages (e.g., MsgConsumerAddition, etc.) * Update x/ccv/provider/types/msg.go Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> * removed double-gas charge on MsgCreateConsumer and imroved the logic of MsgUpdateConsumer * added PopulateMinimumPowerInTopN tested * took into account comments (using protos for marshalling string slice, fixed issues in the UpdateConsumer logic, added extra check to abort spurious proposals) * feat: add fields to consumer validators query (#2167) * extend consumer validators query * nit * nits * fix msg order * deprecate power for consumer_power * modified the way we verify the new owner address, as well as nit refactoring on the ConsumerIds * fixed some rebase issues and changed a proto to be backward-compatible --------- Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * fixed bug on removing previous spawn time & added tests * added some additional tests * added tests on the hooks * removed check that spawn time is in the future * feat: refactor consumer validator set computation (#2175) * add UT * nits * address comments * Update x/ccv/provider/keeper/partial_set_security.go Co-authored-by: insumity <karolos@informal.systems> * fix tests --------- Co-authored-by: insumity <karolos@informal.systems> * add UT * nits * nits * revert legacy prop funcs * fix UT --------- Co-authored-by: insumity <karolos@informal.systems> Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * feat: extend consumer chains query (#2172) * feat: first iteration on Permissionless ICS (#2117) * (partially) renamed chain ids to consumer ids * renamed proposal messages * removed global slash entry * fixed unit tests * added new messages * introduced new state * added functionality for the register and initialize messages * renamed (partially) chainIds to consumerIds * set consumerId to chainId association during registration * added extra check in the initialization so unknokwn, launched, or stopped chains cannot re-initialize * added initial work on traversing initialized chains that are to-be-launched * fixed rebase issues after bringing the VSCMaturedPackets work in * made it so we traverse initialization records instead of addition proposals (+ additional changes so the unit tests pass) * renamed more chainIDs to consumerIds * removed ClientIdToChainId state because chainId already resides on the registration record * nit fixes in go docs * removed MsgConsumerAddition * added CLI commands for new messages * removed consumer modification proposal * removed (partially) consumer removal proposal * rebased to pick up the inactive-validators work (PR #2079) * introduced consumerId in the equivocation messages (and a useful query for Hermes to get the consumerId) * added safeguard so that a validator cannot opt-in to two different chains with the same chain id * renamed some chainIDs to consumerIds * updated based on comments Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> * fixed integration tests * rebased to pick up the removal of legacy proposals (#2130) and re-introduced old messages so that existing proposals can deserialize * changes messages to only have MsgCreateConsumer and MsgUpdateConsumer and modified protos so that we are backward-compatible * cleaned up slightly a few things (mostly committing & pushing) so people can pick up the latest changes * fixed the CreateConsumer and UpdateConsumer logic and made most of the fields optional * fixed hooks and the code around proposalId to consumerId * feat: extend consumer validator query to return commission rate (backport #2162) (#2165) * adapt #2162 changes for permissionless ICS * nits --------- Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * renamed some chainIds to consumerIds * took into account comments and also added safeguard to reject new proposals that still use deprecated messages (e.g., MsgConsumerAddition, etc.) * Update x/ccv/provider/types/msg.go Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> * removed double-gas charge on MsgCreateConsumer and imroved the logic of MsgUpdateConsumer * added PopulateMinimumPowerInTopN tested * took into account comments (using protos for marshalling string slice, fixed issues in the UpdateConsumer logic, added extra check to abort spurious proposals) * feat: add fields to consumer validators query (#2167) * extend consumer validators query * nit * nits * fix msg order * deprecate power for consumer_power * modified the way we verify the new owner address, as well as nit refactoring on the ConsumerIds * fixed some rebase issues and changed a proto to be backward-compatible --------- Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * add phase + metadata * first logic draft * add filter * reformat test * nits * nit * address comments * update tests * nits * update logic * update CLI * revert unwanted changes * remove filter field * nit * fix bad int conversion * update cli --------- Co-authored-by: insumity <karolos@informal.systems> Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> * feat!: migrations for permissionless (#2174) * migrations for permissionless -- wip * feat!: migrations for permissionless (#2177) * initial commit * took into account comments * removed deprecated queries * fix error due to rebase * remove fields from provider genesis * remove commented code --------- Co-authored-by: insumity <karolos@informal.systems> * feat: update `consumer_chains` REST endpoint (#2188) update consumer chains REST endpoint * fix: permissionless added event + cli changes (#2185) * add consumer registration event * fix some queries, CLI and permissionless logic * cli changes * addressed review comments * fix!: several permissionless changes (#2189) * minor fixes in tests * replace IsConsumerProposedOrRegistered with IsConsumerActive * remove param from createStakingValidator * handle error in GetTopN * remove aux methods for getting PowerShapingParameters * remove default PowerShapingParameters * fix tests * handle error messages * fix: fix consumer chain query CLI (#2190) * fix consumer chains query cli * add consumer-chain cli command * fix consumer chains cli bug * fix!: fix store.set nil values (#2193) fix store.set nil values * fix!: msgs validation (#2195) * add testing for ValidateStringField * ValidateInitializationParameters * ValidatePowerShapingParameters * validate NewOwnerAddress * apply review suggestions * fix: fix queries' API REST endpoint path (#2196) * clean uw files * doc * Revert "doc" This reverts commit efc2718. * Revert "clean uw files" This reverts commit 5773cf3. * remove deprecated REST endpoints * update REST endpoint * fix broken UT by previous PR (#2195) * fix: permissionless query (#2191) * fix implementation * addressed comments * cleanup * keep metadata and powershaping data for stopped consumer chains * addressed comments * adapt unit-tests * refactor: mostly refactors (#2194) * handle errors * set the reverse index in SetConsumerClientId * rename method * create consumer_lifecycle.go with launch/stop logic * move methods around * move methods around * handle ChangeRewardDenoms * add TODOs * remove proposal_test * remove todos * apply review suggestions * apply review suggestions * fix UTs * fix!: provider msg validation (#2197) * validate MsgAssignConsumerKey * validate MsgSubmitConsumerMisbehaviour and MsgSubmitConsumerDoubleVoting * move RemoveConsumer * refactor: rearrange messages in msg.go * validate MsgOptIn and MsgOptOut * validate MsgSetConsumerCommissionRate * remove ValidateBasic for deprecated msgs * remove deprecated GetSigners * remove deprecated GetSignBytes * remove deprecated govv1beta1 validation * remove Route and Type * Update x/ccv/provider/keeper/msg_server.go Co-authored-by: insumity <karolos@informal.systems> * apply review suggestions * apply review suggestions --------- Co-authored-by: insumity <karolos@informal.systems> * test: fix e2e tests + PSS e2e tests use permissionless (#2192) Fix e2e tests + make PSS e2e tests use Permissionless ICS * refactor: power shaping (#2207) * update allowlist and denylist power shaping update * fix tests * refactor: move power shaping params to power_shaping.go * rename record to parameters * fix merge conflicts * feat: fix queries (#2209) * remove chain ID from all-pairs-valconsensus-address * add consumer_id to Chain * add GetAllLaunchedConsumerIds * fix UT * apply review suggestions * fix!: remove `stopTime` from `MsgRemoveConsumer` (#2208) * init commit * renamed stop time to removal time * fix errors in tests * add GetAllConsumerWithIBCClients * use GetAllConsumerWithIBCClients for efficiency * fix UTs * remove GetAllLaunchedConsumerIds as not needed * typo in GetAllConsumersWithIBCClients * improve comment * Apply suggestions from code review Co-authored-by: Marius Poke <marius.poke@posteo.de> * took into account comments --------- Co-authored-by: mpoke <marius.poke@posteo.de> * feat: remove provider migration for CV 3 to 6 (#2211) * remove migrations to CV < 7 * add changelog entry * fix version name * fix!: fix migration issue (#2214) * init commit * took into account comments * fix!: register `ConsumerModificationProposal` and others (#2215) init commit * fix: fix misbehaviour and equivocation evidence CLI commands (#2213) * silent errors in MBT driver * fix misbehaviour and double-signing CLI cmds + few nits * revert changes * nit: remove type prefix from consumer phase enum (#2220) remove type prefix in consumer phase enum * refactor: stop using signer field in messages (#2219) * remove error check * stop using signer field in messages * expand TODO comment * replace signer with submitter * fix UTs * chore: update TX CLI help (#2222) * update CLI help * apply review suggestions * feat!: add events for provider messages (#2221) * add events for CreateConsumer * add events for UpdateConsumer * add events for RemoveConsumer * add events for the rest of the messages * apply review suggestions * Id instead of ID * fix events * fix UT * remove duplicate AttributeSubmitterAddress * fix: fix queries' API endpoints (#2226) * endpoint fixes * Update proto/interchain_security/ccv/provider/v1/query.proto Co-authored-by: MSalopek <matija.salopek994@gmail.com> * Update proto/interchain_security/ccv/provider/v1/query.proto Co-authored-by: MSalopek <matija.salopek994@gmail.com> * generate proto --------- Co-authored-by: MSalopek <matija.salopek994@gmail.com> * fix!: replace CanLaunch with InitializeConsumer (#2224) replace CanLaunch with InitializeConsumer * fix: improve commands' description (#2227) init commit --------- Co-authored-by: insumity <karolos@informal.systems> Co-authored-by: bernd-m <43466467+bermuell@users.noreply.github.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> Co-authored-by: kirdatatjana <116630536+kirdatatjana@users.noreply.github.com> Co-authored-by: MSalopek <matija.salopek994@gmail.com>
Description
Adapted e2e tests to current permissionless changes
current status:
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
New Features
Bug Fixes