chore: remove AllowUpdateAfterProposal (#1940)

CHANGELOG.md

@@ -73,6 +73,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (07-tendermint) [\#1097](https://github.com/cosmos/ibc-go/pull/1097) Remove `GetClientID` function from 07-tendermint `Misbehaviour` type.
 * (07-tendermint) [\#1097](https://github.com/cosmos/ibc-go/pull/1097) Deprecate `ClientId` field in 07-tendermint `Misbehaviour` type.
 * (modules/core/exported) [\#1107](https://github.com/cosmos/ibc-go/pull/1107) Merging the `Header` and `Misbehaviour` interfaces into a single `ClientMessage` type.
+* (06-solomachine)[\#1906](https://github.com/cosmos/ibc-go/pull/1906/files) Removed `AllowUpdateAfterProposal` boolean as it has been deprecated (see 01_concepts of the solo machine spec for more details).
 * (07-tendermint) [\#1896](https://github.com/cosmos/ibc-go/pull/1896) Remove error return from `IterateConsensusStateAscending` in `07-tendermint`.
 
 ### State Machine Breaking

modules/core/02-client/keeper/client_test.go

@@ -23,7 +23,7 @@ func (suite *KeeperTestSuite) TestCreateClient() {
 		expPass     bool
 	}{
 		{"success", ibctm.NewClientState(testChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), true},
-		{"client type not supported", solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}, false), false},
+		{"client type not supported", solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}), false},
 	}
 
 	for i, tc := range cases {

modules/core/02-client/keeper/keeper_test.go

@@ -190,7 +190,7 @@ func (suite *KeeperTestSuite) TestValidateSelfClient() {
 		},
 		{
 			"invalid client type",
-			solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}, false),
+			solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}),
 			false,
 		},
 		{

modules/core/02-client/legacy/v100/genesis_test.go

@@ -59,7 +59,6 @@ func (suite *LegacyTestSuite) TestMigrateGenesisSolomachine() {
 				Diversifier: clientState.ConsensusState.Diversifier,
 				Timestamp:   clientState.ConsensusState.Timestamp,
 			},
-			AllowUpdateAfterProposal: clientState.AllowUpdateAfterProposal,
 		}
 
 		// set client state

modules/core/02-client/legacy/v100/store.go

@@ -115,10 +115,9 @@ func migrateSolomachine(clientState *ClientState) *smtypes.ClientState {
 	}
 
 	return &smtypes.ClientState{
-		Sequence:                 clientState.Sequence,
-		IsFrozen:                 isFrozen,
-		ConsensusState:           consensusState,
-		AllowUpdateAfterProposal: clientState.AllowUpdateAfterProposal,
+		Sequence:       clientState.Sequence,
+		IsFrozen:       isFrozen,
+		ConsensusState: consensusState,
 	}
 }
 

modules/core/02-client/legacy/v100/store_test.go

@@ -71,7 +71,6 @@ func (suite *LegacyTestSuite) TestMigrateStoreSolomachine() {
 				Diversifier: clientState.ConsensusState.Diversifier,
 				Timestamp:   clientState.ConsensusState.Timestamp,
 			},
-			AllowUpdateAfterProposal: clientState.AllowUpdateAfterProposal,
 		}
 
 		// set client state

modules/core/02-client/types/genesis_test.go

@@ -114,7 +114,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() {
 					types.NewIdentifiedClientState(
 						soloMachineClientID, ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath),
 					),
-					types.NewIdentifiedClientState(tmClientID0, solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time}, false)),
+					types.NewIdentifiedClientState(tmClientID0, solomachinetypes.NewClientState(0, &solomachinetypes.ConsensusState{suite.solomachine.ConsensusState().PublicKey, suite.solomachine.Diversifier, suite.solomachine.Time})),
 				},
 				nil,
 				nil,

modules/core/02-client/types/proposal_test.go

