feat(sequencer): conditional unbonding + kick for liveness #1343
Conversation
| } | ||
| if errorsmod.IsOf(err, gerrc.ErrNotFound) { | ||
| if !canonical { | ||
| // not from sequencer, and not canonical - it's not interesting |
There was a problem hiding this comment.
why to exclude this scenario? why u can't enforce it for every msg, regardless of canonical or not
There was a problem hiding this comment.
What's your suggestion, concretely? the check is saying it's not from sequencer AND it's not canonical
There was a problem hiding this comment.
my q is why AND it's not canonical required? why not return an error if it's not from a sequencer
There was a problem hiding this comment.
if it's not from sequencer then we would reject ibc updates for non rollapps
| // if containingHPlus1 is not nil then containingH also guaranteed to not be nil | ||
| type stateInfos struct { | ||
| containingH *rollapptypes.StateInfo | ||
| containingHPlus1 *rollapptypes.StateInfo |
There was a problem hiding this comment.
containingHPlus1 used to get next sequencer
can we take the NextSequencer field from the containingH state info instead? (after the HF PR)
x/sequencer/keeper/get_and_set.go
Outdated
| store := ctx.KVStore(k.storeKey) | ||
| bz := store.Get(types.SuccessorByRollappKey(rollapp)) | ||
| if bz == nil { | ||
| return k.GetSequencer(ctx, types.SentinelSeqAddr) |
There was a problem hiding this comment.
how u distinguish between No successer set (no rotation in progress), and sentinal successor?
There was a problem hiding this comment.
Why do you need to do that?
If you need to do that, awaitingLastProposerBlock
Can expose in a query if need be
| return proposer.NoticeElapsed(ctx.BlockTime()) | ||
| } |
There was a problem hiding this comment.
we discussed in the past that this is not reliable enough.
awaitingLastProposerBlock should be true only after chooseSuccessor is called
There was a problem hiding this comment.
Right, good spot, I forgot to move ChooseSuccessorForFinishedNotices to begin block!
Description
features:
changes needed:
tech debts and todos:
remarks:
Closes #1328
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
PR review checkboxes:
I have...
Unreleasedsection inCHANGELOG.mdgodoccommentsSDK Checklist
maptime.Now()sendCoinand notSendCoinsFull security checklist here
For Reviewer:
After reviewer approval: