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.

Sign up for GitHub

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

Jump to bottom

feat!: only perform consumer additions for non-empty chains #1730

Merged
merged 2 commits into from
Mar 27, 2024
Merged

feat!: only perform consumer additions for non-empty chains #1730

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8921e5a
init commit
insumity Mar 26, 2024
66e5adf
Update x/ccv/provider/keeper/proposal.go
insumity Mar 26, 2024
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
9 changes: 8 additions & 1 deletion x/ccv/provider/keeper/proposal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,13 @@ func (k Keeper) BeginBlockInit(ctx sdk.Context) {
propsToExecute := k.GetConsumerAdditionPropsToExecute(ctx)

for _, prop := range propsToExecute {
if prop.Top_N == 0 && len(k.GetAllOptedIn(ctx, prop.ChainId)) == 0 {
// drop the proposal
ctx.Logger().Info("consumer client could not be created because no validator has"+
" opted in the Opt-In chain: %s", prop.ChainId)
insumity marked this conversation as resolved.
Show resolved Hide resolved
continue
}

// create consumer client in a cached context to handle errors
cachedCtx, writeFn, err := k.CreateConsumerClientInCachedCtx(ctx, prop)
if err != nil {
Expand All @@ -384,7 +391,7 @@ func (k Keeper) BeginBlockInit(ctx sdk.Context) {

// 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)
k.SetTopN(cachedCtx, prop.ChainId, prop.Top_N)

// The cached context is created with a new EventManager so we merge the event
// into the original context
Expand Down
87 changes: 75 additions & 12 deletions x/ccv/provider/keeper/proposal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,7 @@
100000000000,
100000000000,
100000000000,
67,
50,
).(*providertypes.ConsumerAdditionProposal),
providertypes.NewConsumerAdditionProposal(
"title", "spawn time passed", "chain2", clienttypes.NewHeight(3, 4), []byte{}, []byte{},
Expand All @@ -938,7 +938,7 @@
100000000000,
100000000000,
100000000000,
0,
50,
).(*providertypes.ConsumerAdditionProposal),
providertypes.NewConsumerAdditionProposal(
"title", "spawn time not passed", "chain3", clienttypes.NewHeight(3, 4), []byte{}, []byte{},
Expand All @@ -950,7 +950,7 @@
100000000000,
100000000000,
100000000000,
0,
50,
).(*providertypes.ConsumerAdditionProposal),
providertypes.NewConsumerAdditionProposal(
"title", "invalid proposal: chain id already exists", "chain2", clienttypes.NewHeight(4, 5), []byte{}, []byte{},
Expand All @@ -962,46 +962,109 @@
100000000000,
100000000000,
100000000000,
50,
).(*providertypes.ConsumerAdditionProposal),
providertypes.NewConsumerAdditionProposal(
"title", "opt-in chain with no validator opted in", "chain4", clienttypes.NewHeight(3, 4), []byte{}, []byte{},
now.Add(-time.Hour*2).UTC(),
"0.75",
10,
"",
10000,
100000000000,
100000000000,
100000000000,
0,
).(*providertypes.ConsumerAdditionProposal),
providertypes.NewConsumerAdditionProposal(
"title", "opt-in chain with at least one validator opted in", "chain5", clienttypes.NewHeight(3, 4), []byte{}, []byte{},
now.Add(-time.Hour*1).UTC(),
"0.75",
10,
"",
10000,
100000000000,
100000000000,
100000000000,
0,
).(*providertypes.ConsumerAdditionProposal),
}

// Expect client creation for only for the 1st and second proposals (spawn time already passed and valid)
gomock.InOrder(
append(testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain1", clienttypes.NewHeight(3, 4)),
testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain2", clienttypes.NewHeight(3, 4))...)...,
)
// Expect client creation for only the first, second, and sixth proposals (spawn time already passed and valid)
expectedCalls := testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain1", clienttypes.NewHeight(3, 4))
expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain2", clienttypes.NewHeight(3, 4))...)
expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain5", clienttypes.NewHeight(3, 4))...)

gomock.InOrder(expectedCalls...)

for _, prop := range pendingProps {
providerKeeper.SetPendingConsumerAdditionProp(ctx, prop)
}

// opt in a sample validator so the chain's proposal can successfully execute
providerKeeper.SetOptedIn(ctx, pendingProps[5].ChainId, providertypes.OptedInValidator{

Check failure on line 1005 in x/ccv/provider/keeper/proposal_test.go

View workflow job for this annotation

GitHub Actions / tests 

undefined: providertypes.OptedInValidator
ProviderAddr: []byte{1},
BlockHeight: int64(2),
Power: int64(3),
PublicKey: []byte{4},
})

providerKeeper.BeginBlockInit(ctx)

// Only the third proposal is still stored as pending
// first proposal is not pending anymore because its spawn time already passed and was executed
_, found := providerKeeper.GetPendingConsumerAdditionProp(
ctx, pendingProps[0].SpawnTime, pendingProps[0].ChainId)
require.False(t, found)
// first proposal was successfully executed and hence consumer genesis was created
_, found = providerKeeper.GetConsumerGenesis(ctx, pendingProps[0].ChainId)
require.True(t, found)

// second proposal is not pending anymore because its spawn time already passed and was executed
_, found = providerKeeper.GetPendingConsumerAdditionProp(
ctx, pendingProps[1].SpawnTime, pendingProps[1].ChainId)
require.False(t, found)
// second proposal was successfully executed and hence consumer genesis was created
_, found = providerKeeper.GetConsumerGenesis(ctx, pendingProps[1].ChainId)
require.True(t, found)

// third proposal is still stored as pending because its spawn time has not passed
_, found = providerKeeper.GetPendingConsumerAdditionProp(
ctx, pendingProps[2].SpawnTime, pendingProps[2].ChainId)
require.True(t, found)
// because the proposal is still pending, no consumer genesis was created
_, found = providerKeeper.GetConsumerGenesis(ctx, pendingProps[2].ChainId)
require.False(t, found)

// check that the invalid proposal was dropped
// check that the invalid proposals were dropped
_, found = providerKeeper.GetPendingConsumerAdditionProp(
ctx, pendingProps[3].SpawnTime, pendingProps[3].ChainId)
require.False(t, found)
// Note that we do not check that `GetConsumerGenesis(ctx, pendingProps[3].ChainId)` returns `false` here because
//`pendingProps[3]` is an invalid proposal due to the chain id already existing so the consumer genesis also exists

// fifth proposal is dropped due to it being an Opt-In chain with no validators opted in
_, found = providerKeeper.GetPendingConsumerAdditionProp(
ctx, pendingProps[4].SpawnTime, pendingProps[4].ChainId)
require.False(t, found)
// because the proposal is dropped, no consumer genesis was created
_, found = providerKeeper.GetConsumerGenesis(ctx, pendingProps[4].ChainId)
require.False(t, found)

// sixth proposal corresponds to an Opt-In chain with one opted-in validator and hence the proposal gets
// successfully executed
_, found = providerKeeper.GetPendingConsumerAdditionProp(
ctx, pendingProps[5].SpawnTime, pendingProps[5].ChainId)
require.False(t, found)
// sixth proposal was successfully executed and hence consumer genesis was created
_, found = providerKeeper.GetConsumerGenesis(ctx, pendingProps[5].ChainId)
require.True(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.Equal(t, uint32(50), topN)

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

// TestBeginBlockCCR tests BeginBlockCCR against the spec.
Expand Down
Loading