Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

Provider-driven governance #416

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

dusan-maksimovic
Copy link
Collaborator

This PR allows governance proposals regarding consumer chains to be submitted and voted on the provider chain. If the proposal passes, it will be submitted to the consumer chain and applied immediately in first EndBlock(). To achieve this, implementation leverages Interchain Accounts (ICA) and Admin modules. The main changes are:

  • New governance type (ConsumerGovernanceProposal) is introduced on the provider chain. It allows arbitrary governance proposal (e.g. SW upgrade, parameters change, ...) for consumer chains to be submitted on the provider chain.
  • Consumer genesis ccv section is extended with ProviderGovernanceAddress field. This field is later used by admin module to register provider's governance module as admin. This will be done automatically in admin modules's EndBlock() so no additional user action is required.
  • ICA controller and host sub-modules are added to provider and consumer chains, respectively. During the provider-consumer channel handshake, provider's governance module registers its interchain account on the consumer chain. This account will be used to send passed proposals to the consumer chain.
  • ICA Authentication module is required by the ICA Controller submodule and it is equivalent to the one on the Cosmos Hub, except that Msg URLs are changed.
  • Admin module source code is added to ICS repository and modified to fit our needs. There are two important changes. First, in EndBlock() provider's governance module interchain account address is added as admin, after it is registered on the consumer chain. Second thing is introduction of whitelists that allow projects to specify which type of proposals are allowed to be submitted to the admin module. Whitelists are separated for provider's governance module admin and all other admins that may be set from the consumer chain side. This allows specific projects (e.g. Neutron) to have, for example, a wasm contracts set as admins with their own whitelist to submit proposals from the consumer chain, while provider's gov module can have a different whitelist.
  • To make all this work, in consumer chain genesis, interchainaccounts->host_genesis_state->params->allow_messages we must add at least "/interchain_security.ccv.adminmodule.v1.MsgSubmitProposal" to allow provider to send proposals to consumer chain through ICA module.

@okwme we would be thankful if you could take a look at how we wired up everything.
cc @stana-ethernal as co-author

dusan-maksimovic and others added 5 commits October 25, 2022 09:29
- Interchain accounts (ICA) module enabled on provider and consumer chains. This allows provider's governance module to register its ICA account and send txs to the consumer chain.
- Added authentication module that is required by the ICA controller submodule on the provider chain.
- Added adminmodule on the consumer side to enable proposals that passed on provider side to be executed immediately on the consumer side.
- Implemented governance for consumer chains on the provider chain + execution of passed proposals on the consumer side.

Co-authored by:
Stana Miric
Dusan Maksimovic
… gov address included into consumer genesis

Co-authored by:
Stana Miric
Dusan Maksimovic
Co-authored by:
Stana Miric
Dusan Maksimovic
// this allows any type of proposal to be submitted to the admin module (everything is whitelisted)
// projects will implement their functions to define what is allowed for provider and consumer admins
func(ccvgovtypes.Content) bool { return true },
func(ccvgovtypes.Content) bool { return true },
Copy link
Contributor

Choose a reason for hiding this comment

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

this is nice. Maybe an example of a switch statement with various proposal types to show how you'd include or exclude specific ones would be nice to have?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we already have similar function for democracy consumer chain that implements whitelisting based on proposal type. We will change this comment to make reference to that function.

@@ -382,6 +432,24 @@ func New(

app.EvidenceKeeper = *evidenceKeeper

adminRouter := govtypes.NewRouter()
adminRouter.AddRoute(govtypes.RouterKey, govtypes.ProposalHandler).
Copy link
Contributor

Choose a reason for hiding this comment

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

would this cause problems if the consumer chain also wanted to have the democracy module included? (aka the normal gov module that votes only on specific param changes?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would not be a problem since the message submitted to the admin module will be interchain_security.ccv.adminmodule.v1.MsgSubmitProposal(and will go in active proposals queue of admin module), while the message submitted to the governance module would be cosmos.gov.v1beta1.MsgSubmitProposal(and would be added to active/inactive queue of gov module). In endblocker of both module proposals will be executed and handler for the Content of the message will be found based on those routes which can be the same for those 2 types of different msgs because the Content can be the same e.g. ParameterChangeProposal but we will use the same handler to execute the msg content.
In consumer-democracy app we have both admin and gov-democracy modules.

@jtremback
Copy link
Contributor

Why are adminmodule and icamauth pasted into our repo? Shouldn't these just be imported from their own repos? If there are modifications needed to these modules, can't you make a fork?

@MSalopek
Copy link
Contributor

MSalopek commented Nov 1, 2022

I was wondering if we could split this into at least 2 smaller PRs?

The reasoning behind the split would be:

  1. it adds parts (or whole?) of https://github.com/cosmos/composer with ICS specific changes
  • having that as separate PR would make it easier to understand and test, and figure out which changes are ICS specific
  • there's some changes in genesis
  1. it adds new Tx handlers
  • it's easier to reason about those when they are separated -> there's a bunch of boilerplate with adding Txs, so it's easy to overlook important bits
  1. it adds interchain account interactions
  • we haven't had that before so we'd need to add test coverage

@jtremback jtremback marked this pull request as draft November 1, 2022 15:21
@jtremback
Copy link
Contributor

@stana-ethernal @dusan-ethernal Converting this to draft for now, will review after the testnet starts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provider-driven governance
5 participants