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

Enforce Pigeon minimum version in Paloma #919

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
3 changes: 3 additions & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ const (
Name = "paloma"

wasmAvailableCapabilities = "iterator,staking,stargate,paloma"

minimumPigeonVersion = "v1.4.0"
taariq marked this conversation as resolved.
Show resolved Hide resolved
)

func getGovProposalHandlers() []govclient.ProposalHandler {
Expand Down Expand Up @@ -525,6 +527,7 @@ func New(
memKeys[valsetmoduletypes.MemStoreKey],
app.GetSubspace(valsetmoduletypes.ModuleName),
app.StakingKeeper,
minimumPigeonVersion,
)

consensusRegistry := consensusmodulekeeper.NewRegistry()
Expand Down
1 change: 1 addition & 0 deletions proto/palomachain/paloma/valset/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ message MsgAddExternalChainInfoForValidatorResponse {}

message MsgKeepAlive {
string creator = 1;
string pigeonVersion = 2;
}

message MsgKeepAliveResponse {}
1 change: 1 addition & 0 deletions testutil/keeper/valset.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func ValsetKeeper(t testing.TB) (*keeper.Keeper, sdk.Context) {
memStoreKey,
paramsSubspace,
nil,
"v1.4.0",
)

ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, log.NewNopLogger())
Expand Down
4 changes: 0 additions & 4 deletions x/consensus/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ func (k msgServer) AddMessagesSignatures(goCtx context.Context, msg *types.MsgAd
); err != nil {
return nil, err
}
err := k.Keeper.valset.KeepValidatorAlive(ctx, valAddr)
if err != nil {
return nil, err
}

return &types.MsgAddMessagesSignaturesResponse{}, nil
}
5 changes: 0 additions & 5 deletions x/consensus/keeper/msg_server_add_evidence.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,5 @@ func (k msgServer) AddEvidence(goCtx context.Context, msg *types.MsgAddEvidence)
return nil, err
}

err = k.Keeper.valset.KeepValidatorAlive(ctx, valAddr)
if err != nil {
return nil, err
}

return &types.MsgAddEvidenceResponse{}, nil
}
4 changes: 0 additions & 4 deletions x/consensus/keeper/msg_server_set_public_access_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ func (k msgServer) SetPublicAccessData(goCtx context.Context, msg *types.MsgSetP
if err != nil {
return nil, err
}
err = k.Keeper.valset.KeepValidatorAlive(ctx, valAddr)
if err != nil {
return nil, err
}

return &types.MsgSetPublicAccessDataResponse{}, nil
}
2 changes: 1 addition & 1 deletion x/consensus/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ type ValsetKeeper interface {
GetSigningKey(ctx sdk.Context, valAddr sdk.ValAddress, chainType, chainReferenceID, signedByAddress string) ([]byte, error)
GetCurrentSnapshot(ctx sdk.Context) (*valsettypes.Snapshot, error)
CanAcceptValidator(ctx sdk.Context, valAddr sdk.ValAddress) error
KeepValidatorAlive(ctx sdk.Context, valAddr sdk.ValAddress) error
KeepValidatorAlive(ctx sdk.Context, valAddr sdk.ValAddress, pigeonVersion string) error
}
22 changes: 14 additions & 8 deletions x/consensus/types/mocks/ValsetKeeper.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion x/evm/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type ValsetKeeper interface {
GetCurrentSnapshot(ctx sdk.Context) (*valsettypes.Snapshot, error)
SetSnapshotOnChain(ctx sdk.Context, snapshotID uint64, chainReferenceID string) error
GetLatestSnapshotOnChain(ctx sdk.Context, chainReferenceID string) (*valsettypes.Snapshot, error)
KeepValidatorAlive(ctx sdk.Context, valAddr sdk.ValAddress) error
KeepValidatorAlive(ctx sdk.Context, valAddr sdk.ValAddress, pigeonVersion string) error
Jail(ctx sdk.Context, valAddr sdk.ValAddress, reason string) error
IsJailed(ctx sdk.Context, val sdk.ValAddress) bool
SetValidatorBalance(ctx sdk.Context, valAddr sdk.ValAddress, chainType string, chainReferenceID string, externalAddress string, balance *big.Int) error
Expand Down
10 changes: 5 additions & 5 deletions x/evm/types/mocks/ValsetKeeper.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion x/valset/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ func GetTxCmd() *cobra.Command {
}

cmd.AddCommand(CmdAddExternalChainInfoForValidator())
cmd.AddCommand(CmdKeepAlive())

return cmd
}
39 changes: 0 additions & 39 deletions x/valset/client/cli/tx_keep_alive.go

