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!: key assignment checking key in use #1339

Merged
merged 26 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
67ec715
feat!: store proposed chainID before voting finishes (#1289)
yaruwangway Oct 2, 2023
3a3e157
fix test
Oct 2, 2023
9fb3871
feat: update GetAllValidatorsByConsumerAddr for fast find consensus k…
yaruwangway Oct 2, 2023
35b4a5c
update changelog
Oct 2, 2023
e09d930
Update proto/interchain_security/ccv/provider/v1/query.proto
sainoe Oct 10, 2023
b97c1cb
Update x/ccv/provider/keeper/gov_hook.go
sainoe Oct 10, 2023
0ae0649
Update x/ccv/provider/keeper/keeper.go
sainoe Oct 10, 2023
880eaef
Update x/ccv/provider/keeper/keeper.go
sainoe Oct 10, 2023
c95faea
Update x/ccv/provider/keeper/keeper.go
sainoe Oct 10, 2023
584ce4e
fix gov hooks
sainoe Oct 11, 2023
b26b453
fix bug and add tests
sainoe Oct 12, 2023
a240ab5
finish unit testing of consu addition legacy prop getter
sainoe Nov 14, 2023
326423c
Merge branch 'main' into sainoe/refactor-key-assignment
sainoe Nov 14, 2023
94fd342
nit
sainoe Nov 15, 2023
f196cc9
update changelog
sainoe Nov 15, 2023
10441bd
Merge branch 'main' into feat/refactor-key-assignment
sainoe Nov 15, 2023
642ca29
lint
sainoe Nov 15, 2023
e1cb6b3
Merge branch 'sainoe/refactor-key-assignment' into feat/refactor-key-…
sainoe Nov 15, 2023
4b3b8ed
update changelog entry
sainoe Nov 15, 2023
50cb42a
Update .changelog/unreleased/features/provider/1339-check-key-assignm…
sainoe Nov 22, 2023
441bcee
fix #1282
sainoe Nov 22, 2023
167a74d
remove todos and update doc
sainoe Nov 23, 2023
efc3f95
tests: fix broken unit tests
MSalopek Nov 27, 2023
29528b2
tests: fix broken integration test consumer registration
MSalopek Nov 27, 2023
4a38b9a
tests: update names
MSalopek Nov 27, 2023
431b706
Resolve conflicts & merge branch 'main' into feat/refactor-key-assign…
MSalopek Nov 27, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Add an entry to the unreleased provider section whenever merging a PR to main th
* (deps) [#1258](https://github.com/cosmos/interchain-security/pull/1258) Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to [v0.47.4](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.4).
* (deps!) [#1196](https://github.com/cosmos/interchain-security/pull/1196) Bump [ibc-go](https://github.com/cosmos/ibc-go) to [v7.2.0](https://github.com/cosmos/ibc-go/releases/tag/v7.2.0).
* `[x/ccv/provider]` (fix) [#1076](https://github.com/cosmos/interchain-security/pull/1076) Add `InitTimeoutTimestamps` and `ExportedVscSendTimestamps` to exported genesis.
* (feature!) [#1282](https://github.com/cosmos/interchain-security/issues/1282) In the `ConsumerAdditionProposal`, consumer chainIDs proposed before the voting period finishes are now stored in the state. The gRPC query `/interchain_security/ccv/provider/proposed_consumer_chainids` and CLI command `query provider proposed-consumer-chains` can be used to retrieve this information.
sainoe marked this conversation as resolved.
Show resolved Hide resolved

## [Unreleased for Consumer]

Expand Down
9 changes: 7 additions & 2 deletions app/provider/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ func New(
AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper))
govConfig := govtypes.DefaultConfig()

app.GovKeeper = *govkeeper.NewKeeper(
govKeeper := govkeeper.NewKeeper(
appCodec,
keys[govtypes.StoreKey],
app.AccountKeeper,
Expand All @@ -455,7 +455,12 @@ func New(
)

// Set legacy router for backwards compatibility with gov v1beta1
app.GovKeeper.SetLegacyRouter(govRouter)
govKeeper.SetLegacyRouter(govRouter)

govHook := app.ProviderKeeper.GovHooks(govKeeper)
app.GovKeeper = *govKeeper.SetHooks(
govtypes.NewMultiGovHooks(govHook),
)

app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
Expand Down
21 changes: 21 additions & 0 deletions proto/interchain_security/ccv/provider/v1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,15 @@ service Query {
option (google.api.http).get =
"/interchain_security/ccv/provider/registered_consumer_reward_denoms";
}

// QueryProposedConsumerChainIDs query chainIDs in consumerAdditionProposals
// before voting period finishes, after voting period finishes, this chainID will be removed from the result
rpc QueryProposedConsumerChainIDs(
sainoe marked this conversation as resolved.
Show resolved Hide resolved
QueryProposedChainIDsRequest)
returns (QueryProposedChainIDsResponse) {
option (google.api.http).get =
"/interchain_security/ccv/provider/proposed_consumer_chains";
}
}

message QueryConsumerGenesisRequest { string chain_id = 1; }
Expand Down Expand Up @@ -187,3 +196,15 @@ message QueryRegisteredConsumerRewardDenomsRequest {}
message QueryRegisteredConsumerRewardDenomsResponse {
repeated string denoms = 1;
}

message QueryProposedChainIDsRequest {}

message QueryProposedChainIDsResponse {
repeated ProposedChain proposedChains = 1
[ (gogoproto.nullable) = false ];
}

message ProposedChain {
string chainID = 1;
uint64 proposalID = 2;
}
3 changes: 1 addition & 2 deletions tests/e2e/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,15 +417,14 @@ func (s *TestConfig) SetRelayerConfig(useRly bool) {
}

// validateStringLiterals enforces that configs follow the constraints
// necessary to to execute the tests
// necessary to execute the tests
//
// Note: Network interfaces (name of virtual ethernet interfaces for ip link)
// within the container will be named as "$CHAIN_ID-$VAL_ID-out" etc.
// where this name is constrained to 15 bytes or less. Therefore each string literal
// used as a validatorID or chainID needs to be 5 char or less.
func (s *TestConfig) validateStringLiterals() {
for valID, valConfig := range s.validatorConfigs {

if len(valID) > 5 {
panic("validator id string literal must be 5 char or less")
}
Expand Down
28 changes: 28 additions & 0 deletions tests/e2e/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type State map[ChainID]ChainState
type ChainState struct {
ValBalances *map[ValidatorID]uint
Proposals *map[uint]Proposal
ProposedConsumerChains *[]string
ValPowers *map[ValidatorID]uint
StakedTokens *map[ValidatorID]uint
Params *[]Param
Expand Down Expand Up @@ -132,6 +133,11 @@ func (tr TestConfig) getChainState(chain ChainID, modelState ChainState) ChainSt
chainState.Proposals = &proposals
}

if modelState.ProposedConsumerChains != nil {
proposedConsumerChains := tr.getProposedConsumerChains(chain)
chainState.ProposedConsumerChains = &proposedConsumerChains
}

if modelState.ValPowers != nil {
tr.waitBlocks(chain, 1, 10*time.Second)
powers := tr.getValPowers(chain, *modelState.ValPowers)
Expand Down Expand Up @@ -772,3 +778,25 @@ func (tr TestConfig) curlJsonRPCRequest(method, params, address string) {
verbosity := false
executeCommandWithVerbosity(cmd, "curlJsonRPCRequest", verbosity)
}

func (tr TestConfig) getProposedConsumerChains(chain ChainID) []string {
tr.waitBlocks(chain, 1, 10*time.Second)
//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
bz, err := exec.Command("docker", "exec", tr.containerConfig.InstanceName, tr.chainConfigs[chain].BinaryName,
"query", "provider", "list-proposed-consumer-chains",
`--node`, tr.getQueryNode(chain),
`-o`, `json`,
).CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}

arr := gjson.Get(string(bz), "proposedChains").Array()
chains := []string{}
for _, c := range arr {
cid := c.Get("chainID").String()
chains = append(chains, cid)
}

return chains
}
2 changes: 2 additions & 0 deletions tests/e2e/steps_start_chains.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func stepsStartConsumerChain(consumerName string, proposalIndex, chainIndex uint
Status: "PROPOSAL_STATUS_VOTING_PERIOD",
},
},
ProposedConsumerChains: &[]string{consumerName},
},
},
},
Expand Down Expand Up @@ -163,6 +164,7 @@ func stepsStartConsumerChain(consumerName string, proposalIndex, chainIndex uint
ValidatorID("bob"): 9500000000,
ValidatorID("carol"): 9500000000,
},
ProposedConsumerChains: &[]string{},
},
ChainID(consumerName): ChainState{
ValBalances: &map[ValidatorID]uint{
Expand Down
28 changes: 28 additions & 0 deletions x/ccv/provider/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func NewQueryCmd() *cobra.Command {
cmd.AddCommand(CmdThrottleState())
cmd.AddCommand(CmdThrottledConsumerPacketData())
cmd.AddCommand(CmdRegisteredConsumerRewardDenoms())
cmd.AddCommand(CmdProposedConsumerChains())

return cmd
}
Expand Down Expand Up @@ -93,6 +94,33 @@ func CmdConsumerChains() *cobra.Command {
return cmd
}

func CmdProposedConsumerChains() *cobra.Command {
cmd := &cobra.Command{
Use: "list-proposed-consumer-chains",
Short: "Query chainIDs in consumer addition proposal before voting finishes",
Args: cobra.ExactArgs(0),
RunE: func(cmd *cobra.Command, args []string) (err error) {
clientCtx, err := client.GetClientQueryContext(cmd)
if err != nil {
return err
}
queryClient := types.NewQueryClient(clientCtx)

req := &types.QueryProposedChainIDsRequest{}
res, err := queryClient.QueryProposedConsumerChainIDs(cmd.Context(), req)
if err != nil {
return err
}

return clientCtx.PrintProto(res)
},
}

flags.AddQueryFlagsToCmd(cmd)

return cmd
}

func CmdConsumerStartProposals() *cobra.Command {
cmd := &cobra.Command{
Use: "list-start-proposals",
Expand Down
93 changes: 93 additions & 0 deletions x/ccv/provider/keeper/gov_hook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package keeper
sainoe marked this conversation as resolved.
Show resolved Hide resolved

import (
"fmt"

"github.com/cosmos/gogoproto/proto"

sdk "github.com/cosmos/cosmos-sdk/types"
govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
sdkgov "github.com/cosmos/cosmos-sdk/x/gov/types"
v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"

"github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
)

type GovHooks struct {
gk *govkeeper.Keeper
k *Keeper
}

// Implements GovHooks interface
// GovHooks exist in cosmos-sdk/x/gov/keeper/hooks.go of v0.45.16-lsm-ics and on
sainoe marked this conversation as resolved.
Show resolved Hide resolved
var _ sdkgov.GovHooks = GovHooks{}

func (k *Keeper) GovHooks(gk *govkeeper.Keeper) GovHooks {
return GovHooks{
gk: gk,
k: k,
}
}

// AfterProposalSubmission - call hook if registered
// After consumerAddition proposal submission, the consumer chainID is stored
func (gh GovHooks) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) {
p, ok := gh.gk.GetProposal(ctx, proposalID)
if !ok {
panic(fmt.Errorf("failed to get proposal %d in gov hook", proposalID))
}
msgs := p.GetMessages()

for _, msg := range msgs {
var msgLegacyContent v1.MsgExecLegacyContent
err := proto.Unmarshal(msg.Value, &msgLegacyContent)
if err != nil {
panic(fmt.Errorf("failed to unmarshal proposal content in gov hook: %w", err))
}

// if the consumer addition proposal cannot be unmarshaled, continue
var consAdditionProp types.ConsumerAdditionProposal
if err := proto.Unmarshal(msgLegacyContent.Content.Value, &consAdditionProp); err != nil {
continue
}

if consAdditionProp.ProposalType() == types.ProposalTypeConsumerAddition {
gh.k.SetProposedConsumerChain(ctx, consAdditionProp.ChainId, proposalID)
}
}
}

// AfterProposalVotingPeriodEnded - call hook if registered
// After proposal voting ends, the consumer chainID in store is deleted.
// When a proposal passes, this chainID will be available in providerKeeper.GetAllPendingConsumerAdditionProps
// or providerKeeper.GetAllConsumerChains(ctx).
func (gh GovHooks) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) {
p, ok := gh.gk.GetProposal(ctx, proposalID)
if !ok {
panic(fmt.Errorf("failed to get proposal %d in gov hook", proposalID))
}
msgs := p.GetMessages()

for _, msg := range msgs {
var msgLegacyContent v1.MsgExecLegacyContent
err := proto.Unmarshal(msg.Value, &msgLegacyContent)
if err != nil {
panic(fmt.Errorf("failed to unmarshal proposal content in gov hook: %w", err))
}

var consAdditionProp types.ConsumerAdditionProposal
// if the proposal is not ConsumerAdditionProposal, return
if err := proto.Unmarshal(msgLegacyContent.Content.Value, &consAdditionProp); err != nil {
continue
}

if consAdditionProp.ProposalType() == types.ProposalTypeConsumerAddition {
gh.k.DeleteProposedConsumerChainInStore(ctx, proposalID)
}
}
}

func (gh GovHooks) AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) {
}
func (gh GovHooks) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) {}
func (gh GovHooks) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) {}
14 changes: 14 additions & 0 deletions x/ccv/provider/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,17 @@ func (k Keeper) QueryRegisteredConsumerRewardDenoms(goCtx context.Context, req *
Denoms: denoms,
}, nil
}

