diff --git a/x/committee/keeper/keeper.go b/x/committee/keeper/keeper.go index 523ba417..59337b39 100644 --- a/x/committee/keeper/keeper.go +++ b/x/committee/keeper/keeper.go @@ -19,12 +19,10 @@ type Keeper struct { } func NewKeeper(cdc *codec.Codec, storeKey sdk.StoreKey, router govtypes.Router) Keeper { - // It is vital to seal the governance proposal router here as to not allow - // further handlers to be registered after the keeper is created since this - // could create invalid or non-deterministic behavior. - // TODO why? - // Not sealing the router because for some reason the function panics if it has already been sealed and there is no way to tell if has already been called. - // router.Seal() + // Logic in the keeper methods assume the set of gov handlers is fixed. + // So the gov router must be sealed so no handlers can be added or removed after the keeper is created. + // Note: for some reason the gov router panics if it has already been sealed, so a helper func is used to make sealing idempotent. + sealGovRouterIdempotently(router) return Keeper{ cdc: cdc, @@ -33,6 +31,13 @@ func NewKeeper(cdc *codec.Codec, storeKey sdk.StoreKey, router govtypes.Router) } } +func sealGovRouterIdempotently(router govtypes.Router) { + defer func() { + recover() + }() + router.Seal() +} + // ---------- Committees ---------- // GetCommittee gets a committee from the store. diff --git a/x/committee/keeper/proposal.go b/x/committee/keeper/proposal.go index acb604ba..4aeeac82 100644 --- a/x/committee/keeper/proposal.go +++ b/x/committee/keeper/proposal.go @@ -22,8 +22,6 @@ func (k Keeper) SubmitProposal(ctx sdk.Context, proposer sdk.AccAddress, committ } // Check proposal is valid - // TODO what if it's not valid now but will be in the future? - // TODO does this need to be before permission check? if err := k.ValidatePubProposal(ctx, pubProposal); err != nil { return 0, err } @@ -92,7 +90,6 @@ func (k Keeper) CloseOutProposal(ctx sdk.Context, proposalID uint64) sdk.Error { } func (k Keeper) ValidatePubProposal(ctx sdk.Context, pubProposal types.PubProposal) sdk.Error { - // TODO not sure if the basic validation is required - should be run in msg.ValidateBasic if pubProposal == nil { return sdk.ErrInternal("proposal is empty") } diff --git a/x/committee/types/msg.go b/x/committee/types/msg.go index 508d4a4a..74b21fa4 100644 --- a/x/committee/types/msg.go +++ b/x/committee/types/msg.go @@ -1,7 +1,5 @@ package types -// These msg types should be basically the same as for gov, but without deposits. - // MsgSubmitProposal is used by committee members to create a new proposal that they can vote on. type MsgSubmitProposal struct { // TODO