fix(sequencer): ensure sequencer cannot propose twice | graceful hand…

…ling of non proposer in notice queue (#1513)

x/sequencer/keeper/fraud.go

@@ -21,7 +21,7 @@ func (k Keeper) TryKickProposer(ctx sdk.Context, kicker types.Sequencer) error {
 	}
 
 	// clear the proposer
-	k.SetProposer(ctx, ra, types.SentinelSeqAddr)
+	k.abruptRemoveProposer(ctx, ra)
 
 	// TODO: refund/burn if needed
 	k.unbond(ctx, &proposer)

x/sequencer/keeper/hook_listener.go

@@ -62,7 +62,7 @@ func (hook rollappHook) OnHardFork(ctx sdk.Context, rollappID string, _ uint64)
 	}
 
 	// clear current proposer and successor
-	hook.k.SetProposer(ctx, rollappID, types.SentinelSeqAddr)
+	hook.k.abruptRemoveProposer(ctx, rollappID)
 	hook.k.SetSuccessor(ctx, rollappID, types.SentinelSeqAddr)
 
 	return nil

x/sequencer/keeper/msg_server_update.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/dymensionxyz/gerr-cosmos/gerrc"
 	"github.com/dymensionxyz/sdk-utils/utils/uevent"
 
 	"github.com/dymensionxyz/dymension/v3/x/sequencer/types"
@@ -49,6 +50,12 @@ func (k msgServer) UpdateOptInStatus(goCtx context.Context,
 		return nil, err
 	}
 
+	if seq.NoticeStarted() {
+		// prevent a sequencer who proposed in the past from becoming chosen as proposer again
+		return nil, gerrc.ErrFailedPrecondition.Wrap(`tried to change opt in status after rotating: not allowed because
+sequencers can only be proposer at most once`)
+	}
+
 	if err := seq.SetOptedIn(ctx, msg.OptedIn); err != nil {
 		return nil, err
 	}

x/sequencer/keeper/proposer.go

@@ -10,21 +10,21 @@ import (
 	"github.com/dymensionxyz/sdk-utils/utils/uevent"
 )
 
+func (k Keeper) abruptRemoveProposer(ctx sdk.Context, rollapp string) {
+	proposer := k.GetProposer(ctx, rollapp)
+	k.removeFromNoticeQueue(ctx, proposer)
+	k.SetProposer(ctx, rollapp, types.SentinelSeqAddr)
+}
+
 // OptOutAllSequencers : change every sequencer of the rollapp to be opted out.
 // Can optionally pass a list of exclusions: those sequencers won't be modified.
-func (k Keeper) optOutAllSequencers(ctx sdk.Context, rollapp string, excl ...string) error {
+func (k Keeper) optOutAllSequencers(ctx sdk.Context, rollapp string) error {
 	seqs := k.RollappSequencers(ctx, rollapp)
-	exclMap := make(map[string]struct{}, len(excl))
-	for _, addr := range excl {
-		exclMap[addr] = struct{}{}
-	}
 	for _, seq := range seqs {
-		if _, ok := exclMap[seq.Address]; !ok {
-			if err := seq.SetOptedIn(ctx, false); err != nil {
-				return errorsmod.Wrap(err, "set opted in")
-			}
-			k.SetSequencer(ctx, seq)
+		if err := seq.SetOptedIn(ctx, false); err != nil {
+			return errorsmod.Wrap(err, "set opted in")
 		}
+		k.SetSequencer(ctx, seq)
 	}
 	return nil
 }

x/sequencer/keeper/rotation_test.go

@@ -47,6 +47,54 @@ func (s *SequencerTestSuite) TestRotationHappyFlow() {
 	s.Require().False(s.k().IsSuccessor(s.Ctx, s.seq(bob)))
 }
 
+// Make sure a sequencer cannot be proposer twice
+func (s *SequencerTestSuite) TestRotationReOptInFlow() {
+	// init
+	ra := s.createRollapp()
+	s.createSequencerWithBond(s.Ctx, ra.RollappId, alice, ucoin.SimpleMul(bond, 3))
+	s.createSequencerWithBond(s.Ctx, ra.RollappId, bob, ucoin.SimpleMul(bond, 2)) // bob has prio over charlie
+	s.createSequencerWithBond(s.Ctx, ra.RollappId, charlie, ucoin.SimpleMul(bond, 1))
+	s.Require().True(s.k().IsProposer(s.Ctx, s.seq(alice)))
+	s.Require().False(s.k().IsSuccessor(s.Ctx, s.seq(bob)))
+
+	prop := alice
+	succ := bob
+
+	for range 2 {
+		// proposer tries to unbond
+		mUnbond := &types.MsgUnbond{Creator: pkAddr(prop)}
+		res, err := s.msgServer.Unbond(s.Ctx, mUnbond)
+		s.Require().NoError(err)
+
+		// notice period has not yet elapsed
+		err = s.k().ChooseSuccessorForFinishedNotices(s.Ctx, s.Ctx.BlockTime())
+		s.Require().NoError(err)
+		s.Require().False(s.k().IsSuccessor(s.Ctx, s.seq(succ)))
+
+		// proposer cannot yet submit last
+		err = s.k().OnProposerLastBlock(s.Ctx, s.seq(prop))
+		utest.IsErr(s.Require(), err, gerrc.ErrFault)
+
+		// advance clock past notice
+		s.Require().True(res.GetNoticePeriodCompletionTime().After(s.Ctx.BlockTime()))
+		s.Ctx = s.Ctx.WithBlockTime(*res.GetNoticePeriodCompletionTime())
+
+		// notice period has now elapsed
+		err = s.k().ChooseSuccessorForFinishedNotices(s.Ctx, s.Ctx.BlockTime())
+		s.Require().NoError(err)
+
+		// proposer can submit last
+		err = s.k().OnProposerLastBlock(s.Ctx, s.seq(prop))
+		s.Require().NoError(err)
+		s.Require().False(s.k().IsProposer(s.Ctx, s.seq(prop)))
+		s.Require().False(s.k().IsSuccessor(s.Ctx, s.seq(succ)))
+
+		// We can rotate Alice -> Bob but not Bob -> Alice
+		s.Require().Equal(succ == bob, s.k().IsProposer(s.Ctx, s.seq(succ)))
+		prop, succ = succ, prop
+	}
+}
+
 // A wants to rotate but there is no B to take over. Proposer should be sentinel afterwards.
 func (s *SequencerTestSuite) TestRotationNoSuccessor() {
 	s.App.RollappKeeper.SetHooks(nil)