diff --git a/x/consensus/keeper/concensus_keeper.go b/x/consensus/keeper/concensus_keeper.go index af207556..bab1e3fd 100644 --- a/x/consensus/keeper/concensus_keeper.go +++ b/x/consensus/keeper/concensus_keeper.go @@ -318,8 +318,8 @@ 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 @@ -327,7 +327,7 @@ func (k Keeper) jailValidatorsIfNecessary( return k.jailValidatorsWhichMissedAttestation(sdkCtx, msg) } -func (k Keeper) jailValidatorsWhichMissedRelaying( +func (k Keeper) punishValidatorForMissingRelay( sdkCtx sdk.Context, msg types.QueuedSignedMessageI, ) error { @@ -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( diff --git a/x/consensus/keeper/concensus_keeper_test.go b/x/consensus/keeper/concensus_keeper_test.go index 83134e3c..8a0db77f 100644 --- a/x/consensus/keeper/concensus_keeper_test.go +++ b/x/consensus/keeper/concensus_keeper_test.go @@ -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") diff --git a/x/consensus/keeper/keeper_setup_test.go b/x/consensus/keeper/keeper_setup_test.go index c7e3995b..d35109db 100644 --- a/x/consensus/keeper/keeper_setup_test.go +++ b/x/consensus/keeper/keeper_setup_test.go @@ -22,6 +22,7 @@ import ( type mockedServices struct { ValsetKeeper *mocks.ValsetKeeper + MetrixKeeper *mocks.MetrixKeeper } func newConsensusKeeper(t testing.TB) (*Keeper, mockedServices, sdk.Context) { @@ -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, @@ -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 diff --git a/x/consensus/types/expected_keepers.go b/x/consensus/types/expected_keepers.go index 747a98b9..f60b9db3 100644 --- a/x/consensus/types/expected_keepers.go +++ b/x/consensus/types/expected_keepers.go @@ -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" ) @@ -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) +} diff --git a/x/consensus/types/mocks/MetrixKeeper.go b/x/consensus/types/mocks/MetrixKeeper.go new file mode 100644 index 00000000..496aa7a8 --- /dev/null +++ b/x/consensus/types/mocks/MetrixKeeper.go @@ -0,0 +1,34 @@ +// Code generated by mockery v2.43.2. DO NOT EDIT. + +package mocks + +import ( + context "context" + + types "github.com/palomachain/paloma/x/metrix/types" + mock "github.com/stretchr/testify/mock" +) + +// MetrixKeeper is an autogenerated mock type for the MetrixKeeper type +type MetrixKeeper struct { + mock.Mock +} + +// OnConsensusMessageAttested provides a mock function with given fields: _a0, _a1 +func (_m *MetrixKeeper) OnConsensusMessageAttested(_a0 context.Context, _a1 types.MessageAttestedEvent) { + _m.Called(_a0, _a1) +} + +// NewMetrixKeeper creates a new instance of MetrixKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMetrixKeeper(t interface { + mock.TestingT + Cleanup(func()) +}) *MetrixKeeper { + mock := &MetrixKeeper{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +}