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

ICS28: Enable the removal of a consumer chain from the provider #707

Merged
merged 93 commits into from
May 10, 2022

Conversation

mpoke
Copy link
Contributor

@mpoke mpoke commented Apr 7, 2022

Closes #651

Closes #669

  • handle timeouts on the consumer chain side (see TODOs here and here)

mpoke and others added 30 commits January 17, 2022 22:08
@mpoke mpoke requested review from josef-widder, jtremback and sainoe and removed request for cwgoes April 7, 2022 18:04
@mpoke mpoke self-assigned this Apr 7, 2022
@mpoke mpoke added the ready-for-review Pull requests which are ready for review. label Apr 7, 2022
- `spawnTime` is the time on the provider chain at which the consumer chain genesis is finalized and all validators are responsible to start their consumer chain validator node.
- `lockUnbondingOnTimeout` is a boolean value that indicates whether the funds corresponding to the outstanding unbonding operations are to be released in case of a timeout. In case `lockUnbondingOnTimeout == true`, a governance proposal to stop the timed out consumer chain would be necessary to release the locked funds.
```typescript
interface StopConsumerChainProposal {
Copy link
Member

Choose a reason for hiding this comment

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

This is really a signaling proposal correct? There's no enforcement involved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean by enforcement? If the proposal passes, then the consumer chain is removed once stopTime elapses. Validators can still run full nodes of the consumer chain without being penalized. So in that sense, it is indeed a signaling proposal. The only thing enforced is that validators are no longer slashed for not validating on the consumer chain. Should I make this clearer in the spec?

Copy link
Member

Choose a reason for hiding this comment

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

No one gets slashed. The child chain could effectively continue running as an independent chai

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but with a different set of validators. Or at least not validators got from Interchain Security. Most likely, this would entail the consumer chain stops and is restarted as an independent chain.

Base automatically changed from marius/702-ccv-inith to marius/ccv April 20, 2022 09:06
@mpoke
Copy link
Contributor Author

mpoke commented Apr 20, 2022

@AdityaSripal Regarding handling timeouts on the consumer chain side (see TODOs here and here), is there anything else that must be done besides shutting down the chain?

How is the provider chain notified that the consumer chain shut down? I guess that the provider either receives a MsgTimeoutOnClose message from the relayer proving that the consumer closed the CCV channel or waits until its own packets timeout, right?

Finally, how can the consumer chain move its CCV channel to CLOSED and shut down at the same time?

@AdityaSripal
Copy link
Member

So long as our side does ChanCloseInit, a relayer should be able to do TimeoutOnClose on the counterpary

@mpoke
Copy link
Contributor Author

mpoke commented Apr 20, 2022

So long as our side does ChanCloseInit, a relayer should be able to do TimeoutOnClose on the counterpary

If the consumer chain executes onTimeoutPacket(), the IBC logic will move the channel to CLOSED (e.g., here). This should be sufficient for the relayer to do TimeoutOnClose on the counterpary. However, when is the consumer chain shutting down? It cannot be in onTimeoutPacket(), since the channel will not chage to CLOSED. We could add the following lines to the BeginBlock() of the consumer CCV module

channel = provableStore.get(channelPath(ConsumerPortId, providerChannel))
abortSystemUnless(providerChannel == "" OR channel.state != CLOSED)

What do you say?

@AdityaSripal
Copy link
Member

I don't know if there's a way to shut down the consumer chain within the state machine itself. I suppose validators can simply shut down their nodes once the channel has closed (block gets committed). Though they still need to make their nodes available so relayers can query proofs of channel close

@mpoke
Copy link
Contributor Author

mpoke commented Apr 20, 2022

I don't know if there's a way to shut down the consumer chain within the state machine itself. I suppose validators can simply shut down their nodes once the channel has closed (block gets committed). Though they still need to make their nodes available so relayers can query proofs of channel close

This is important since as soon as the CCV channel is closed, the consumer chain is no longer secured, i.e., the validators can do whatever they want. We should be able to cleanly shut down (e.g., export the state and no longer accept TXs).

@mpoke
Copy link
Contributor Author

mpoke commented Apr 20, 2022

I tried to deal with shutting down the consumer chain in the commit ea6919b. @AdityaSripal Could you please have a quick look?

Copy link
Member

@AdityaSripal AdityaSripal left a comment

Choose a reason for hiding this comment

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

Nice work Marius!

chainId: string
initialHeight: Height
spawnTime: Timestamp
lockUnbondingOnTimeout: Bool
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this influences some of the properties in system_model_and_properties.md? For instance, some properties only hold in the case of lockUnbondingOnTimeout = TRUE. In the FALSE case, some misbehavior might not lead to a slashing on the provider chain, for instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'll look into it.

@mpoke
Copy link
Contributor Author

mpoke commented May 2, 2022

@josef-widder I added a note discussing the implications of calling StopConsumerChain with lockUnbonding set to false (see 3f24b4b).

@mpoke mpoke merged commit dfc0398 into marius/ccv May 10, 2022
@mpoke mpoke deleted the marius/651-ccv-remove-consumer branch May 10, 2022 07:43
mpoke added a commit that referenced this pull request Aug 9, 2022
* ICS 28 CCV draft w/ init and validator set update (#640)

* Create README.md

* Add files with CCV spec

* fix links to ICS 4

* fix links to ICS 7

* add ICS 28 to main README.md

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/technical_specification.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/README.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/README.md

Co-authored-by: Sergio Mena <[email protected]>

* minor, remove confusing phrasing

* child -> consumer; parent -> provider

* clarify which staking module

* extend staking assumptions, remove redundant inv, prove staking props and sys inv

* break long lines

* break long lines

* remove dependecies to Cosmos SDK

* changes in the security model

* specify multiple consumer chains

* channel init overview

* split Valid Blockchain assumption

* minor changes after discussion w/ Josef

* make the split of Valid Blockchain consistent

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Aditya <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Aditya <[email protected]>

* resolve conversations

Co-authored-by: Sergio Mena <[email protected]>
Co-authored-by: Aditya <[email protected]>

* merge CODEOWNERS from master

* ICS28: CCV draft w/ complete unbonding (#646)

* Create README.md

* Add files with CCV spec

* fix links to ICS 4

* fix links to ICS 7

* add ICS 28 to main README.md

* adding tech spec for unbonding delegations

* add context on unbonding operations

* add unbonding operation diagram

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/technical_specification.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/README.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/README.md

Co-authored-by: Sergio Mena <[email protected]>

* minor, remove confusing phrasing

* child -> consumer; parent -> provider

* clarify which staking module

* extend staking assumptions, remove redundant inv, prove staking props and sys inv

* modify staking hooks spec to cover other unbonding ops

* provider Staking module

* break long lines

* break long lines

* remove dependecies to Cosmos SDK

* changes in the security model

* specify multiple consumer chains

* channel init overview

* address issues #27 and #33 from cosmos/interchain-security repo

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Josef Widder <[email protected]>

Co-authored-by: Sergio Mena <[email protected]>
Co-authored-by: Josef Widder <[email protected]>

* ICS28: Extend consumer InitGenesis (#659)

* Create README.md

* Add files with CCV spec

* fix links to ICS 4

* fix links to ICS 7

* add ICS 28 to main README.md

* adding tech spec for unbonding delegations

* add context on unbonding operations

* add unbonding operation diagram

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/technical_specification.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/README.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/README.md

Co-authored-by: Sergio Mena <[email protected]>

* minor, remove confusing phrasing

* child -> consumer; parent -> provider

* clarify which staking module

* extend staking assumptions, remove redundant inv, prove staking props and sys inv

* modify staking hooks spec to cover other unbonding ops

* provider Staking module

* break long lines

* break long lines

* remove dependecies to Cosmos SDK

* changes in the security model

* specify multiple consumer chains

* channel init overview

* address issues #27 and #33 from cosmos/interchain-security repo

* extend consumer InitGenesis

* describe mechanism to disseminate genesis state

* remove ExportGenesis and restarted chains

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Josef Widder <[email protected]>

* validate channel IDs on provider genesis

Co-authored-by: Sergio Mena <[email protected]>
Co-authored-by: Josef Widder <[email protected]>

* ICS28: Consumer Initiated Slashing (aka Evidence) (#663)

* Create README.md

* Add files with CCV spec

* fix links to ICS 4

* fix links to ICS 7

* add ICS 28 to main README.md

* adding tech spec for unbonding delegations

* add context on unbonding operations

* add unbonding operation diagram

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/technical_specification.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/README.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/README.md

Co-authored-by: Sergio Mena <[email protected]>

* minor, remove confusing phrasing

* child -> consumer; parent -> provider

* clarify which staking module

* extend staking assumptions, remove redundant inv, prove staking props and sys inv

* modify staking hooks spec to cover other unbonding ops

* provider Staking module

* break long lines

* break long lines

* remove dependecies to Cosmos SDK

* changes in the security model

* specify multiple consumer chains

* channel init overview

* address issues #27 and #33 from cosmos/interchain-security repo

* extend consumer InitGenesis

* describe mechanism to disseminate genesis state

* describe mapping heights provider <> consumer

* remove ExportGenesis and restarted chains

* add overview of consumer initiated slashing

* add slashing invariant

* add assumptions needed by evidence

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Josef Widder <[email protected]>

* draft CCV props for slashing

* replace time w/ height; add HtoVSC and VSCtoH

* replace time with height in invariants and properties

* validate channel IDs on provider genesis

* prove Slashing Invariant

* enable mapping from consumer to provider heights

* technical spec for slashing

* minor changes

* fix links to tendermint spec

* clarify Staking vs Slashing modules

* replace VSC acks w/ VSCMaturedPackets

* fix some TODOs

* fix properties

* HtoVSC and VSCtoH from () to []

* fix infraction height and add intuition diagram

* keep ValidatorSet in consumer CCV module state

* add outstanding downtime flag and decouple from validatorSet

* fix issues pointed by Simon

* dealing with downtime slashing atomicity

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Aditya <[email protected]>

* addressing Aditya's comments

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Josef Widder <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Josef Widder <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Josef Widder <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/technical_specification.md

Co-authored-by: Josef Widder <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/technical_specification.md

Co-authored-by: Josef Widder <[email protected]>

* addressing Josef's comments

* link TODOs with issues

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Aditya <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Aditya <[email protected]>

* apply suggestions from code review

* cleanup outdated note on restarted consumer chains

Co-authored-by: Sergio Mena <[email protected]>
Co-authored-by: Josef Widder <[email protected]>
Co-authored-by: Aditya <[email protected]>

* ICS28: Remove CCV channel state (#678)

* Create README.md

* Add files with CCV spec

* fix links to ICS 4

* fix links to ICS 7

* add ICS 28 to main README.md

* adding tech spec for unbonding delegations

* add context on unbonding operations

* add unbonding operation diagram

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/technical_specification.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/README.md

Co-authored-by: Sergio Mena <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/README.md

Co-authored-by: Sergio Mena <[email protected]>

* minor, remove confusing phrasing

* child -> consumer; parent -> provider

* clarify which staking module

* extend staking assumptions, remove redundant inv, prove staking props and sys inv

* modify staking hooks spec to cover other unbonding ops

* provider Staking module

* break long lines

* break long lines

* remove dependecies to Cosmos SDK

* changes in the security model

* specify multiple consumer chains

* channel init overview

* address issues #27 and #33 from cosmos/interchain-security repo

* extend consumer InitGenesis

* describe mechanism to disseminate genesis state

* describe mapping heights provider <> consumer

* remove ExportGenesis and restarted chains

* add overview of consumer initiated slashing

* add slashing invariant

* add assumptions needed by evidence

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Josef Widder <[email protected]>

* draft CCV props for slashing

* replace time w/ height; add HtoVSC and VSCtoH

* replace time with height in invariants and properties

* validate channel IDs on provider genesis

* prove Slashing Invariant

* enable mapping from consumer to provider heights

* technical spec for slashing

* minor changes

* fix links to tendermint spec

* clarify Staking vs Slashing modules

* replace VSC acks w/ VSCMaturedPackets

* fix some TODOs

* fix properties

* HtoVSC and VSCtoH from () to []

* fix infraction height and add intuition diagram

* keep ValidatorSet in consumer CCV module state

* remove CCV channel status

* add outstanding downtime flag and decouple from validatorSet

* adressing Josef's comment

* fix issues pointed by Simon

* dealing with downtime slashing atomicity

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Aditya <[email protected]>

* addressing Aditya's comments

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Josef Widder <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Josef Widder <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/overview_and_basic_concepts.md

Co-authored-by: Josef Widder <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/technical_specification.md

Co-authored-by: Josef Widder <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/technical_specification.md

Co-authored-by: Josef Widder <[email protected]>

* addressing Josef's comments

* link TODOs with issues

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Aditya <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Aditya <[email protected]>

* apply suggestions from code review

* cleanup outdated note on restarted consumer chains

Co-authored-by: Sergio Mena <[email protected]>
Co-authored-by: Josef Widder <[email protected]>
Co-authored-by: Aditya <[email protected]>

* Update technical_specification.md

Fix typo: VSCtoH replaced with HtoVSC in SendSlashRequest

* ICS28: Replace "Initiator" w/ "Caller" and “Trigger Event” (#696)

* update init methods and ics26 methods

* updating ValSet Update methods

* updating Consumer Initiated Slashing methods

* ICS28: Handle pending proposals to spawn consumer chains (#697)

* handle pending proposals

* ICS28: Remove genesisHash from specification (#699)

* remove genesis hash

* remove details of genesis state dissemination

* ICS28: CCV Reward Distribution subprotocol (#704)

* add overview of reward distribution

* add CCVHandshakeMetadata and update channel handshake methods signatures

* initiate opening handshake for transfer channel

* add DistributeRewards() method

* address review comments

* add distribution invariant

* replace system invariants with system properties

Co-authored-by: Josef Widder <[email protected]>

* ICS28: Set initH in onChanOpenConfirm (#705)

* set initH in onChanOpenConfirm

* replace initH with initialHeights

* ICS28: Enable the removal of a consumer chain from the provider (#707)

* stopping a consumer chain

* deal with timeouts on the consumer side

* fix typo

* add note on how to shut down the consumer

* add note on safety implication of lockUnbondingOnTimeout

* ICS28: Remove BeforeUnbondingOpCompleted (#711)

* remove BeforeUnbondingOpCompleted hook

* cleanup method names

* ICS28: Update SlashPacketData (#728)

* update SlashPacket

* ICS28: Remove dependency on Tendermint and ABCI (#730)

* remove mention of V2

* remove ABCI dependency from overview; also small fixes

* remove dependencies from system model and README

* remove dependencies from tech spec

* call UnbondMaturePackets() earlier (#747)

* fix edge case enabled by aggregation (#746)

* Use `h < hp'` instead of `h <= hp'` in Bond-Based Consumer Voting Power (#749)

* replace slashRequests w/ downtimeSlashRequests (#745)

* ics28 update sendPacket (#756)

* ICS28: Restructure technical spec (#760)

* split technical spec into two files

* restructure methods and subprotocols

* add links to readme.md

* fix typo - initH replaced w/ initialHeights

* ICS28: Unbonding ops are put on hold even w/o consumer chains (#763)

* PutUnbondingOnHold only if consumer chains exist

* Use currentTimestamp() >= p.stopTime in StopConsumerChainProposalHandler (#769)

Tackle #768

* adding consumerUnbondingPeriod (#771)

* fix sendFungibleTokens signature

* add authors

* ICS28: Removing the only consumer chain (#770)

* remove unbonding op with no consumer chains

* add postcondition

* add conditions for CreateConsumerClient and StopConsumerChain (#775)

* update created date

* ICS28: Account for slashing in Bond-Based Consumer Voting Power property (#793)

* extend Bond-Based Consumer Voting Power w/ slashing

* fix Bond-Based Consumer Voting Power formula

* expand note for clarity

* add pUnbonding

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Josef Widder <[email protected]>

* clarify mathematical writeup of property

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Josef Widder <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Josef Widder <[email protected]>

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Josef Widder <[email protected]>

* make changes to sumUnbonding / sumSlash consistent

* add history

Co-authored-by: Josef Widder <[email protected]>

Co-authored-by: Sergio Mena <[email protected]>
Co-authored-by: Aditya <[email protected]>
Co-authored-by: Josef Widder <[email protected]>
Co-authored-by: Daniel T <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review Pull requests which are ready for review.
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants