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!: add a signalling mechanism for coordinated upgrades #2832

Merged
merged 39 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
65b954c
test: add minor version upgrade testing
cmwaters Oct 31, 2023
2477ef4
require version to be set
cmwaters Oct 31, 2023
1c7ad45
remove print statement
cmwaters Oct 31, 2023
f446568
use an upgrade flag for v2
cmwaters Nov 1, 2023
4ac4bdb
incorporate suggestions
cmwaters Nov 1, 2023
93a42f9
Simplify IsGreater
cmwaters Nov 2, 2023
f36db3a
add assertions that the network progresses
cmwaters Nov 6, 2023
4285736
Merge branch 'cal/minor-upgrade-testing' of github.com:celestiaorg/ce…
cmwaters Nov 6, 2023
c13fa61
Merge branch 'main' into cal/minor-upgrade-testing
cmwaters Nov 6, 2023
e14038e
Merge branch 'cal/minor-upgrade-testing' into cal/configured-upgrades
cmwaters Nov 6, 2023
a4eb85d
Apply suggestions from code review
cmwaters Nov 6, 2023
59c3b5d
fix parsing e2e_version env
cmwaters Nov 6, 2023
cfcfa5c
set global knuu timeout
cmwaters Nov 6, 2023
215a562
Merge branch 'main' into cal/minor-upgrade-testing
cmwaters Nov 6, 2023
4aa82ea
Update test/e2e/versions_test.go
cmwaters Nov 6, 2023
ebe2022
Merge branch 'main' into cal/minor-upgrade-testing
cmwaters Nov 6, 2023
4d14422
Merge branch 'cal/minor-upgrade-testing' into cal/configured-upgrades
cmwaters Nov 6, 2023
9e2ebab
add go doc to upgrade
cmwaters Nov 6, 2023
eae9549
Merge branch 'main' into cal/minor-upgrade-testing
cmwaters Nov 8, 2023
2856aaf
Merge branch 'cal/minor-upgrade-testing' into cal/configured-upgrades
cmwaters Nov 8, 2023
e36e241
define upgrade protos
cmwaters Nov 9, 2023
1365421
write out a large chunk of the logic
cmwaters Nov 9, 2023
f4e5489
resolve conflicts
cmwaters Nov 20, 2023
261ea06
Merge branch 'main' into cal/upgrade-module
cmwaters Nov 20, 2023
1b4c693
Merge branch 'main' into cal/upgrade-module
cmwaters Nov 27, 2023
1853460
use sdk.Dec instead of uint32. Write tests
cmwaters Nov 28, 2023
c81743a
wrap up unit tests
cmwaters Nov 28, 2023
4674c0f
Update x/upgrade/types/params.go
cmwaters Nov 28, 2023
0580b1b
Update proto/celestia/upgrade/v1/tx.proto
cmwaters Nov 28, 2023
9de3714
Update x/upgrade/types/params.go
cmwaters Nov 28, 2023
823c1ea
implement suggestions
cmwaters Nov 28, 2023
3d6160d
lint and rebuild protos
cmwaters Nov 28, 2023
6418b64
chore: make proto-gen
rootulp Nov 28, 2023
6156f3e
remove params and fix the threshold calculation
cmwaters Nov 29, 2023
5184c9a
Merge branch 'cal/upgrade-module' of github.com:celestiaorg/celestia-…
cmwaters Nov 29, 2023
b9349a1
Merge branch 'main' into cal/upgrade-module
cmwaters Nov 29, 2023
41f80b1
update the codec with the query server
cmwaters Nov 29, 2023
3ad204d
Merge branch 'cal/upgrade-module' of github.com:celestiaorg/celestia-…
cmwaters Nov 29, 2023
6274c98
revert the last commit
cmwaters Nov 29, 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
2 changes: 1 addition & 1 deletion app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func New(
)

app.FeeGrantKeeper = feegrantkeeper.NewKeeper(appCodec, keys[feegrant.StoreKey], app.AccountKeeper)
app.UpgradeKeeper = upgrade.NewKeeper(keys[upgradetypes.StoreKey], upgradeHeight, app.StakingKeeper, app.GetSubspace(upgradetypes.ModuleName))
app.UpgradeKeeper = upgrade.NewKeeper(keys[upgradetypes.StoreKey], upgradeHeight, app.StakingKeeper)