@@ -117,7 +117,6 @@ func (suite *TypesTestSuite) TestUpgradeProposalValidateBasic() {
 			"fails validate abstract - empty title", func() {
 				proposal, err = types.NewUpgradeProposal("", ibctesting.Description, plan, cs.ZeroCustomFields())
 				suite.Require().NoError(err)
-
 			}, false,
 		},
 		{

modules/core/legacy/v100/genesis_test.go

@@ -86,7 +86,6 @@ func (suite *LegacyTestSuite) TestMigrateGenesisSolomachine() {
 				Diversifier: clientState.ConsensusState.Diversifier,
 				Timestamp:   clientState.ConsensusState.Timestamp,
 			},
-			AllowUpdateAfterProposal: clientState.AllowUpdateAfterProposal,
 		}
 
 		// set client state

modules/light-clients/06-solomachine/client_state.go

@@ -18,12 +18,12 @@ import (
 var _ exported.ClientState = (*ClientState)(nil)
 
 // NewClientState creates a new ClientState instance.
-func NewClientState(latestSequence uint64, consensusState *ConsensusState, allowUpdateAfterProposal bool) *ClientState {
+func NewClientState(latestSequence uint64, consensusState *ConsensusState) *ClientState {
 	return &ClientState{
-		Sequence:                 latestSequence,
-		IsFrozen:                 false,
-		ConsensusState:           consensusState,
-		AllowUpdateAfterProposal: allowUpdateAfterProposal,
+		Sequence:       latestSequence,
+		IsFrozen:       false,
+		ConsensusState: consensusState,
+		// AllowUpdateAfterProposal has been DEPRECATED. See 01_concepts in the solo machine spec repo for more details.
 	}
 }
 
@@ -75,11 +75,10 @@ func (cs ClientState) Validate() error {
 	return cs.ConsensusState.ValidateBasic()
 }
 
-// ZeroCustomFields returns solomachine client state with client-specific fields FrozenSequence,
-// and AllowUpdateAfterProposal zeroed out
+// ZeroCustomFields returns solomachine client state with client-specific field FrozenSequence zeroed out
 func (cs ClientState) ZeroCustomFields() exported.ClientState {
 	return NewClientState(
-		cs.Sequence, cs.ConsensusState, false,
+		cs.Sequence, cs.ConsensusState,
 	)
 }
 

modules/light-clients/06-solomachine/client_state_test.go

@@ -52,22 +52,22 @@ func (suite *SoloMachineTestSuite) TestClientStateValidateBasic() {
 			},
 			{
 				"sequence is zero",
-				solomachine.NewClientState(0, &solomachine.ConsensusState{sm.ConsensusState().PublicKey, sm.Diversifier, sm.Time}, false),
+				solomachine.NewClientState(0, &solomachine.ConsensusState{sm.ConsensusState().PublicKey, sm.Diversifier, sm.Time}),
 				false,
 			},
 			{
 				"timestamp is zero",
-				solomachine.NewClientState(1, &solomachine.ConsensusState{sm.ConsensusState().PublicKey, sm.Diversifier, 0}, false),
+				solomachine.NewClientState(1, &solomachine.ConsensusState{sm.ConsensusState().PublicKey, sm.Diversifier, 0}),
 				false,
 			},
 			{
 				"diversifier is blank",
-				solomachine.NewClientState(1, &solomachine.ConsensusState{sm.ConsensusState().PublicKey, "  ", 1}, false),
+				solomachine.NewClientState(1, &solomachine.ConsensusState{sm.ConsensusState().PublicKey, "  ", 1}),
 				false,
 			},
 			{
 				"pubkey is empty",
-				solomachine.NewClientState(1, &solomachine.ConsensusState{nil, sm.Diversifier, sm.Time}, false),
+				solomachine.NewClientState(1, &solomachine.ConsensusState{nil, sm.Diversifier, sm.Time}),
 				false,
 			},
 		}
@@ -76,7 +76,6 @@ func (suite *SoloMachineTestSuite) TestClientStateValidateBasic() {
 			tc := tc
 
 			suite.Run(tc.name, func() {
-
 				err := tc.clientState.Validate()
 
 				if tc.expPass {
@@ -193,7 +192,6 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() {
 
 					proof, err = suite.chainA.Codec.Marshal(signatureDoc)
 					suite.Require().NoError(err)
-
 				},
 				true,
 			},
@@ -455,7 +453,7 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() {
 						PublicKey: sm.ConsensusState().PublicKey,
 					}
 
-					clientState = solomachine.NewClientState(sm.Sequence-1, consensusState, false)
+					clientState = solomachine.NewClientState(sm.Sequence-1, consensusState)
 				},
 				false,
 			},
@@ -492,7 +490,7 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() {
 						PublicKey: sm.ConsensusState().PublicKey,
 					}
 
-					clientState = solomachine.NewClientState(sm.Sequence, consensusState, false)
+					clientState = solomachine.NewClientState(sm.Sequence, consensusState)
 				},
 				false,
 			},