This file was deleted.

3 changes: 2 additions & 1 deletion x/valset/keeper/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ const (

ErrCannotJailValidator = whoops.Errorf("cannot jail validator: %s")

ErrValidatorAlreadyJailed = whoops.Errorf("validator already jailed: %s")
ErrValidatorAlreadyJailed = whoops.Errorf("validator already jailed: %s")
ErrValidatorPigeonOutOfDate = whoops.Errorf("validator %s pigeon is out of date. Version %s less than required %s")
)
27 changes: 18 additions & 9 deletions x/valset/keeper/keep_alive.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import (
"github.com/palomachain/paloma/util/libvalid"
"github.com/palomachain/paloma/util/slice"
"github.com/palomachain/paloma/x/valset/types"
"golang.org/x/mod/semver"
)

const (
// TODO: make this a param
defaultKeepAliveDuration = 5 * time.Minute
cValidatorJailedErrorMessage = "validator is jailed"
cValidatorNotBondedErrorMessage = "validator is not bonded"
Expand All @@ -31,14 +31,9 @@ type keepAliveData struct {
AliveUntil time.Time
}

func (k Keeper) KeepValidatorAlive(ctx sdk.Context, valAddr sdk.ValAddress) error {
if err := k.CanAcceptValidator(ctx, valAddr); err != nil {
// Make sure we allow validators to keep alive even if jailed
// or not bonded.
// https://github.com/VolumeFi/paloma/issues/254
if whoops.Is(err, ErrValidatorWithAddrNotFound) {
return err
}
func (k Keeper) KeepValidatorAlive(ctx sdk.Context, valAddr sdk.ValAddress, pigeonVersion string) error {
if err := k.CanAcceptKeepAlive(ctx, valAddr, pigeonVersion); err != nil {
return err
}

store := k.keepAliveStore(ctx)
Expand Down Expand Up @@ -83,6 +78,20 @@ func (k Keeper) ValidatorAliveUntil(ctx sdk.Context, valAddr sdk.ValAddress) (ti
return data.AliveUntil, nil
}

func (k Keeper) CanAcceptKeepAlive(ctx sdk.Context, valAddr sdk.ValAddress, pigeonVersion string) error {
stakingVal := k.staking.Validator(ctx, valAddr)

if stakingVal == nil {
return ErrValidatorWithAddrNotFound.Format(valAddr.String())
}

if semver.Compare(pigeonVersion, k.minimumPigeonVersion) < 0 {
return ErrValidatorPigeonOutOfDate.Format(valAddr.String(), pigeonVersion, k.minimumPigeonVersion)
}

return nil
}

func (k Keeper) CanAcceptValidator(ctx sdk.Context, valAddr sdk.ValAddress) error {
stakingVal := k.staking.Validator(ctx, valAddr)

Expand Down
68 changes: 67 additions & 1 deletion x/valset/keeper/keep_alive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestJailingInactiveValidators(t *testing.T) {
vali.On("GetConsAddr").Return(consAddr, nil)
ms.StakingKeeper.On("Jail", mock.Anything, consAddr)
} else {
err := k.KeepValidatorAlive(ctx.WithBlockTime(ctx.BlockTime().Add(-defaultKeepAliveDuration/2)), val)
err := k.KeepValidatorAlive(ctx.WithBlockTime(ctx.BlockTime().Add(-defaultKeepAliveDuration/2)), val, "v1.4.0")
require.NoError(t, err)
}
return vali, val
Expand All @@ -56,6 +56,72 @@ func TestJailingInactiveValidators(t *testing.T) {
require.NoError(t, err)
}

func TestCanAcceptKeepAlive(t *testing.T) {
valAddr := sdk.ValAddress("testvalidator")

testcases := []struct {
name string
inputPigeonVersion string
setup func(services mockedServices)
expectedErr error
}{
{
name: "validator not found",
inputPigeonVersion: "v1.4.0",
setup: func(services mockedServices) {
services.StakingKeeper.On("Validator", mock.Anything, mock.Anything).Return(nil)
},
expectedErr: ErrValidatorWithAddrNotFound.Format(valAddr.String()),
},
{
name: "pigeon version too low",
inputPigeonVersion: "v1.3.100",
setup: func(services mockedServices) {
services.StakingKeeper.On("Validator", mock.Anything, mock.Anything).Return(mocks.NewStakingValidatorI(t))
},
expectedErr: ErrValidatorPigeonOutOfDate.Format(valAddr.String(), "v1.3.100", "v1.4.0"),
},
{
name: "pigeon version equal, validator found",
inputPigeonVersion: "v1.4.0",
setup: func(services mockedServices) {
services.StakingKeeper.On("Validator", mock.Anything, mock.Anything).Return(mocks.NewStakingValidatorI(t))
},
},
{
name: "pigeon version major higher, validator found",
inputPigeonVersion: "v2.0.0",
setup: func(services mockedServices) {
services.StakingKeeper.On("Validator", mock.Anything, mock.Anything).Return(mocks.NewStakingValidatorI(t))
},
},
{
name: "pigeon version patch higher, validator found",
inputPigeonVersion: "v1.4.1",
setup: func(services mockedServices) {
services.StakingKeeper.On("Validator", mock.Anything, mock.Anything).Return(mocks.NewStakingValidatorI(t))
},
},
{
name: "pigeon version minor higher, validator found",
inputPigeonVersion: "v1.5.0",
setup: func(services mockedServices) {
services.StakingKeeper.On("Validator", mock.Anything, mock.Anything).Return(mocks.NewStakingValidatorI(t))
},
},
}

for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
k, ms, ctx := newValsetKeeper(t)
tt.setup(ms)

actualErr := k.CanAcceptKeepAlive(ctx, valAddr, tt.inputPigeonVersion)
require.Equal(t, tt.expectedErr, actualErr)
})
}
}

func TestUpdateGracePeriod(t *testing.T) {
k, ms, ctx := newValsetKeeper(t)
ctx = ctx.WithBlockTime(time.Unix(1000000000, 0)).WithBlockHeight(100)
Expand Down
14 changes: 9 additions & 5 deletions x/valset/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ type Keeper struct {
ider keeperutil.IDGenerator

SnapshotListeners []types.OnSnapshotBuiltListener

minimumPigeonVersion string
}

func NewKeeper(
Expand All @@ -43,18 +45,20 @@ func NewKeeper(
memKey storetypes.StoreKey,
ps paramtypes.Subspace,
staking types.StakingKeeper,
minimumPigeonVersion string,
) *Keeper {
// set KeyTable if it has not already been set
if !ps.HasKeyTable() {
ps = ps.WithKeyTable(types.ParamKeyTable())
}

k := &Keeper{
cdc: cdc,
storeKey: storeKey,
memKey: memKey,
paramstore: ps,
staking: staking,
cdc: cdc,
storeKey: storeKey,
memKey: memKey,
paramstore: ps,
staking: staking,
minimumPigeonVersion: minimumPigeonVersion,
}
k.ider = keeperutil.NewIDGenerator(keeperutil.StoreGetterFn(func(ctx sdk.Context) sdk.KVStore {
return prefix.NewStore(ctx.KVStore(k.storeKey), []byte("IDs"))
Expand Down
Loading