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

feat!: enable Opt In and Top N chains through gov proposals #1615

Merged
merged 12 commits into from
Feb 5, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Enable Opt In and Top N chains through gov proposals.
([\#1587](https://github.com/cosmos/interchain-security/pull/1587))
5 changes: 5 additions & 0 deletions proto/interchain_security/ccv/provider/v1/provider.proto
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ message ConsumerAdditionProposal {
// chain. it is most relevant for chains performing a sovereign to consumer
// changeover in order to maintain the existing ibc transfer channel
string distribution_transmission_channel = 14;
// Corresponds to the percentage of validators that have to validate the chain under the Top N case.
// For example, 53 corresponds to a Top 53% chain, meaning that the top 53% provider validators by voting power
// have to validate the proposed consumer chain. top_N can either be 0 or any value in [50, 100].
// A chain can join with top_N == 0 as an Opt In chain, or with top_N ∈ [50, 100] as a Top N chain.
uint32 top_N = 15;
}

// ConsumerRemovalProposal is a governance proposal on the provider chain to
Expand Down
1 change: 1 addition & 0 deletions testutil/keeper/unit_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ func GetTestConsumerAdditionProp() *providertypes.ConsumerAdditionProposal {
types.DefaultCCVTimeoutPeriod,
types.DefaultTransferTimeoutPeriod,
types.DefaultConsumerUnbondingPeriod,
0,
).(*providertypes.ConsumerAdditionProposal)

return prop
Expand Down
6 changes: 4 additions & 2 deletions x/ccv/provider/client/proposal_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ Where proposal.json contains:
"transfer_timeout_period": 3600000000000,
"ccv_timeout_period": 2419200000000000,
"unbonding_period": 1728000000000000,
"deposit": "10000stake"
"deposit": "10000stake",
"top_n": 0,
}
`,
RunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -86,7 +87,7 @@ Where proposal.json contains:
proposal.GenesisHash, proposal.BinaryHash, proposal.SpawnTime,
proposal.ConsumerRedistributionFraction, proposal.BlocksPerDistributionTransmission,
proposal.DistributionTransmissionChannel, proposal.HistoricalEntries,
proposal.CcvTimeoutPeriod, proposal.TransferTimeoutPeriod, proposal.UnbondingPeriod)
proposal.CcvTimeoutPeriod, proposal.TransferTimeoutPeriod, proposal.UnbondingPeriod, proposal.TopN)

from := clientCtx.GetFromAddress()

Expand Down Expand Up @@ -241,6 +242,7 @@ type ConsumerAdditionProposalJSON struct {
UnbondingPeriod time.Duration `json:"unbonding_period"`

Deposit string `json:"deposit"`
TopN uint32 `json:"top_N"`
}

type ConsumerAdditionProposalReq struct {
Expand Down
48 changes: 48 additions & 0 deletions x/ccv/provider/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -1136,3 +1136,51 @@ func (k Keeper) GetAllRegisteredAndProposedChainIDs(ctx sdk.Context) []string {

return allConsumerChains
}

// SetTopN stores the N value associated to chain with `chainID`
func (k Keeper) SetTopN(
ctx sdk.Context,
chainID string,
N uint32,
) {
store := ctx.KVStore(k.storeKey)

buf := make([]byte, 4)
binary.BigEndian.PutUint32(buf, N)

store.Set(types.TopNKey(chainID), buf)
}

// DeleteTopN removes the N value associated to chain with `chainID`
func (k Keeper) DeleteTopN(
ctx sdk.Context,
chainID string,
) {
store := ctx.KVStore(k.storeKey)
store.Delete(types.TopNKey(chainID))
}

// GetTopN returns (N, true) if chain `chainID` has a top N associated, and (0, false) otherwise.
func (k Keeper) GetTopN(
ctx sdk.Context,
chainID string,
) (uint32, bool) {
store := ctx.KVStore(k.storeKey)
buf := store.Get(types.TopNKey(chainID))
if buf == nil {
return 0, false
}
return binary.BigEndian.Uint32(buf), true
}

// IsTopN returns true if chain with `chainID` is a Top N chain (i.e., enforces at least one validator to validate chain `chainID`)
func (k Keeper) IsTopN(ctx sdk.Context, chainID string) bool {
topN, found := k.GetTopN(ctx, chainID)
return found && topN > 0
}

// IsOptIn returns true if chain with `chainID` is an Opt In chain (i.e., no validator is forced to validate chain `chainID`)
func (k Keeper) IsOptIn(ctx sdk.Context, chainID string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

It is strange to me that 0 and not found are treated the same here, should ensure this doesn't lead to errors at some point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is strange, indeed. The problem stems from the fact that we use uint32 which has a zero value of 0. It would be impossible to distinguish on whether a user that submits a ConsumerAdditionProposal sets 0 to Top_N or does not include Top_N at all. Because, if a user forgets to set Top_N, it would be considered the same as 0 when we look at it.
I guess a solution would be to use a string or something and if the string is empty, then we can infer that the user did not provide a value, but I'm not sure if it's worth it.

At the end of the day, everything has to go through a proposal, so a proposal would be scrutinized before it passes. If for a proposal we have Top_N = 0 but the proposal is for a Top N chain, then something obviously went wrong with this proposal and it should be rejected.

topN, found := k.GetTopN(ctx, chainID)
return !found || topN == 0
}
31 changes: 31 additions & 0 deletions x/ccv/provider/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -628,3 +628,34 @@ func TestGetAllProposedConsumerChainIDs(t *testing.T) {
}
}
}

// TestTopN tests `SetTopN`, `GetTopN`, `IsTopN`, and `IsOptIn` methods
func TestTopN(t *testing.T) {
insumity marked this conversation as resolved.
Show resolved Hide resolved
providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

tests := []struct {
chainID string
N uint32
isOptIn bool
}{
{chainID: "TopNChain1", N: 50, isOptIn: false},
{chainID: "TopNChain2", N: 100, isOptIn: false},
{chainID: "OptInChain", N: 0, isOptIn: true},
}

for _, test := range tests {
providerKeeper.SetTopN(ctx, test.chainID, test.N)
topN, found := providerKeeper.GetTopN(ctx, test.chainID)
require.Equal(t, test.N, topN)
require.True(t, found)

if test.isOptIn {
require.True(t, providerKeeper.IsOptIn(ctx, test.chainID))
require.False(t, providerKeeper.IsTopN(ctx, test.chainID))
} else {
require.False(t, providerKeeper.IsOptIn(ctx, test.chainID))
require.True(t, providerKeeper.IsTopN(ctx, test.chainID))
}
}
}
7 changes: 7 additions & 0 deletions x/ccv/provider/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, closeChan boo
k.DeleteUnbondingOpIndex(ctx, chainID, unbondingOpsIndex.VscId)
}

k.DeleteTopN(ctx, chainID)

k.Logger(ctx).Info("consumer chain removed from provider", "chainID", chainID)

return nil
Expand Down Expand Up @@ -365,6 +367,11 @@ func (k Keeper) BeginBlockInit(ctx sdk.Context) {
ctx.Logger().Info("consumer client could not be created: %w", err)
continue
}

// Only set Top N at the moment a chain starts. If we were to do this earlier (e.g., during the proposal),
// then someone could create a bogus ConsumerAdditionProposal to override the Top N for a specific chain.
k.SetTopN(ctx, prop.ChainId, prop.Top_N)

// The cached context is created with a new EventManager so we merge the event
// into the original context
ctx.EventManager().EmitEvents(cachedCtx.EventManager().Events())
Expand Down
18 changes: 18 additions & 0 deletions x/ccv/provider/keeper/proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func TestHandleConsumerAdditionProposal(t *testing.T) {
100000000000,
100000000000,
100000000000,
0,
).(*providertypes.ConsumerAdditionProposal),
blockTime: now,
expAppendProp: true,
Expand All @@ -89,6 +90,7 @@ func TestHandleConsumerAdditionProposal(t *testing.T) {
100000000000,
100000000000,
100000000000,
0,
).(*providertypes.ConsumerAdditionProposal),
blockTime: now,
expAppendProp: false,
Expand Down Expand Up @@ -552,6 +554,10 @@ func TestStopConsumerChain(t *testing.T) {
require.Error(t, err)
} else {
require.NoError(t, err)

// in case the chain was successfully stopped, it should not contain a Top N associated to it
_, found := providerKeeper.GetTopN(ctx, "chainID")
require.False(t, found)
}

testkeeper.TestProviderStateIsCleanedAfterConsumerChainIsStopped(t, ctx, providerKeeper, "chainID", "channelID")
Expand Down Expand Up @@ -923,6 +929,7 @@ func TestBeginBlockInit(t *testing.T) {
100000000000,
100000000000,
100000000000,
67,
).(*providertypes.ConsumerAdditionProposal),
providertypes.NewConsumerAdditionProposal(
"title", "spawn time passed", "chain2", clienttypes.NewHeight(3, 4), []byte{}, []byte{},
Expand All @@ -934,6 +941,7 @@ func TestBeginBlockInit(t *testing.T) {
100000000000,
100000000000,
100000000000,
0,
).(*providertypes.ConsumerAdditionProposal),
providertypes.NewConsumerAdditionProposal(
"title", "spawn time not passed", "chain3", clienttypes.NewHeight(3, 4), []byte{}, []byte{},
Expand All @@ -945,6 +953,7 @@ func TestBeginBlockInit(t *testing.T) {
100000000000,
100000000000,
100000000000,
0,
).(*providertypes.ConsumerAdditionProposal),
providertypes.NewConsumerAdditionProposal(
"title", "invalid proposal: chain id already exists", "chain2", clienttypes.NewHeight(4, 5), []byte{}, []byte{},
Expand All @@ -956,6 +965,7 @@ func TestBeginBlockInit(t *testing.T) {
100000000000,
100000000000,
100000000000,
0,
).(*providertypes.ConsumerAdditionProposal),
}

Expand Down Expand Up @@ -988,6 +998,13 @@ func TestBeginBlockInit(t *testing.T) {
_, found = providerKeeper.GetPendingConsumerAdditionProp(
ctx, pendingProps[3].SpawnTime, pendingProps[3].ChainId)
require.False(t, found)

// test that Top N is set correctly
require.True(t, providerKeeper.IsTopN(ctx, "chain1"))
topN, found := providerKeeper.GetTopN(ctx, "chain1")
require.Equal(t, uint32(67), topN)

require.True(t, providerKeeper.IsOptIn(ctx, "chain2"))
}

// TestBeginBlockCCR tests BeginBlockCCR against the spec.
Expand Down Expand Up @@ -1057,6 +1074,7 @@ func TestBeginBlockCCR(t *testing.T) {
//
// Test execution
//

providerKeeper.BeginBlockCCR(ctx)

// Only the 3rd (final) proposal is still stored as pending
Expand Down
1 change: 1 addition & 0 deletions x/ccv/provider/proposal_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func TestProviderProposalHandler(t *testing.T) {
100000000000,
100000000000,
100000000000,
0,
),
blockTime: hourFromNow, // ctx blocktime is after proposal's spawn time
expValidConsumerAddition: true,
Expand Down
8 changes: 8 additions & 0 deletions x/ccv/provider/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ const (
// ProposedConsumerChainByteKey is the byte prefix storing the consumer chainId in consumerAddition gov proposal submitted before voting finishes
ProposedConsumerChainByteKey

// TopNBytePrefix is the byte prefix storing the mapping from a consumer chain to the N value of this chain,
// that corresponds to the N% of the top validators that have to validate this consumer chain
TopNBytePrefix
// NOTE: DO NOT ADD NEW BYTE PREFIXES HERE WITHOUT ADDING THEM TO getAllKeyPrefixes() IN keys_test.go
)

Expand Down Expand Up @@ -517,6 +520,11 @@ func ParseProposedConsumerChainKey(prefix byte, bz []byte) (uint64, error) {
return proposalID, nil
}

// TopNKey returns the key of consumer chain `chainID`
func TopNKey(chainID string) []byte {
return ChainIdWithLenKey(TopNBytePrefix, chainID)
}

//
// End of generic helpers section
//
1 change: 1 addition & 0 deletions x/ccv/provider/types/keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func getAllKeyPrefixes() []byte {
providertypes.VSCMaturedHandledThisBlockBytePrefix,
providertypes.EquivocationEvidenceMinHeightBytePrefix,
providertypes.ProposedConsumerChainByteKey,
providertypes.TopNBytePrefix,
}
}

Expand Down
8 changes: 8 additions & 0 deletions x/ccv/provider/types/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func NewConsumerAdditionProposal(title, description, chainID string,
ccvTimeoutPeriod time.Duration,
transferTimeoutPeriod time.Duration,
unbondingPeriod time.Duration,
topN uint32,
) govv1beta1.Content {
return &ConsumerAdditionProposal{
Title: title,
Expand All @@ -65,6 +66,7 @@ func NewConsumerAdditionProposal(title, description, chainID string,
CcvTimeoutPeriod: ccvTimeoutPeriod,
TransferTimeoutPeriod: transferTimeoutPeriod,
UnbondingPeriod: unbondingPeriod,
Top_N: topN,
}
}

Expand Down Expand Up @@ -135,6 +137,12 @@ func (cccp *ConsumerAdditionProposal) ValidateBasic() error {
return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "unbonding period cannot be zero")
}

// Top N corresponds to the top N% of validators that have to validate the consumer chain and can only be 0 (for an
// Opt In chain) or in the range [50, 100] (for a Top N chain).
if cccp.Top_N != 0 && cccp.Top_N < 50 || cccp.Top_N > 100 {
return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "Top N can either be 0 or in the range [50, 100]")
}

return nil
}

Expand Down
Loading
Loading