feat!: Customizable Slashing and Jailing #2403

stana-miric · 2024-11-15T16:42:32Z

This PR introduces the ability for each consumer chain to set custom infraction parameters for slashing and jailing (double signing and downtime). The MsgCreateConsumer and MsgUpdateConsumer messages have been extended to support this functionality.

MsgCreateConsumer:
If infraction parameters are not provided in the message, the values will default to those from the provider and will be stored per consumer chain id.

MsgUpdateConsumer:
Pre-launch phase: Parameters are updated immediately.
Post-launch phase: Parameters are added to a queue, and the update will take effect after the unbonding period expires.

Queue Logic:
If parameters for the consumer already exist in the queue:
If the new parameters match the current infraction param for that consumer, the existing queued entry is removed to cancel the update. If the new parameters differ, the existing queued entry is replaced with the new parameters.
If parameters do not exist in the queue and the new parameters differ from the current ones, a new entry is added to schedule the update.

The unbonding period expiration is checked in the BeginBlocker, leveraging the existing time queue infrastructure.
When a consumer chain is removed, related queue records will also be deleted, but the infraction parameters will remain( following the same logic applied for power parameters). Additionally, a migration script has been implemented to set default infraction parameters for all currently active consumer chains.

x/ccv/provider/module.go

mpoke

Partial review. See comments below.

.changelog/unreleased/features/provider/2403-customizable-slashing-jailing.md

.changelog/unreleased/state-breaking/2403-customizable-slashing-jailing.md

docs/docs/features/slashing.md

proto/interchain_security/ccv/provider/v1/provider.proto

x/ccv/provider/migrations/v9/migrations.go

mpoke

Approve the general logic. Great work @stana-miric. See my comments below.

Please add also interchain tests to ensure that updating the infraction params works.

x/ccv/provider/keeper/msg_server.go

x/ccv/provider/keeper/relay.go

This reverts commit f1e7c6a.

stana-miric · 2024-11-27T13:27:57Z

Approve the general logic. Great work @stana-miric. See my comments below.

Please add also interchain tests to ensure that updating the infraction params works.

Cannot add interchain tests at this point cuz its another module and I can import the right version of ics in ics interchain test only after this pr is merged. I've added test that tests: adding infraction parameters through MsgCreateConsumer and MsgUpdateConsumer, also added tests that test queuing and overriding existing queued item as well as cancelling update, and tests that confirms that when jailing and lashing is called for misbehavior on consumer chain that parameters for that specific consumer is used.
I was thisnking to test this with interchain with the consumer suite that we're currently working on and will use those custon infraction parameters for consume chains.

mpoke · 2024-11-27T17:33:40Z

Approve the general logic. Great work @stana-miric. See my comments below.
Please add also interchain tests to ensure that updating the infraction params works.

Cannot add interchain tests at this point cuz its another module and I can import the right version of ics in ics interchain test only after this pr is merged.

Sounds good.

I've added test that tests: adding infraction parameters through MsgCreateConsumer and MsgUpdateConsumer, also added tests that test queuing and overriding existing queued item as well as cancelling update, and tests that confirms that when jailing and lashing is called for misbehavior on consumer chain that parameters for that specific consumer is used.

Great

I was thisnking to test this with interchain with the consumer suite that we're currently working on and will use those custon infraction parameters for consume chains.

Not sure we need to consumer side. That needed in general to test infractions (downtime and double signing). This feature just enables consumer chain owners to change the params. So the e2e test for it should just involve the provider API -- create consumer and update consumer. For example, can we use interchain tests to verify that an update will take the unbonding period? without waiting for 3 weeks :)

mpoke

Great work @stana-miric

stana-miric · 2024-11-29T12:29:28Z

Approve the general logic. Great work @stana-miric. See my comments below.
Please add also interchain tests to ensure that updating the infraction params works.

Cannot add interchain tests at this point cuz its another module and I can import the right version of ics in ics interchain test only after this pr is merged.

Sounds good.

I've added test that tests: adding infraction parameters through MsgCreateConsumer and MsgUpdateConsumer, also added tests that test queuing and overriding existing queued item as well as cancelling update, and tests that confirms that when jailing and lashing is called for misbehavior on consumer chain that parameters for that specific consumer is used.

Great

I was thisnking to test this with interchain with the consumer suite that we're currently working on and will use those custon infraction parameters for consume chains.

Not sure we need to consumer side. That needed in general to test infractions (downtime and double signing). This feature just enables consumer chain owners to change the params. So the e2e test for it should just involve the provider API -- create consumer and update consumer. For example, can we use interchain tests to verify that an update will take the unbonding period? without waiting for 3 weeks :)

Interchain tests are now implemented here. Once this pr is merged, pr with tests can be updated(ics version in go.mod, and image version) and reviewed

MSalopek

LGTM.

I still need to do an deep dive into the tests. The logic actually seems straightforward - any time a slash/jail operation needs to take place we first consult the consumer parameters and apply them instead of using the sdk defaults.

MSalopek · 2024-11-29T12:55:30Z

UPGRADING.md


 ## Unreleased

+### Provider


Can we add some context here? Just a couple sentences would go a long way.