@@ -666,7 +664,7 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() {
 						PublicKey: sm.ConsensusState().PublicKey,
 					}
 
-					clientState = solomachine.NewClientState(sm.Sequence-1, consensusState, false)
+					clientState = solomachine.NewClientState(sm.Sequence-1, consensusState)
 				},
 				false,
 			},
@@ -703,7 +701,7 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() {
 						PublicKey: sm.ConsensusState().PublicKey,
 					}
 
-					clientState = solomachine.NewClientState(sm.Sequence, consensusState, false)
+					clientState = solomachine.NewClientState(sm.Sequence, consensusState)
 				},
 				false,
 			},

modules/light-clients/06-solomachine/misbehaviour_test.go

@@ -112,7 +112,6 @@ func (suite *SoloMachineTestSuite) TestMisbehaviourValidateBasic() {
 			tc := tc
 
 			suite.Run(tc.name, func() {
-
 				misbehaviour := sm.CreateMisbehaviour()
 				tc.malleateMisbehaviour(misbehaviour)
 

modules/light-clients/06-solomachine/proposal_handle.go

@@ -22,10 +22,6 @@ func (cs ClientState) CheckSubstituteAndUpdateState(
 	ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore,
 	_ sdk.KVStore, substituteClient exported.ClientState,
 ) error {
-	if !cs.AllowUpdateAfterProposal {
-		return sdkerrors.Wrapf(clienttypes.ErrUpdateClientFailed, "solo machine client is not allowed to updated with a proposal")
-	}
-
 	substituteClientState, ok := substituteClient.(*ClientState)
 	if !ok {
 		return sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "substitute client state type %T, expected  %T", substituteClient, &ClientState{})

modules/light-clients/06-solomachine/proposal_handle_test.go

@@ -28,11 +28,6 @@ func (suite *SoloMachineTestSuite) TestCheckSubstituteAndUpdateState() {
 					subjectClientState.AllowUpdateAfterProposal = true
 				}, true,
 			},
-			{
-				"subject not allowed to be updated", func() {
-					subjectClientState.AllowUpdateAfterProposal = false
-				}, false,
-			},
 			{
 				"substitute is not the solo machine", func() {
 					substituteClientState = &ibctm.ClientState{}
@@ -63,7 +58,6 @@ func (suite *SoloMachineTestSuite) TestCheckSubstituteAndUpdateState() {
 				suite.SetupTest()
 
 				subjectClientState = sm.ClientState()
-				subjectClientState.AllowUpdateAfterProposal = true
 				substitute := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "substitute", "testing", 5)
 				substituteClientState = substitute.ClientState()
 

modules/light-clients/06-solomachine/spec/01_concepts.md

@@ -133,7 +133,6 @@ If the update is successful:
 An update by a governance proposal will only succeed if:
 
 - the substitute provided is parseable to solo machine client state
-- the `AllowUpdateAfterProposal` client parameter is set to `true`
 - the new consensus state public key does not equal the current consensus state public key
 
 If the update is successful:
@@ -142,6 +141,8 @@ If the update is successful:
 - the subject consensus state is updated to the substitute consensus state
 - the client is unfrozen (if it was previously frozen)
 
+NOTE: Previously, `AllowUpdateAfterProposal` was used to signal the update/recovery options for the solo machine client.  However, this has now been deprecated because a code migration can overwrite the client and consensus states regardless of the value of this parameter. If governance would vote to overwrite a client or consensus state, it is likely that governance would also be willing to perform a code migration to do the same.
+
 ## Misbehaviour
 
 Misbehaviour handling will only succeed if:

modules/light-clients/06-solomachine/update_test.go

@@ -83,7 +83,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessageHeader() {
 			{
 				"signature uses wrong sequence",
 				func() {
-
 					sm.Sequence++
 					clientMsg = sm.CreateHeader(sm.Diversifier)
 				},
@@ -125,7 +124,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessageHeader() {
 
 					clientState = cs
 					clientMsg = h
-
 				},
 				false,
 			},
@@ -341,7 +339,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessageMisbehaviour() {
 			{
 				"signatures sign over different sequence",
 				func() {
-
 					// store in temp before assigning to interface type
 					m := sm.CreateMisbehaviour()
 
@@ -470,16 +467,13 @@ func (suite *SoloMachineTestSuite) TestUpdateState() {
 						clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.Codec, suite.store, clientMsg)
 					})
 				}
-
 			})
 		}
 	}
 }
 
 func (suite *SoloMachineTestSuite) TestCheckForMisbehaviour() {
-	var (
-		clientMsg exported.ClientMessage
-	)
+	var clientMsg exported.ClientMessage
 
 	// test singlesig and multisig public keys
 	for _, sm := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
@@ -519,7 +513,6 @@ func (suite *SoloMachineTestSuite) TestCheckForMisbehaviour() {
 				} else {
 					suite.Require().False(foundMisbehaviour)
 				}
-
 			})
 		}
 	}