app.BlobstreamKeeper = *bsmodulekeeper.NewKeeper(
appCodec,
Expand Down
17 changes: 0 additions & 17 deletions proto/celestia/upgrade/v1/params.proto

This file was deleted.

14 changes: 1 addition & 13 deletions proto/celestia/upgrade/v1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,11 @@ syntax = "proto3";
package celestia.upgrade.v1;

import "google/api/annotations.proto";
import "celestia/upgrade/v1/params.proto";

option go_package = "github.com/celestiaorg/celestia-app/x/upgrade/types";

// Query defines the upgrade Query service.
service Query {
// Params allows the querying of the params
rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {
option (google.api.http).get = "/upgrade/v1/params";
}

// VersionTally allows the querying of the tally of voting power by all
// validators that have signalled for each version
rpc VersionTally(QueryVersionTallyRequest)
Expand All @@ -21,12 +15,6 @@ service Query {
}
}

// QueryParamsRequest is the request type for the Params RPC method.
message QueryParamsRequest {}

// QueryParamsResponse is the response type for the Params RPC method.
message QueryParamsResponse { Params params = 1; }

// QueryVersionTallyRequest is the request type for the UpgradeStatus RPC
// method.
message QueryVersionTallyRequest { uint64 version = 1; }
Expand All @@ -35,6 +23,6 @@ message QueryVersionTallyRequest { uint64 version = 1; }
// method.
message QueryVersionTallyResponse {
uint64 voting_power = 1;
uint64 threshold = 2;
uint64 threshold_power = 2;
uint64 total_voting_power = 3;
}
43 changes: 24 additions & 19 deletions x/upgrade/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,34 @@ import (
"github.com/celestiaorg/celestia-app/x/upgrade/types"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// Keeper implements the MsgServer and QueryServer interfaces
var (
_ types.MsgServer = Keeper{}
_ types.QueryServer = Keeper{}

defaultSignalTheshold = Fraction{Numerator: 5, Denominator: 6}
)

type Fraction struct {
Numerator int64
Denominator int64
}
Comment on lines +22 to +25
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be done later, but could we have a quick doc here for this type since its exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah okay, maybe I just make it private. There's no need to have it public as it's not queryable


// SignalThreshold is the fraction of voting power that is required
// to signal for a version change. It is set to 5/6 as the middle point
// between 2/3 and 3/3 providing 1/6 fault tolerance to halting the
// network during an upgrade period. It can be modified through a
// hard fork change that modified the app version
func SignalThreshold(version uint64) Fraction {
switch version {
default:
return defaultSignalTheshold
}
}

type Keeper struct {
// we use the same upgrade store key so existing IBC client state can
// safely be ported over without any migration
Expand All @@ -35,23 +53,18 @@ type Keeper struct {
// staking keeper is used to fetch validators to calculate the total power
// signalled to a version
stakingKeeper StakingKeeper

// paramStore provides access to the signal quorum param
paramStore paramtypes.Subspace
}

// NewKeeper constructs an upgrade keeper
func NewKeeper(
storeKey storetypes.StoreKey,
upgradeHeight int64,
stakingKeeper StakingKeeper,
paramStore paramtypes.Subspace,
) Keeper {
return Keeper{
storeKey: storeKey,
upgradeHeight: upgradeHeight,
stakingKeeper: stakingKeeper,
paramStore: paramStore,
}
}

Expand Down Expand Up @@ -100,18 +113,11 @@ func (k Keeper) VersionTally(ctx context.Context, req *types.QueryVersionTallyRe
threshold := k.GetVotingPowerThreshold(sdkCtx)
return &types.QueryVersionTallyResponse{
VotingPower: currentVotingPower.Uint64(),
Threshold: threshold.Uint64(),
ThresholdPower: threshold.Uint64(),
TotalVotingPower: totalVotingPower.Uint64(),
}, nil
}

// Params is a method required by the QueryServer interface
func (k Keeper) Params(ctx context.Context, _ *types.QueryParamsRequest) (*types.QueryParamsResponse, error) {
sdkCtx := sdk.UnwrapSDKContext(ctx)
params := k.GetParams(sdkCtx)
return &types.QueryParamsResponse{Params: &params}, nil
}

// SetValidatorVersion saves a signalled version for a validator using the keeper's store key
func (k Keeper) SetValidatorVersion(ctx sdk.Context, valAddress sdk.ValAddress, version uint64) {
store := ctx.KVStore(k.storeKey)
Expand Down Expand Up @@ -169,13 +175,12 @@ func (k Keeper) TallyVotingPower(ctx sdk.Context, threshold int64) (bool, uint64
}

// GetVotingPowerThreshold returns the voting power threshold required to
// upgrade to a new version. It converts the signal quorum parameter which
// is a number between 0 and math.MaxUint32 representing a fraction and
// then multiplies it by the total voting power
// upgrade to a new version.
func (k Keeper) GetVotingPowerThreshold(ctx sdk.Context) sdkmath.Int {
quorum := k.SignalQuorum(ctx)
// contract: totalVotingPower should not exceed MaxUit64
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[typo]

Suggested change
// contract: totalVotingPower should not exceed MaxUit64
// contract: totalVotingPower should not exceed MaxUint64

totalVotingPower := k.stakingKeeper.GetLastTotalPower(ctx)
return quorum.MulInt(totalVotingPower).RoundInt()
thresholdFraction := SignalThreshold(ctx.BlockHeader().Version.App)
return totalVotingPower.MulRaw(thresholdFraction.Numerator).QuoRaw(thresholdFraction.Denominator)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to investigate / enforce that this never overflows

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref: #2878

}

// ShouldUpgradeToV2 returns true if the current height is one before
Expand Down
2 changes: 1 addition & 1 deletion x/upgrade/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func init() {
}

const (
consensusVersion uint64 = 2
consensusVersion uint64 = 3
)

var (
Expand Down
24 changes: 0 additions & 24 deletions x/upgrade/params.go

This file was deleted.

78 changes: 19 additions & 59 deletions x/upgrade/tally_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,19 @@ import (
"cosmossdk.io/math"
"github.com/celestiaorg/celestia-app/x/upgrade"
"github.com/celestiaorg/celestia-app/x/upgrade/types"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/store"
sdk "github.com/cosmos/cosmos-sdk/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
"github.com/stretchr/testify/require"

testutil "github.com/celestiaorg/celestia-app/test/util"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
"github.com/tendermint/tendermint/libs/log"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
tmversion "github.com/tendermint/tendermint/proto/tendermint/version"
tmdb "github.com/tendermint/tm-db"
)

func TestSignalQuorum(t *testing.T) {
upgradeKeeper, ctx, _ := setup(t)
require.Equal(t, types.DefaultSignalQuorum, upgradeKeeper.SignalQuorum(ctx))
require.EqualValues(t, 100, upgradeKeeper.GetVotingPowerThreshold(ctx).Int64())
newParams := types.DefaultParams()
newParams.SignalQuorum = types.MinSignalQuorum
upgradeKeeper.SetParams(ctx, newParams)
require.Equal(t, types.MinSignalQuorum, upgradeKeeper.SignalQuorum(ctx))
require.EqualValues(t, 80, upgradeKeeper.GetVotingPowerThreshold(ctx).Int64())
}

func TestSignalVersion(t *testing.T) {
upgradeKeeper, ctx, _ := setup(t)
goCtx := sdk.WrapSDKContext(ctx)
Expand Down Expand Up @@ -63,8 +49,8 @@ func TestSignalVersion(t *testing.T) {
Version: 2,
})
require.NoError(t, err)
require.EqualValues(t, 30, res.VotingPower)
require.EqualValues(t, 100, res.Threshold)
require.EqualValues(t, 40, res.VotingPower)
require.EqualValues(t, 100, res.ThresholdPower)
require.EqualValues(t, 120, res.TotalVotingPower)
}

Expand Down Expand Up @@ -92,8 +78,8 @@ func TestTallyingLogic(t *testing.T) {
Version: 2,
})
require.NoError(t, err)
require.EqualValues(t, 30, res.VotingPower)
require.EqualValues(t, 100, res.Threshold)
require.EqualValues(t, 40, res.VotingPower)
require.EqualValues(t, 100, res.ThresholdPower)
require.EqualValues(t, 120, res.TotalVotingPower)

_, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{
Expand All @@ -106,27 +92,16 @@ func TestTallyingLogic(t *testing.T) {
Version: 2,
})
require.NoError(t, err)
require.EqualValues(t, 80, res.VotingPower)
require.EqualValues(t, 100, res.Threshold)
require.EqualValues(t, 100, res.VotingPower)
require.EqualValues(t, 100, res.ThresholdPower)
require.EqualValues(t, 120, res.TotalVotingPower)

upgradeKeeper.EndBlock(ctx)
shouldUpgrade, version := upgradeKeeper.ShouldUpgrade()
require.False(t, shouldUpgrade)
require.Equal(t, uint64(0), version)

// modify the quorum so we are right on the boundary 80/120 = 2/3
newParams := types.DefaultParams()
newParams.SignalQuorum = types.MinSignalQuorum
upgradeKeeper.SetParams(ctx, newParams)

upgradeKeeper.EndBlock(ctx)
shouldUpgrade, version = upgradeKeeper.ShouldUpgrade()
require.False(t, shouldUpgrade)
require.Equal(t, uint64(0), version)
require.EqualValues(t, 80, upgradeKeeper.GetVotingPowerThreshold(ctx).Int64())

// we now have 81/120 = 0.675 > 2/3
// we now have 101/120
_, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{
ValidatorAddress: testutil.ValAddrs[1].String(),
Version: 2,
Expand Down Expand Up @@ -156,38 +131,34 @@ func TestTallyingLogic(t *testing.T) {
Version: 2,
})
require.NoError(t, err)
require.EqualValues(t, 51, res.VotingPower)
require.EqualValues(t, 80, res.Threshold)
require.EqualValues(t, 61, res.VotingPower)
require.EqualValues(t, 100, res.ThresholdPower)
require.EqualValues(t, 120, res.TotalVotingPower)

// remove one of the validators from the set
delete(mockStakingKeeper.validators, testutil.ValAddrs[2].String())
mockStakingKeeper.totalVotingPower -= 50
delete(mockStakingKeeper.validators, testutil.ValAddrs[1].String())
mockStakingKeeper.totalVotingPower--

res, err = upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{
Version: 2,
})
require.NoError(t, err)
require.EqualValues(t, 1, res.VotingPower)
require.EqualValues(t, 47, res.Threshold)
require.EqualValues(t, 70, res.TotalVotingPower)
require.EqualValues(t, 60, res.VotingPower)
require.EqualValues(t, 99, res.ThresholdPower)
require.EqualValues(t, 119, res.TotalVotingPower)

// That validator should not be able to signal a version
_, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{
ValidatorAddress: testutil.ValAddrs[2].String(),
ValidatorAddress: testutil.ValAddrs[1].String(),
Version: 2,
})
require.Error(t, err)
}

