-
Notifications
You must be signed in to change notification settings - Fork 137
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.
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
feat!: added E2E test and docs for ConsumerModificationProposal #1949
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -129,3 +129,9 @@ Yes, the consumer chain will halt with an ERR CONSENSUS FAILURE error after the | |||||||||||||||||||||
## Can validators set a commission rate for chains they have not opted in to? | ||||||||||||||||||||||
Yes, and this is useful for validators that are not in the top N% of the provider chain, but might move into the top N% in the future. | ||||||||||||||||||||||
By setting the commission rate ahead of time, they can make sure that they immediately have a commission rate of their choosing as soon as they are in the top N%. | ||||||||||||||||||||||
|
||||||||||||||||||||||
## Can a consumer chain modify its power shaping parameters? | ||||||||||||||||||||||
Yes, by issuing a [`ConsumerModificationProposal`](./features/proposals.md#consumermodificationproposal). | ||||||||||||||||||||||
|
||||||||||||||||||||||
## Can a Top N consumer chain become Opt-In or vice versa? | ||||||||||||||||||||||
Yes, by issuing a [`ConsumerModificationProposal`](./features/proposals.md#consumermodificationproposal). | ||||||||||||||||||||||
Comment on lines
+133
to
+137
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add FAQ entries about modifying power shaping parameters and transitioning between Top N and Opt-In chains using Consider adding blank lines around headings for better markdown formatting. - ## Can a Top N consumer chain become Opt-In or vice versa?
+
+ ## Can a Top N consumer chain become Opt-In or vice versa?
+ Committable suggestion
Suggested change
ToolsMarkdownlint
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -12,7 +12,7 @@ The experience gained in the testnet will prepare you for validating interchain | |||||
:::tip | ||||||
Provider and consumer chain represent distinct networks and infrastructures operated by the same validator set. | ||||||
|
||||||
For general information about running cosmos-sdk based chains check out the [validator basics](https://hub.cosmos.network/validators/validator-setup) and [Running a Node section](https://docs.cosmos.network/main/run-node/run-node) of Cosmos SDK docs | ||||||
For general information about running cosmos-sdk based chains check out the [validator basics](https://hub.cosmos.network/main/validators/validator-setup) and [Running a Node section](https://docs.cosmos.network/main/run-node/run-node) of Cosmos SDK docs | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because this PR was touching docs as well, fixed those stale links. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update URLs to point to the correct resources. Consider adding a comma after "chains" for better readability. - For general information about running cosmos-sdk based chains check out the [validator basics](https://hub.cosmos.network/main/validators/validator-setup) and [Running a Node section](https://docs.cosmos.network/main/run-node/run-node) of Cosmos SDK docs
+ For general information about running cosmos-sdk based chains, check out the [validator basics](https://hub.cosmos.network/main/validators/validator-setup) and [Running a Node section](https://docs.cosmos.network/main/run-node/run-node) of Cosmos SDK docs Committable suggestion
Suggested change
ToolsLanguageTool
|
||||||
::: | ||||||
|
||||||
## Joining the provider chain | ||||||
|
@@ -79,7 +79,7 @@ gaiad tx staking create-validator \ | |||||
``` | ||||||
|
||||||
:::tip | ||||||
Check this [guide](https://hub.cosmos.network/validators/validator-setup#edit-validator-description) to edit your validator. | ||||||
Check this [guide](https://hub.cosmos.network/main/validators/validator-setup#edit-validator-description) to edit your validator. | ||||||
::: | ||||||
|
||||||
After this step, your validator is created and you can start your node and catch up to the rest of the network. It is recommended that you use `statesync` to catch up to the rest of the network. | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,7 +85,7 @@ At present, the consumer chain can report evidence about downtime infractions to | |
:::info | ||
Causing a downtime infraction on any consumer chain will not incur a slash penalty. Instead, the offending validator will be jailed on the provider chain and consequently on all consumer chains. | ||
|
||
To unjail, the validator must wait for the jailing period to elapse on the provider chain and [submit an unjail transaction](https://hub.cosmos.network/validators/validator-setup#unjail-validator) on the provider chain. After unjailing on the provider, the validator will be unjailed on all consumer chains. | ||
To unjail, the validator must wait for the jailing period to elapse on the provider chain and [submit an unjail transaction](https://hub.cosmos.network/main/validators/validator-setup#unjail-validator) on the provider chain. After unjailing on the provider, the validator will be unjailed on all consumer chains. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because this PR was touching docs as well, fixed those stale links. |
||
|
||
More information is available in [Downtime Slashing documentation](../features/slashing.md#downtime-infractions) | ||
::: | ||
|
@@ -99,7 +99,7 @@ Validators can use different consensus keys on the provider and each of the cons | |
For more information check out the [Key assignment overview and guide](../features/key-assignment.md) | ||
|
||
## References: | ||
- [Cosmos Hub Validators FAQ](https://hub.cosmos.network/validators/validator-faq) | ||
- [Cosmos Hub Running a validator](https://hub.cosmos.network/validators/validator-setup) | ||
- [Cosmos Hub Validators FAQ](https://hub.cosmos.network/main/validators/validator-faq) | ||
- [Cosmos Hub Running a validator](https://hub.cosmos.network/main/validators/validator-setup) | ||
- [Startup Sequence](https://github.com/cosmos/testnets/blob/master/interchain-security/CONSUMER_LAUNCH_GUIDE.md#chain-launch) | ||
- [Submit Unjailing Transaction](https://hub.cosmos.network/validators/validator-setup#unjail-validator) | ||
- [Submit Unjailing Transaction](https://hub.cosmos.network/main/validators/validator-setup#unjail-validator) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -399,6 +399,79 @@ func (tr TestConfig) submitConsumerRemovalProposal( | |
tr.waitBlocks(ChainID("provi"), 2, 20*time.Second) | ||
} | ||
|
||
type SubmitConsumerModificationProposalAction struct { | ||
Chain ChainID | ||
From ValidatorID | ||
Deposit uint | ||
ConsumerChain ChainID | ||
TopN uint32 | ||
ValidatorsPowerCap uint32 | ||
ValidatorSetCap uint32 | ||
Allowlist []string | ||
Denylist []string | ||
} | ||
|
||
func (tr TestConfig) submitConsumerModificationProposal( | ||
action SubmitConsumerModificationProposalAction, | ||
target ExecutionTarget, | ||
verbose bool, | ||
) { | ||
prop := client.ConsumerModificationProposalJSON{ | ||
Title: "Propose the modification of the PSS parameters of a chain", | ||
Summary: "summary of a modification proposal", | ||
ChainId: string(tr.chainConfigs[action.ConsumerChain].ChainId), | ||
Deposit: fmt.Sprint(action.Deposit) + `stake`, | ||
TopN: action.TopN, | ||
ValidatorsPowerCap: action.ValidatorsPowerCap, | ||
ValidatorSetCap: action.ValidatorSetCap, | ||
Allowlist: action.Allowlist, | ||
Denylist: action.Denylist, | ||
} | ||
|
||
bz, err := json.Marshal(prop) | ||
if err != nil { | ||
log.Fatal(err) | ||
} | ||
|
||
jsonStr := string(bz) | ||
if strings.Contains(jsonStr, "'") { | ||
log.Fatal("prop json contains single quote") | ||
} | ||
|
||
//#nosec G204 -- bypass unsafe quoting warning (no production code) | ||
bz, err = target.ExecCommand( | ||
"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, "/temp-proposal.json"), | ||
|
||
).CombinedOutput() | ||
if err != nil { | ||
log.Fatal(err, "\n", string(bz)) | ||
} | ||
|
||
// CONSUMER MODIFICATION PROPOSAL | ||
cmd := target.ExecCommand( | ||
tr.chainConfigs[action.Chain].BinaryName, | ||
"tx", "gov", "submit-legacy-proposal", "consumer-modification", "/temp-proposal.json", | ||
`--from`, `validator`+fmt.Sprint(action.From), | ||
`--chain-id`, string(tr.chainConfigs[action.Chain].ChainId), | ||
`--home`, tr.getValidatorHome(action.Chain, action.From), | ||
`--gas`, `900000`, | ||
`--node`, tr.getValidatorNode(action.Chain, action.From), | ||
`--keyring-backend`, `test`, | ||
`-y`, | ||
) | ||
if verbose { | ||
log.Println("submitConsumerModificationProposal cmd: ", cmd.String()) | ||
} | ||
|
||
bz, err = cmd.CombinedOutput() | ||
|
||
if err != nil { | ||
log.Fatal(err, "\n", string(bz)) | ||
} | ||
|
||
// wait for inclusion in a block -> '--broadcast-mode block' is deprecated | ||
tr.waitBlocks(ChainID("provi"), 2, 10*time.Second) | ||
} | ||
Comment on lines
+402
to
+473
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review the implementation of The function
- log.Fatal(err, "\n", string(bz))
+ return err
Toolsgolangci-lint
GitHub Check: CodeQL
|
||
|
||
type SubmitParamChangeLegacyProposalAction struct { | ||
Chain ChainID | ||
From ValidatorID | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -192,6 +192,12 @@ var stepChoices = map[string]StepChoice{ | |
description: "test partial set security for an Opt-In chain that has a validator denylisted", | ||
testConfig: DefaultTestCfg, | ||
}, | ||
"partial-set-security-modification-proposal": { | ||
name: "partial-set-security-modification-proposal", | ||
steps: stepsModifyChain(), | ||
description: "test partial set security parameters can be changed through a modification proposal", | ||
testConfig: DefaultTestCfg, | ||
}, | ||
Comment on lines
+195
to
+200
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure comprehensive test coverage for The new test case should thoroughly test all aspects of the security modifications it aims to cover. Consider adding more detailed steps or checks to ensure comprehensive coverage. |
||
} | ||
|
||
func getTestCaseUsageString() string { | ||
|
@@ -280,6 +286,7 @@ func getTestCases(selectedPredefinedTests, selectedTestFiles TestSet, providerVe | |
"consumer-double-downtime", "partial-set-security-opt-in", "partial-set-security-top-n", | ||
"partial-set-security-validator-set-cap", "partial-set-security-validators-power-cap", | ||
"partial-set-security-validators-allowlisted", "partial-set-security-validators-denylisted", | ||
"partial-set-security-modification-proposal", | ||
} | ||
if includeMultiConsumer != nil && *includeMultiConsumer { | ||
selectedPredefinedTests = append(selectedPredefinedTests, "multiconsumer") | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -76,6 +76,14 @@ type ConsumerRemovalProposal struct { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (p ConsumerRemovalProposal) isProposal() {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type ConsumerModificationProposal struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Deposit uint | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Chain ChainID | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Status string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (p ConsumerModificationProposal) isProposal() {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type Rewards struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
IsRewarded map[ValidatorID]bool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// if true it will calculate if the validator/delegator is rewarded between 2 successive blocks, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -482,6 +490,22 @@ func (tr TestConfig) getProposal(chain ChainID, proposal uint) Proposal { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Chain: chain, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
StopTime: int(stopTime.Milliseconds()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
case "/interchain_security.ccv.provider.v1.ConsumerModificationProposal": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chainId := gjson.Get(string(bz), `messages.0.content.chain_id`).String() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var chain ChainID | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for i, conf := range tr.chainConfigs { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if string(conf.ChainId) == chainId { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain = i | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
break | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ConsumerModificationProposal{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Deposit: uint(deposit), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Status: status, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Chain: chain, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+493
to
+508
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Address potential non-determinism in map iteration. The iteration over - for i, conf := range tr.chainConfigs {
+ keys := make([]string, 0, len(tr.chainConfigs))
+ for k := range tr.chainConfigs {
+ keys = append(keys, k)
+ }
+ sort.Strings(keys) // Ensure the keys are sorted if order is important
+ for _, k := range keys {
+ conf := tr.chainConfigs[k]
if string(conf.ChainId) == chainId {
chain = i
break
}
} Committable suggestion
Suggested change
ToolsGitHub Check: CodeQL
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
case "/cosmos.params.v1beta1.ParameterChangeProposal": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ParamsProposal{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Deposit: uint(deposit), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comma after 'Otherwise' for grammatical correctness.
Committable suggestion
Tools
LanguageTool