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

remove all references to LockUnbondingOnTimeout #551

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions proto/interchain_security/ccv/provider/v1/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,15 @@ message ConsumerState {
string client_id = 3;
// InitalHeight defines the initial block height for the consumer chain
uint64 initial_height = 4;
// LockUnbondingOnTimeout defines whether the unbonding funds should be released for this
// chain in case of a IBC channel timeout
bool lock_unbonding_on_timeout = 5;
// ConsumerGenesis defines the initial consumer chain genesis states
interchain_security.ccv.consumer.v1.GenesisState consumer_genesis = 6
interchain_security.ccv.consumer.v1.GenesisState consumer_genesis = 5
[ (gogoproto.nullable) = false ];
// PendingValsetChanges defines the pending validator set changes for the consumer chain
repeated interchain_security.ccv.v1.ValidatorSetChangePacketData pending_valset_changes = 7
repeated interchain_security.ccv.v1.ValidatorSetChangePacketData pending_valset_changes = 6
[ (gogoproto.nullable) = false ];
repeated string slash_downtime_ack = 8;
repeated string slash_downtime_ack = 7;
// UnbondingOpsIndex defines the unbonding operations on the consumer chain
repeated UnbondingOpIndex unbonding_ops_index = 9
repeated UnbondingOpIndex unbonding_ops_index = 8
[ (gogoproto.nullable) = false ];
}