func setup(t *testing.T) (upgrade.Keeper, sdk.Context, *mockStakingKeeper) {
upgradeStore := sdk.NewKVStoreKey(types.StoreKey)
paramStoreKey := sdk.NewKVStoreKey(paramtypes.StoreKey)
tStoreKey := storetypes.NewTransientStoreKey(paramtypes.TStoreKey)
db := tmdb.NewMemDB()
stateStore := store.NewCommitMultiStore(db)
stateStore.MountStoreWithDB(paramStoreKey, storetypes.StoreTypeIAVL, nil)
stateStore.MountStoreWithDB(tStoreKey, storetypes.StoreTypeTransient, nil)
stateStore.MountStoreWithDB(upgradeStore, storetypes.StoreTypeIAVL, nil)
require.NoError(t, stateStore.LoadLatestVersion())
mockCtx := sdk.NewContext(stateStore, tmproto.Header{
Expand All @@ -199,25 +170,14 @@ func setup(t *testing.T) (upgrade.Keeper, sdk.Context, *mockStakingKeeper) {
mockStakingKeeper := &mockStakingKeeper{
totalVotingPower: 120,
validators: map[string]int64{
testutil.ValAddrs[0].String(): 30,
testutil.ValAddrs[0].String(): 40,
testutil.ValAddrs[1].String(): 1,
testutil.ValAddrs[2].String(): 50,
testutil.ValAddrs[3].String(): 39,
testutil.ValAddrs[2].String(): 60,
testutil.ValAddrs[3].String(): 19,
},
}

registry := codectypes.NewInterfaceRegistry()
cdc := codec.NewProtoCodec(registry)

paramsSubspace := paramtypes.NewSubspace(cdc,
testutil.MakeTestCodec(),
paramStoreKey,
tStoreKey,
types.ModuleName,
)
paramsSubspace.WithKeyTable(types.ParamKeyTable())
upgradeKeeper := upgrade.NewKeeper(upgradeStore, 0, mockStakingKeeper, paramsSubspace)
upgradeKeeper.SetParams(mockCtx, types.DefaultParams())
upgradeKeeper := upgrade.NewKeeper(upgradeStore, 0, mockStakingKeeper)
return upgradeKeeper, mockCtx, mockStakingKeeper
}

Expand Down
Loading
Loading