feat!: add protection against accidental downgrades (backport cosmos#…

…10407) (cosmos#11026)

* feat!: add protection against accidental downgrades (cosmos#10407)

Closes: cosmos#10318

---

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

(cherry picked from commit 5622115)

* chore: resolve conflicts

Co-authored-by: MD Aleem <72057206+aleem1314@users.noreply.github.com>
Co-authored-by: aleem1314 <aleem@vitwit.com>

CHANGELOG.md

@@ -41,6 +41,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * (grpc) [\#10985](https://github.com/cosmos/cosmos-sdk/pull/10992) The `/cosmos/tx/v1beta1/txs/{hash}` endpoint returns a 404 when a tx does not exist.
 
+### Improvements
+
+* [\#10407](https://github.com/cosmos/cosmos-sdk/pull/10407) Add validation to `x/upgrade` module's `BeginBlock` to check accidental binary downgrades
+
+
 ## [v0.45.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.0) - 2022-01-18
 
 ### State Machine Breaking

x/upgrade/abci_test.go

@@ -483,3 +483,70 @@ func TestBinaryVersion(t *testing.T) {
 		}
 	}
 }
+
+// TODO: add testcase to for `no upgrade handler is present for last applied upgrade`.
+func TestBinaryVersion(t *testing.T) {
+	var skipHeight int64 = 15
+	s := setupTest(10, map[int64]bool{skipHeight: true})
+
+	testCases := []struct {
+		name        string
+		preRun      func() (sdk.Context, abci.RequestBeginBlock)
+		expectPanic bool
+	}{
+		{
+			"test not panic: no scheduled upgrade or applied upgrade is present",
+			func() (sdk.Context, abci.RequestBeginBlock) {
+				req := abci.RequestBeginBlock{Header: s.ctx.BlockHeader()}
+				return s.ctx, req
+			},
+			false,
+		},
+		{
+			"test not panic: upgrade handler is present for last applied upgrade",
+			func() (sdk.Context, abci.RequestBeginBlock) {
+				s.keeper.SetUpgradeHandler("test0", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
+					return vm, nil
+				})
+
+				err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "Upgrade test", Plan: types.Plan{Name: "test0", Height: s.ctx.BlockHeight() + 2}})
+				require.Nil(t, err)
+
+				newCtx := s.ctx.WithBlockHeight(12)
+				s.keeper.ApplyUpgrade(newCtx, types.Plan{
+					Name:   "test0",
+					Height: 12,
+				})
+
+				req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
+				return newCtx, req
+			},
+			false,
+		},
+		{
+			"test panic: upgrade needed",
+			func() (sdk.Context, abci.RequestBeginBlock) {
+				err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "Upgrade test", Plan: types.Plan{Name: "test2", Height: 13}})
+				require.Nil(t, err)
+
+				newCtx := s.ctx.WithBlockHeight(13)
+				req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
+				return newCtx, req
+			},
+			true,
+		},
+	}
+
+	for _, tc := range testCases {
+		ctx, req := tc.preRun()
+		if tc.expectPanic {
+			require.Panics(t, func() {
+				s.module.BeginBlock(ctx, req)
+			})
+		} else {
+			require.NotPanics(t, func() {
+				s.module.BeginBlock(ctx, req)
+			})
+		}
+	}
+}

x/upgrade/keeper/keeper.go

@@ -444,3 +444,13 @@ func (k *Keeper) SetDowngradeVerified(v bool) {
 func (k Keeper) DowngradeVerified() bool {
 	return k.downgradeVerified
 }
+
+// SetDowngradeVerified updates downgradeVerified.
+func (k *Keeper) SetDowngradeVerified(v bool) {
+	k.downgradeVerified = v
+}
+
+// DowngradeVerified returns downgradeVerified.
+func (k Keeper) DowngradeVerified() bool {
+	return k.downgradeVerified
+}