func (k Keeper) QueryProposedConsumerChainIDs(goCtx context.Context, req *types.QueryProposedChainIDsRequest) (*types.QueryProposedChainIDsResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}

ctx := sdk.UnwrapSDKContext(goCtx)

chains := k.GetAllProposedConsumerChainIDs(ctx)

return &types.QueryProposedChainIDsResponse{
ProposedChains: chains,
}, nil
}
26 changes: 19 additions & 7 deletions x/ccv/provider/keeper/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,28 @@ func ValidatorConsensusKeyInUse(k *Keeper, ctx sdk.Context, valAddr sdk.ValAddre
panic("could not get validator cons addr ")
}

inUse := false

for _, validatorConsumerAddrs := range k.GetAllValidatorsByConsumerAddr(ctx, nil) {
if sdk.ConsAddress(validatorConsumerAddrs.ConsumerAddr).Equals(consensusAddr) {
inUse = true
break
allConsumerChains := []string{}
consumerChains := k.GetAllConsumerChains(ctx)
for _, consumerChain := range consumerChains {
allConsumerChains = append(allConsumerChains, consumerChain.ChainId)
}
proposedChains := k.GetAllProposedConsumerChainIDs(ctx)
for _, proposedChain := range proposedChains {
allConsumerChains = append(allConsumerChains, proposedChain.ChainID)
}
pendingChainIDs := k.GetAllPendingConsumerChainIDs(ctx)
allConsumerChains = append(allConsumerChains, pendingChainIDs...)

for _, c := range allConsumerChains {
if _, exist := k.GetValidatorByConsumerAddr(
ctx,
c,
providertypes.NewConsumerConsAddress(consensusAddr)); exist {
return true
}
}

return inUse
return false
}

func (h Hooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) error {
Expand Down
4 changes: 4 additions & 0 deletions x/ccv/provider/keeper/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func TestValidatorConsensusKeyInUse(t *testing.T) {
newValidator.ConsumerConsAddress(),
anotherValidator0.ProviderConsAddress(),
)
k.SetConsumerClientId(ctx, "chainid", "clientID")
MSalopek marked this conversation as resolved.
Show resolved Hide resolved
},
expect: true,
},
Expand All @@ -50,10 +51,13 @@ func TestValidatorConsensusKeyInUse(t *testing.T) {
newValidator.ConsumerConsAddress(),
anotherValidator0.ProviderConsAddress(),
)
k.SetConsumerClientId(ctx, "chainid0", "clientID0")

k.SetValidatorByConsumerAddr(ctx, "chainid1",
anotherValidator1.ConsumerConsAddress(),
anotherValidator1.ProviderConsAddress(),
)
k.SetConsumerClientId(ctx, "chainid1", "clientID1")
},
expect: true,
},
Expand Down
Loading
Loading