fixed a068fcb

MSalopek · 2024-11-29T12:59:30Z

x/ccv/provider/client/cli/tx.go

@@ -257,6 +257,16 @@ where create_consumer.json has the following structure:
    "allow_inactive_vals": false,
    "prioritylist": ["cosmosvalcons..."]
  },
+  "infraction_parameters": {


Nit: fix spacing.

fixed a068fcb

MSalopek · 2024-11-29T13:00:46Z

proto/interchain_security/ccv/provider/v1/tx.proto

@@ -363,6 +363,9 @@ message MsgCreateConsumer {

  // allowlisted reward denoms of the consumer
  AllowlistedRewardDenoms allowlisted_reward_denoms = 6;
+
+  // infraction parameters for slashing and jailing
+  InfractionParameters infraction_parameters = 7;


Is this optional when the Tx is submitted via CLI/RPC?

It seems very annoying to think about such details during MsgCreateConsumer.

yes, its optional. if its not set, default values(same as the provider values) will be set.

stana-miric self-assigned this Nov 15, 2024

github-actions bot added C:x/provider Assigned automatically by the PR labeler C:x/types Assigned automatically by the PR labeler labels Nov 15, 2024

github-advanced-security bot found potential problems Nov 15, 2024

View reviewed changes

x/ccv/provider/module.go Dismissed Show dismissed Hide dismissed

Customizable Slashing and Jailing

9fa5ec3

stana-miric force-pushed the smiric/custom-slashing-and-jailing branch from c848f9d to 9fa5ec3 Compare November 15, 2024 17:31

stana-miric added 2 commits November 21, 2024 17:42

additional tests

973dbee

relay test fixes

99d4eea

github-actions bot added the C:Testing Assigned automatically by the PR labeler label Nov 22, 2024

stana-miric added 4 commits November 25, 2024 08:57

integration tests fix

86fc22c

migrations

f1e7c6a

slash with right params

6d9f644

comment fix

544331e

stana-miric force-pushed the smiric/custom-slashing-and-jailing branch from 76e01b7 to 544331e Compare November 26, 2024 12:42

docs

e9f7021

github-actions bot added the C:Docs Assigned automatically by the PR labeler label Nov 26, 2024

stana-miric marked this pull request as ready for review November 26, 2024 14:35

stana-miric requested a review from a team as a code owner November 26, 2024 14:35

mpoke reviewed Nov 26, 2024

View reviewed changes

x/ccv/provider/keeper/msg_server.go Outdated Show resolved Hide resolved

x/ccv/provider/keeper/relay.go Outdated Show resolved Hide resolved

stana-miric added 2 commits November 27, 2024 12:27

Revert "migrations"

1288a3f

This reverts commit f1e7c6a.

cr fix

69ec878

updrade.md update

2b0eb15

stana-miric force-pushed the smiric/custom-slashing-and-jailing branch from 3f7df35 to 2b0eb15 Compare November 28, 2024 11:38

stana-miric mentioned this pull request Nov 28, 2024

feat!: Add upgrade handler for ics consumer infraction params cosmos/gaia#3430

Draft

mpoke approved these changes Nov 29, 2024

View reviewed changes

stana-miric mentioned this pull request Nov 29, 2024

ci: Added infraction tests to provider suite #2416

Draft

MSalopek approved these changes Nov 29, 2024

View reviewed changes

cr fix

a068fcb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Customizable Slashing and Jailing #2403

feat!: Customizable Slashing and Jailing #2403

stana-miric commented Nov 15, 2024 •

edited by mpoke

Loading

mpoke left a comment

mpoke left a comment

stana-miric commented Nov 27, 2024

mpoke commented Nov 27, 2024

mpoke left a comment

stana-miric commented Nov 29, 2024

MSalopek left a comment

MSalopek Nov 29, 2024

stana-miric Dec 2, 2024

MSalopek Nov 29, 2024

stana-miric Dec 2, 2024 •

edited

Loading

MSalopek Nov 29, 2024

stana-miric Dec 2, 2024

feat!: Customizable Slashing and Jailing #2403

Are you sure you want to change the base?

feat!: Customizable Slashing and Jailing #2403

Conversation

stana-miric commented Nov 15, 2024 • edited by mpoke Loading

mpoke left a comment

Choose a reason for hiding this comment

mpoke left a comment

Choose a reason for hiding this comment

stana-miric commented Nov 27, 2024

mpoke commented Nov 27, 2024

mpoke left a comment

Choose a reason for hiding this comment

stana-miric commented Nov 29, 2024

MSalopek left a comment

Choose a reason for hiding this comment

MSalopek Nov 29, 2024

Choose a reason for hiding this comment

stana-miric Dec 2, 2024

Choose a reason for hiding this comment

MSalopek Nov 29, 2024

Choose a reason for hiding this comment

stana-miric Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

MSalopek Nov 29, 2024

Choose a reason for hiding this comment

stana-miric Dec 2, 2024

Choose a reason for hiding this comment

stana-miric commented Nov 15, 2024 •

edited by mpoke

Loading

stana-miric Dec 2, 2024 •

edited

Loading