Skip to content

Commit

Permalink
feat!: add protection against accidental downgrades (backport cosmos#…
Browse files Browse the repository at this point in the history
…10407) (cosmos#11026)

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

## Description

Closes: cosmos#10318

---

### Author Checklist

*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

### Reviewers Checklist

*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)

# Conflicts:
#	CHANGELOG.md
#	x/upgrade/keeper/keeper.go

* chore: resolve conflicts

Co-authored-by: MD Aleem <72057206+aleem1314@users.noreply.github.com>
Co-authored-by: aleem1314 <aleem@vitwit.com>
  • Loading branch information
3 people authored and JeancarloBarrios committed Sep 28, 2024
1 parent 9e03b97 commit 6907302
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 6 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,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
Expand Down
38 changes: 32 additions & 6 deletions x/upgrade/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"context"
"encoding/binary"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"sort"
Expand Down Expand Up @@ -33,12 +33,8 @@ type Keeper struct {
skipUpgradeHeights map[int64]bool // map of heights to skip for an upgrade
cdc codec.BinaryCodec // App-wide binary codec
upgradeHandlers map[string]types.UpgradeHandler // map of plan name to upgrade handler
versionModifier server.VersionModifier // implements setting the protocol version field on BaseApp
versionSetter xp.ProtocolVersionSetter // implements setting the protocol version field on BaseApp
downgradeVerified bool // tells if we've already sanity checked that this binary version isn't being used against an old state.
authority string // the address capable of executing and canceling an upgrade. Usually the gov module account
initVersionMap appmodule.VersionMap // the module version map at init genesis

consensusKeeper types.ConsensusKeeper
}

// NewKeeper constructs an upgrade Keeper which requires the following arguments:
Expand Down Expand Up @@ -324,6 +320,26 @@ func encodeDoneKey(name string, height int64) []byte {
return key
}

// GetLastCompletedUpgrade returns the last applied upgrade name and height.
func (k Keeper) GetLastCompletedUpgrade(ctx sdk.Context) (string, int64) {
iter := sdk.KVStoreReversePrefixIterator(ctx.KVStore(k.storeKey), []byte{types.DoneByte})
defer iter.Close()
if iter.Valid() {
return parseDoneKey(iter.Key()), int64(binary.BigEndian.Uint64(iter.Value()))
}

return "", 0
}

// parseDoneKey - split upgrade name from the done key
func parseDoneKey(key []byte) string {
if len(key) < 2 {
panic(fmt.Sprintf("expected key of length at least %d, got %d", 2, len(key)))
}

return string(key[1:])
}

// GetDoneHeight returns the height at which the given upgrade was executed
func (k Keeper) GetDoneHeight(ctx context.Context, name string) (int64, error) {
store := k.KVStoreService.OpenKVStore(ctx)
Expand Down Expand Up @@ -555,3 +571,13 @@ type upgradeInfo struct {
// Height has types.Plan.Height value
Info string `json:"info,omitempty"`
}

// SetDowngradeVerified updates downgradeVerified.
func (k *Keeper) SetDowngradeVerified(v bool) {
k.downgradeVerified = v
}

// DowngradeVerified returns downgradeVerified.
func (k Keeper) DowngradeVerified() bool {
return k.downgradeVerified
}
39 changes: 39 additions & 0 deletions x/upgrade/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,45 @@ func (s *KeeperTestSuite) TestLastCompletedUpgradeOrdering() {
require.NoError(err)
}

func (s *KeeperTestSuite) TestLastCompletedUpgrade() {
keeper := s.app.UpgradeKeeper
require := s.Require()

s.T().Log("verify empty name if applied upgrades are empty")
name, height := keeper.GetLastCompletedUpgrade(s.ctx)
require.Equal("", name)
require.Equal(int64(0), height)

keeper.SetUpgradeHandler("test0", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
return vm, nil
})

keeper.ApplyUpgrade(s.ctx, types.Plan{
Name: "test0",
Height: 10,
})

s.T().Log("verify valid upgrade name and height")
name, height = keeper.GetLastCompletedUpgrade(s.ctx)
require.Equal("test0", name)
require.Equal(int64(10), height)

keeper.SetUpgradeHandler("test1", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
return vm, nil
})

newCtx := s.ctx.WithBlockHeight(15)
keeper.ApplyUpgrade(newCtx, types.Plan{
Name: "test1",
Height: 15,
})

s.T().Log("verify valid upgrade name and height with multiple upgrades")
name, height = keeper.GetLastCompletedUpgrade(newCtx)
require.Equal("test1", name)
require.Equal(int64(15), height)
}

func TestKeeperTestSuite(t *testing.T) {
suite.Run(t, new(KeeperTestSuite))
}
Expand Down

0 comments on commit 6907302

Please sign in to comment.