Expand Down
6 changes: 1 addition & 5 deletions proto/interchain_security/ccv/provider/v1/provider.proto
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,10 @@ message ConsumerAdditionProposal {
// will be responsible for starting their consumer chain validator node.
google.protobuf.Timestamp spawn_time = 7
[(gogoproto.stdtime) = true, (gogoproto.nullable) = false];
// Indicates whether the outstanding unbonding operations should be released
// in case of a channel time-outs. When set to true, a governance proposal
// on the provider chain would be necessary to release the locked funds.
bool lock_unbonding_on_timeout = 8;
}
// ConsumerRemovalProposal is a governance proposal on the provider chain to remove (and stop) a consumer chain.
// If it passes, all the consumer chain's state is removed from the provider chain. The outstanding unbonding
// operation funds are released if the LockUnbondingOnTimeout parameter is set to false for the consumer chain ID.
// operation funds are released.
message ConsumerRemovalProposal {
// the title of the proposal
string title = 1;
Expand Down
9 changes: 4 additions & 5 deletions tests/e2e/stop_consumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (s *CCVTestSuite) TestStopConsumerChain() {
// - setup CCV channel; establish CCV channel and set channelToChain, chainToChannel and initHeight mapping for the consumer chain ID
// - delegate the total bond amount to the chosed validator
// - undelegate the shares in four consecutive blocks evenly; create UnbondigOp and UnbondingOpIndex entries for the consumer chain ID
// - set SlashAck and LockUnbondingOnTimeout states for the consumer chain ID
// - set SlashAck state for the consumer chain ID
setupOperations := []struct {
fn func(suite *CCVTestSuite) error
}{
Expand Down Expand Up @@ -76,7 +76,6 @@ func (s *CCVTestSuite) TestStopConsumerChain() {
{
func(suite *CCVTestSuite) error {
providerKeeper.SetSlashAcks(s.providerCtx(), consumerChainID, []string{"validator-1", "validator-2", "validator-3"})
providerKeeper.SetLockUnbondingOnTimeout(s.providerCtx(), consumerChainID)
providerKeeper.AppendPendingPackets(s.providerCtx(), consumerChainID, ccv.ValidatorSetChangePacketData{ValsetUpdateId: 1})
return nil
},
Expand All @@ -89,7 +88,7 @@ func (s *CCVTestSuite) TestStopConsumerChain() {
}

// stop the consumer chain
err = providerKeeper.StopConsumerChain(s.providerCtx(), consumerChainID, false, true)
err = providerKeeper.StopConsumerChain(s.providerCtx(), consumerChainID, true)
s.Require().NoError(err)

// check all states are removed and the unbonding operation released
Expand All @@ -106,7 +105,7 @@ func (s *CCVTestSuite) TestStopConsumerOnChannelClosed() {
providerKeeper := s.providerApp.GetProviderKeeper()

// stop the consumer chain
err := providerKeeper.StopConsumerChain(s.providerCtx(), s.consumerChain.ChainID, true, true)
err := providerKeeper.StopConsumerChain(s.providerCtx(), s.consumerChain.ChainID, true)
s.Require().NoError(err)

err = s.path.EndpointA.UpdateClient()
Expand Down Expand Up @@ -160,7 +159,7 @@ func (s *CCVTestSuite) checkConsumerChainIsRemoved(chainID string, lockUbd bool,
// verify consumer chain's states are removed
_, found := providerKeeper.GetConsumerGenesis(s.providerCtx(), chainID)
s.Require().False(found)
s.Require().False(providerKeeper.GetLockUnbondingOnTimeout(s.providerCtx(), chainID))

_, found = providerKeeper.GetConsumerClientId(s.providerCtx(), chainID)
s.Require().False(found)

Expand Down
11 changes: 4 additions & 7 deletions x/ccv/provider/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
if err := k.SetConsumerGenesis(ctx, chainID, cs.ConsumerGenesis); err != nil {
panic(fmt.Errorf("consumer chain genesis could not be persisted: %w", err))
}
if cs.LockUnbondingOnTimeout {
k.SetLockUnbondingOnTimeout(ctx, chainID)
}

// check if the CCV channel was established
if cs.ChannelId != "" {
k.SetChannelToChain(ctx, cs.ChannelId, chainID)
Expand Down Expand Up @@ -93,10 +91,9 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {

// initial consumer chain states
cs := types.ConsumerState{
ChainId: chainID,
ClientId: clientID,
ConsumerGenesis: gen,
LockUnbondingOnTimeout: k.GetLockUnbondingOnTimeout(ctx, chainID),
ChainId: chainID,
ClientId: clientID,
ConsumerGenesis: gen,
}

// try to find channel id for the current consumer chain
Expand Down
2 changes: 0 additions & 2 deletions x/ccv/provider/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,6 @@ func assertConsumerChainStates(ctx sdk.Context, t *testing.T, pk keeper.Keeper,
require.True(t, found)
}

require.Equal(t, cs.LockUnbondingOnTimeout, pk.GetLockUnbondingOnTimeout(ctx, chainID))

if expVSC := cs.GetPendingValsetChanges(); expVSC != nil {
gotVSC := pk.GetPendingPackets(ctx, chainID)
require.Equal(t, expVSC, gotVSC)
Expand Down
20 changes: 0 additions & 20 deletions x/ccv/provider/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,26 +751,6 @@ func (k Keeper) DeletePendingPackets(ctx sdk.Context, chainID string) {
store.Delete(types.PendingVSCsKey(chainID))
}

// GetLockUnbondingOnTimeout returns the mapping from the given consumer chain ID to a boolean value indicating whether
// the unbonding operation funds should be locked on CCV channel timeout
func (k Keeper) GetLockUnbondingOnTimeout(ctx sdk.Context, chainID string) bool {
store := ctx.KVStore(k.storeKey)
bz := store.Get(types.LockUnbondingOnTimeoutKey(chainID))
return bz != nil
}

// SetLockUnbondingOnTimeout locks the unbonding operation funds in case of a CCV channel timeouts for the given consumer chain ID
func (k Keeper) SetLockUnbondingOnTimeout(ctx sdk.Context, chainID string) {
store := ctx.KVStore(k.storeKey)
store.Set(types.LockUnbondingOnTimeoutKey(chainID), []byte{})
}

// DeleteLockUnbondingOnTimeout deletes the unbonding operation lock in case of a CCV channel timeouts for the given consumer chain ID
func (k Keeper) DeleteLockUnbondingOnTimeout(ctx sdk.Context, chainID string) {
store := ctx.KVStore(k.storeKey)
store.Delete(types.LockUnbondingOnTimeoutKey(chainID))
}

// SetConsumerClientId sets the client ID for the given chain ID
func (k Keeper) SetConsumerClientId(ctx sdk.Context, chainID, clientID string) {
store := ctx.KVStore(k.storeKey)
Expand Down
82 changes: 36 additions & 46 deletions x/ccv/provider/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ import (
// Spec tag: [CCV-PCF-HCAPROP.1]
func (k Keeper) HandleConsumerAdditionProposal(ctx sdk.Context, p *types.ConsumerAdditionProposal) error {
if !ctx.BlockTime().Before(p.SpawnTime) {
// lockUbdOnTimeout is set to be false, regardless of what the proposal says, until we can specify and test issues around this use case more thoroughly
return k.CreateConsumerClient(ctx, p.ChainId, p.InitialHeight, false)
return k.CreateConsumerClient(ctx, p.ChainId, p.InitialHeight)
}

err := k.SetPendingConsumerAdditionProp(ctx, p)
Expand All @@ -47,7 +46,7 @@ func (k Keeper) HandleConsumerAdditionProposal(ctx sdk.Context, p *types.Consume
// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-crclient1
// Spec tag: [CCV-PCF-CRCLIENT.1]
func (k Keeper) CreateConsumerClient(ctx sdk.Context, chainID string,
initialHeight clienttypes.Height, lockUbdOnTimeout bool) error {
initialHeight clienttypes.Height) error {

// check that a client for this chain does not exist
if _, found := k.GetConsumerClientId(ctx, chainID); found {
Expand Down Expand Up @@ -91,11 +90,6 @@ func (k Keeper) CreateConsumerClient(ctx sdk.Context, chainID string,
ts := ctx.BlockTime().Add(k.GetParams(ctx).InitTimeoutPeriod)
k.SetInitTimeoutTimestamp(ctx, chainID, uint64(ts.UnixNano()))

// store LockUnbondingOnTimeout flag
if lockUbdOnTimeout {
k.SetLockUnbondingOnTimeout(ctx, chainID)
}

ctx.EventManager().EmitEvent(
sdk.NewEvent(
ccv.EventTypeConsumerClientCreated,
Expand All @@ -121,20 +115,20 @@ func (k Keeper) CreateConsumerClient(ctx sdk.Context, chainID string,
func (k Keeper) HandleConsumerRemovalProposal(ctx sdk.Context, p *types.ConsumerRemovalProposal) error {

if !ctx.BlockTime().Before(p.StopTime) {
return k.StopConsumerChain(ctx, p.ChainId, false, true)
return k.StopConsumerChain(ctx, p.ChainId, true)
}

k.SetPendingConsumerRemovalProp(ctx, p.ChainId, p.StopTime)
return nil
}

// StopConsumerChain cleans up the states for the given consumer chain ID and, if the given lockUbd is false,
// StopConsumerChain cleans up the states for the given consumer chain ID and
// it completes the outstanding unbonding operations lock by the consumer chain.
//
// This method implements StopConsumerChain from spec.
// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-stcc1
// Spec tag: [CCV-PCF-STCC.1]
func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, lockUbd, closeChan bool) (err error) {
func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, closeChan bool) (err error) {
// check that a client for chainID exists
if _, found := k.GetConsumerClientId(ctx, chainID); !found {
// drop the proposal
Expand All @@ -144,7 +138,6 @@ func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, lockUbd, clos
// clean up states
k.DeleteConsumerClientId(ctx, chainID)
k.DeleteConsumerGenesis(ctx, chainID)
k.DeleteLockUnbondingOnTimeout(ctx, chainID)
k.DeleteInitTimeoutTimestamp(ctx, chainID)

// close channel and delete the mappings between chain ID and channel ID
Expand Down Expand Up @@ -172,40 +165,38 @@ func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, lockUbd, clos

// release unbonding operations if they aren't locked
var vscIDs []uint64
if !lockUbd {
// iterate over the consumer chain's unbonding operation VSC ids
k.IterateOverUnbondingOpIndex(ctx, chainID, func(vscID uint64, ids []uint64) (stop bool) {
// iterate over the unbonding operations for the current VSC ID
var maturedIds []uint64
for _, id := range ids {
unbondingOp, found := k.GetUnbondingOp(ctx, id)
if !found {
err = fmt.Errorf("could not find UnbondingOp according to index - id: %d", id)
return true // stop the iteration
}
// remove consumer chain ID from unbonding op record
unbondingOp.UnbondingConsumerChains, _ = removeStringFromSlice(unbondingOp.UnbondingConsumerChains, chainID)

// If unbonding op is completely unbonded from all relevant consumer chains
if len(unbondingOp.UnbondingConsumerChains) == 0 {
// Store id of matured unbonding op for later completion of unbonding in staking module
maturedIds = append(maturedIds, unbondingOp.Id)
// Delete unbonding op
k.DeleteUnbondingOp(ctx, unbondingOp.Id)
} else {
if err := k.SetUnbondingOp(ctx, unbondingOp); err != nil {
panic(fmt.Errorf("unbonding op could not be persisted: %w", err))
}
}
// iterate over the consumer chain's unbonding operation VSC ids
k.IterateOverUnbondingOpIndex(ctx, chainID, func(vscID uint64, ids []uint64) (stop bool) {
// iterate over the unbonding operations for the current VSC ID
var maturedIds []uint64
for _, id := range ids {
unbondingOp, found := k.GetUnbondingOp(ctx, id)
if !found {
err = fmt.Errorf("could not find UnbondingOp according to index - id: %d", id)
return true // stop the iteration
}
if err := k.AppendMaturedUnbondingOps(ctx, maturedIds); err != nil {
panic(fmt.Errorf("mature unbonding ops could not be appended: %w", err))
// remove consumer chain ID from unbonding op record
unbondingOp.UnbondingConsumerChains, _ = removeStringFromSlice(unbondingOp.UnbondingConsumerChains, chainID)

// If unbonding op is completely unbonded from all relevant consumer chains
if len(unbondingOp.UnbondingConsumerChains) == 0 {
// Store id of matured unbonding op for later completion of unbonding in staking module
maturedIds = append(maturedIds, unbondingOp.Id)
// Delete unbonding op
k.DeleteUnbondingOp(ctx, unbondingOp.Id)
} else {
if err := k.SetUnbondingOp(ctx, unbondingOp); err != nil {
panic(fmt.Errorf("unbonding op could not be persisted: %w", err))
}
}
}
if err := k.AppendMaturedUnbondingOps(ctx, maturedIds); err != nil {
panic(fmt.Errorf("mature unbonding ops could not be appended: %w", err))
}

vscIDs = append(vscIDs, vscID)
return false // do not stop the iteration
})
}
vscIDs = append(vscIDs, vscID)
return false // do not stop the iteration
})

if err != nil {
return err
Expand Down Expand Up @@ -323,8 +314,7 @@ func (k Keeper) BeginBlockInit(ctx sdk.Context) {
propsToExecute := k.ConsumerAdditionPropsToExecute(ctx)

for _, prop := range propsToExecute {
// lockUbdOnTimeout is set to be false, regardless of what the proposal says, until we can specify and test issues around this use case more thoroughly
err := k.CreateConsumerClient(ctx, prop.ChainId, prop.InitialHeight, false)
err := k.CreateConsumerClient(ctx, prop.ChainId, prop.InitialHeight)
if err != nil {
panic(fmt.Errorf("consumer client could not be created: %w", err))
}
Expand Down Expand Up @@ -448,7 +438,7 @@ func (k Keeper) BeginBlockCCR(ctx sdk.Context) {
propsToExecute := k.ConsumerRemovalPropsToExecute(ctx)

for _, prop := range propsToExecute {
err := k.StopConsumerChain(ctx, prop.ChainId, false, true)
err := k.StopConsumerChain(ctx, prop.ChainId, true)
if err != nil {
panic(fmt.Errorf("consumer chain failed to stop: %w", err))
}
Expand Down
10 changes: 1 addition & 9 deletions x/ccv/provider/keeper/proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ func TestHandleConsumerAdditionProposal(t *testing.T) {
)
}

tc.prop.LockUnbondingOnTimeout = false // Full functionality not implemented yet.

err := providerKeeper.HandleConsumerAdditionProposal(ctx, tc.prop)
require.NoError(t, err)

Expand Down Expand Up @@ -187,10 +185,6 @@ func testCreatedConsumerClient(t *testing.T,
require.True(t, found, "consumer client not found")
require.Equal(t, expectedClientID, clientId)

// Lock unbonding on timeout flag always false for now.
lockUbdOnTimeout := providerKeeper.GetLockUnbondingOnTimeout(ctx, expectedChainID)
require.False(t, lockUbdOnTimeout)

// Only assert that consumer genesis was set,
// more granular tests on consumer genesis should be defined in TestMakeConsumerGenesis
_, ok := providerKeeper.GetConsumerGenesis(ctx, expectedChainID)
Expand Down Expand Up @@ -438,7 +432,7 @@ func TestStopConsumerChain(t *testing.T) {
// Setup specific to test case
tc.setup(ctx, &providerKeeper, mocks)

err := providerKeeper.StopConsumerChain(ctx, "chainID", false, true)
err := providerKeeper.StopConsumerChain(ctx, "chainID", true)

if tc.expErr {
require.Error(t, err)
Expand All @@ -458,8 +452,6 @@ func testProviderStateIsCleaned(t *testing.T, ctx sdk.Context, providerKeeper pr

_, found := providerKeeper.GetConsumerClientId(ctx, expectedChainID)
require.False(t, found)
found = providerKeeper.GetLockUnbondingOnTimeout(ctx, expectedChainID)
require.False(t, found)
_, found = providerKeeper.GetChainToChannel(ctx, expectedChainID)
require.False(t, found)
_, found = providerKeeper.GetChannelToChain(ctx, expectedChannelID)
Expand Down
22 changes: 7 additions & 15 deletions x/ccv/provider/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,8 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac
// The VSC packet data could not be successfully decoded.
// This should never happen.
if chainID, ok := k.GetChannelToChain(ctx, packet.SourceChannel); ok {
// stop consumer chain and uses the LockUnbondingOnTimeout flag
// to decide whether the unbonding operations should be released
return k.StopConsumerChain(ctx, chainID, k.GetLockUnbondingOnTimeout(ctx, chainID), false)
// stop consumer chain
return k.StopConsumerChain(ctx, chainID, false)
}
return sdkerrors.Wrapf(types.ErrUnknownConsumerChannelId, "recv ErrorAcknowledgement on unknown channel %s", packet.SourceChannel)
}
Expand All @@ -116,9 +115,8 @@ func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet) err
packet.SourceChannel,
)
}
// stop consumer chain and uses the LockUnbondingOnTimeout flag
// to decide whether the unbonding operations should be released
return k.StopConsumerChain(ctx, chainID, k.GetLockUnbondingOnTimeout(ctx, chainID), false)
// stop consumer chain
return k.StopConsumerChain(ctx, chainID, false)
}

// EndBlockVSU contains the EndBlock logic needed for
Expand Down Expand Up @@ -343,7 +341,7 @@ func (k Keeper) EndBlockCCR(ctx sdk.Context) {
// stop the consumer chain and unlock the unbonding.
// Note that the CCV channel was not established,
// thus closeChan is irrelevant
err := k.StopConsumerChain(ctx, chainID, false, false)
err := k.StopConsumerChain(ctx, chainID, false)
if err != nil {
panic(fmt.Errorf("consumer chain failed to stop: %w", err))
}
Expand Down Expand Up @@ -372,14 +370,8 @@ func (k Keeper) EndBlockCCR(ctx sdk.Context) {
})
// remove consumers that timed out
for _, chainID := range chainIdsToRemove {
// stop the consumer chain and use lockUnbondingOnTimeout
// to decide whether to lock the unbonding
err := k.StopConsumerChain(
ctx,
chainID,
k.GetLockUnbondingOnTimeout(ctx, chainID),
true,
)
// stop the consumer chain
err := k.StopConsumerChain(ctx, chainID, true)
if err != nil {
panic(fmt.Errorf("consumer chain failed to stop: %w", err))
}
Expand Down
Loading