diff --git a/x/sequencer/keeper/fraud.go b/x/sequencer/keeper/fraud.go index 2cfd7d258..28141ea1b 100644 --- a/x/sequencer/keeper/fraud.go +++ b/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) diff --git a/x/sequencer/keeper/hook_listener.go b/x/sequencer/keeper/hook_listener.go index 3c0e65f4c..ce47dc3c8 100644 --- a/x/sequencer/keeper/hook_listener.go +++ b/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 diff --git a/x/sequencer/keeper/msg_server_update.go b/x/sequencer/keeper/msg_server_update.go index 6c2cb2a5c..76decfc54 100644 --- a/x/sequencer/keeper/msg_server_update.go +++ b/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 } diff --git a/x/sequencer/keeper/proposer.go b/x/sequencer/keeper/proposer.go index 8e5bdcbeb..e388f28de 100644 --- a/x/sequencer/keeper/proposer.go +++ b/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 } diff --git a/x/sequencer/keeper/rotation_test.go b/x/sequencer/keeper/rotation_test.go index da1cc1c7e..fb53d41f1 100644 --- a/x/sequencer/keeper/rotation_test.go +++ b/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)