modules/light-clients/07-tendermint/client_state_test.go

@@ -23,9 +23,7 @@ const (
 	fiftyOneCharChainID = "123456789012345678901234567890123456789012345678901"
 )
 
-var (
-	invalidProof = []byte("invalid proof")
-)
+var invalidProof = []byte("invalid proof")
 
 func (suite *TendermintTestSuite) TestStatus() {
 	var (
@@ -162,7 +160,6 @@ func (suite *TendermintTestSuite) TestValidate() {
 }
 
 func (suite *TendermintTestSuite) TestInitialize() {
-
 	testCases := []struct {
 		name           string
 		consensusState exported.ConsensusState

modules/light-clients/07-tendermint/consensus_state_test.go

@@ -14,35 +14,44 @@ func (suite *TendermintTestSuite) TestConsensusStateValidateBasic() {
 		consensusState *ibctm.ConsensusState
 		expectPass     bool
 	}{
-		{"success",
+		{
+			"success",
 			&ibctm.ConsensusState{
 				Timestamp:          suite.now,
 				Root:               commitmenttypes.NewMerkleRoot([]byte("app_hash")),
 				NextValidatorsHash: suite.valsHash,
 			},
-			true},
-		{"success with sentinel",
+			true,
+		},
+		{
+			"success with sentinel",
 			&ibctm.ConsensusState{
 				Timestamp:          suite.now,
 				Root:               commitmenttypes.NewMerkleRoot([]byte(ibctm.SentinelRoot)),
 				NextValidatorsHash: suite.valsHash,
 			},
-			true},
-		{"root is nil",
+			true,
+		},
+		{
+			"root is nil",
 			&ibctm.ConsensusState{
 				Timestamp:          suite.now,
 				Root:               commitmenttypes.MerkleRoot{},
 				NextValidatorsHash: suite.valsHash,
 			},
-			false},
-		{"root is empty",
+			false,
+		},
+		{
+			"root is empty",
 			&ibctm.ConsensusState{
 				Timestamp:          suite.now,
 				Root:               commitmenttypes.MerkleRoot{},
 				NextValidatorsHash: suite.valsHash,
 			},
-			false},
-		{"nextvalshash is invalid",
+			false,
+		},
+		{
+			"nextvalshash is invalid",
 			&ibctm.ConsensusState{
 				Timestamp:          suite.now,
 				Root:               commitmenttypes.NewMerkleRoot([]byte("app_hash")),
@@ -51,7 +60,8 @@ func (suite *TendermintTestSuite) TestConsensusStateValidateBasic() {
 			false,
 		},
 
-		{"timestamp is zero",
+		{
+			"timestamp is zero",
 			&ibctm.ConsensusState{
 				Timestamp:          time.Time{},
 				Root:               commitmenttypes.NewMerkleRoot([]byte("app_hash")),