Skip to content

Commit

Permalink
chore: just punish validators, don't jail them
Browse files Browse the repository at this point in the history
  • Loading branch information
maharifu committed Sep 19, 2024
1 parent 425b01b commit b13c90b
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 8 deletions.
9 changes: 4 additions & 5 deletions x/consensus/keeper/concensus_keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,16 +318,16 @@ func (k Keeper) jailValidatorsIfNecessary(

if msg.GetPublicAccessData() == nil && msg.GetErrorData() == nil {
// The message was never delivered, so we need to update the validator
// metrics with a failure and jail the validator
return k.jailValidatorsWhichMissedRelaying(sdkCtx, msg)
// metrics with a failure
return k.punishValidatorForMissingRelay(sdkCtx, msg)
}

// Otherwise, there was a delivery attempt, so only jail validators that
// missed attestation
return k.jailValidatorsWhichMissedAttestation(sdkCtx, msg)
}

func (k Keeper) jailValidatorsWhichMissedRelaying(
func (k Keeper) punishValidatorForMissingRelay(
sdkCtx sdk.Context,
msg types.QueuedSignedMessageI,
) error {
Expand Down Expand Up @@ -363,8 +363,7 @@ func (k Keeper) jailValidatorsWhichMissedRelaying(
})
}

jailMsg := fmt.Sprintf("Failed to relay message %d", msg.GetId())
return k.valset.Jail(sdkCtx, valAddr, jailMsg)
return nil
}

func (k Keeper) jailValidatorsWhichMissedAttestation(
Expand Down
21 changes: 18 additions & 3 deletions x/consensus/keeper/concensus_keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,26 @@ func TestJailValidatorsIfNecessary(t *testing.T) {
err = keeper.jailValidatorsIfNecessary(ctx, queue, mID)
require.NoError(t, err, "should not do anything")
})
t.Run("with neither error nor public access data set", func(t *testing.T) {
mID, err := keeper.PutMessageInQueue(ctx, queue, &testMsg, nil)
t.Run("with neither error nor public access data set without gas estimate", func(t *testing.T) {
mID, err := keeper.PutMessageInQueue(ctx, queue, &testMsg,
&consensus.PutOptions{RequireGasEstimation: true})
require.NoError(t, err)

ms.ValsetKeeper.On("Jail", mock.Anything, assignee, mock.Anything).Return(nil)
err = keeper.jailValidatorsIfNecessary(ctx, queue, mID)
require.NoError(t, err, "should not do anything")
})
t.Run("with neither error nor public access data set with gas estimate", func(t *testing.T) {
mID, err := keeper.PutMessageInQueue(ctx, queue, &testMsg,
&consensus.PutOptions{RequireGasEstimation: true})
require.NoError(t, err)

cq, err := keeper.getConsensusQueue(ctx, queue)
require.NoError(t, err)

err = cq.SetElectedGasEstimate(ctx, mID, 1)
require.NoError(t, err)

ms.MetrixKeeper.On("OnConsensusMessageAttested", mock.Anything, mock.Anything).Return(nil)

err = keeper.jailValidatorsIfNecessary(ctx, queue, mID)
require.NoError(t, err, "should not do anything")
Expand Down
7 changes: 7 additions & 0 deletions x/consensus/keeper/keeper_setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

type mockedServices struct {
ValsetKeeper *mocks.ValsetKeeper
MetrixKeeper *mocks.MetrixKeeper
}

func newConsensusKeeper(t testing.TB) (*Keeper, mockedServices, sdk.Context) {
Expand Down Expand Up @@ -56,8 +57,12 @@ func newConsensusKeeper(t testing.TB) (*Keeper, mockedServices, sdk.Context) {
memStoreKey,
"ConsensusParams",
)

metrixKeeper := mocks.NewMetrixKeeper(t)

ms := mockedServices{
ValsetKeeper: mocks.NewValsetKeeper(t),
MetrixKeeper: metrixKeeper,
}
k := NewKeeper(
appCodec,
Expand All @@ -68,6 +73,8 @@ func newConsensusKeeper(t testing.TB) (*Keeper, mockedServices, sdk.Context) {
nil,
)

k.AddMessageConsensusAttestedListener(metrixKeeper)

ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, logger)

// Initialize params
Expand Down
6 changes: 6 additions & 0 deletions x/consensus/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"
xchain "github.com/palomachain/paloma/internal/x-chain"
metrixtypes "github.com/palomachain/paloma/x/metrix/types"
valsettypes "github.com/palomachain/paloma/x/valset/types"
)

Expand Down Expand Up @@ -33,3 +34,8 @@ type ValsetKeeper interface {
type EvmKeeper interface {
PickValidatorForMessage(ctx context.Context, chainReferenceID string, requirements *xchain.JobRequirements) (string, string, error)
}

//go:generate mockery --name=MetrixKeeper
type MetrixKeeper interface {
OnConsensusMessageAttested(context.Context, metrixtypes.MessageAttestedEvent)
}
34 changes: 34 additions & 0 deletions x/consensus/types/mocks/MetrixKeeper.go

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

0 comments on commit b13c90b

Please sign in to comment.