feat(autobahn): multi-epoch window for CommitQC/AppQC flow (CON-358) #3736
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| package types | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
|
|
||
| "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" | ||
| ) | ||
|
|
||
| // EpochDuo is a sliding window of up to two consecutive epochs. | ||
| // Current is always set; Prev is absent only for epoch 0. | ||
| // | ||
|
There was a problem hiding this comment. [suggestion] This is the crux of Codex's P1: the PR title/description describe an |
||
| // Next is intentionally not held: new-committee lane traffic is admitted only | ||
| // after CommitQC advances Current into the next epoch. | ||
| type EpochDuo struct { | ||
| Prev utils.Option[*Epoch] // absent if Current is epoch 0 | ||
| Current *Epoch | ||
| } | ||
|
|
||
| // ErrRoadBeforeWindow is returned by EpochForRoad when the road is older than | ||
| // WindowFirst (behind the duo). | ||
| var ErrRoadBeforeWindow = errors.New("road before epoch duo window") | ||
|
|
||
| // ErrRoadAfterWindow is returned by EpochForRoad when the road is newer than | ||
| // Current (at or past Current.Next). | ||
| var ErrRoadAfterWindow = errors.New("road after epoch duo window") | ||
|
|
||
| func (w EpochDuo) all() [2]utils.Option[*Epoch] { | ||
| return [2]utils.Option[*Epoch]{utils.Some(w.Current), w.Prev} | ||
|
There was a problem hiding this comment. [suggestion] |
||
| } | ||
|
|
||
| // EpochForRoad returns the epoch whose road range contains roadIdx. | ||
| // Prefers Current so an open-range Prev cannot shadow later epochs. | ||
| // Returns ErrRoadBeforeWindow or ErrRoadAfterWindow when out of window. | ||
|
There was a problem hiding this comment. [nit] The doc promises "Prefers Current so an open-range Prev cannot shadow later epochs" and to return |
||
| func (w EpochDuo) EpochForRoad(roadIdx RoadIndex) (*Epoch, error) { | ||
| for _, oep := range w.all() { | ||
| if ep, ok := oep.Get(); ok && ep.RoadRange().Has(roadIdx) { | ||
| return ep, nil | ||
| } | ||
| } | ||
| if roadIdx < w.WindowFirst() { | ||
| return nil, fmt.Errorf("road %d before window %v: %w", roadIdx, w, ErrRoadBeforeWindow) | ||
| } | ||
| return nil, fmt.Errorf("road %d after window %v: %w", roadIdx, w, ErrRoadAfterWindow) | ||
| } | ||
|
|
||
| // EpochOptForRoad is EpochForRoad as an Option (None when out of window). | ||
| func (w EpochDuo) EpochOptForRoad(roadIdx RoadIndex) utils.Option[*Epoch] { | ||
| if ep, err := w.EpochForRoad(roadIdx); err == nil { | ||
| return utils.Some(ep) | ||
| } | ||
| return utils.None[*Epoch]() | ||
| } | ||
|
|
||
| // CurrentForRoad returns Current when roadIdx is in Current's range; else None. | ||
| // Unlike EpochOptForRoad, Prev is never admitted. | ||
| func (w EpochDuo) CurrentForRoad(roadIdx RoadIndex) utils.Option[*Epoch] { | ||
| if w.Current != nil && w.Current.RoadRange().Has(roadIdx) { | ||
| return utils.Some(w.Current) | ||
| } | ||
| return utils.None[*Epoch]() | ||
| } | ||
|
|
||
| // WindowFirst is the earliest road still in Prev|Current. | ||
| func (w EpochDuo) WindowFirst() RoadIndex { | ||
| if prev, ok := w.Prev.Get(); ok { | ||
| return prev.RoadRange().First | ||
| } | ||
| return w.Current.RoadRange().First | ||
| } | ||
|
|
||
| // EpochForIndex returns Current or Prev by epoch index. | ||
| func (w EpochDuo) EpochForIndex(idx EpochIndex) (*Epoch, error) { | ||
| if w.Current != nil && w.Current.EpochIndex() == idx { | ||
| return w.Current, nil | ||
| } | ||
| if prev, ok := w.Prev.Get(); ok && prev.EpochIndex() == idx { | ||
| return prev, nil | ||
| } | ||
| return nil, fmt.Errorf("epoch %d not in window %v", idx, w) | ||
| } | ||
|
Comment on lines
+57
to
+81
There was a problem hiding this comment. 🟡 In Extended reasoning...
Despite that, No code path can actually produce an The Step-by-step: (1) Fix: drop |
||
|
|
||
| // String returns a compact description of the epoch indices in the window. | ||
| func (w EpochDuo) String() string { | ||
| s := "epochs [" | ||
| sep := "" | ||
| for _, oep := range w.all() { | ||
| if ep, ok := oep.Get(); ok { | ||
| s += fmt.Sprintf("%s%d", sep, ep.EpochIndex()) | ||
| sep = ", " | ||
| } | ||
| } | ||
| return s + "]" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| package types_test | ||
|
|
||
| import ( | ||
| "errors" | ||
| "testing" | ||
|
|
||
| "github.com/sei-protocol/sei-chain/sei-tendermint/autobahn/types" | ||
| "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" | ||
| ) | ||
|
|
||
| func testDuoEpochs(t *testing.T) (prev, current *types.Epoch) { | ||
| t.Helper() | ||
| rng := utils.TestRng() | ||
| weights := map[types.PublicKey]uint64{} | ||
| for range 3 { | ||
| weights[types.GenSecretKey(rng).Public()] = 1 | ||
| } | ||
| committee := utils.OrPanic1(types.NewCommittee(weights)) | ||
| prev = types.NewEpoch(0, types.RoadRange{First: 0, Next: 100}, utils.GenTimestamp(rng), committee, 1) | ||
| current = types.NewEpoch(1, types.RoadRange{First: 100, Next: 200}, utils.GenTimestamp(rng), committee, 101) | ||
| return prev, current | ||
| } | ||
|
|
||
| func TestEpochForRoad_HitsCurrentEpoch(t *testing.T) { | ||
| _, current := testDuoEpochs(t) | ||
| w := types.EpochDuo{Current: current} | ||
| ep, err := w.EpochForRoad(150) | ||
| if err != nil { | ||
| t.Fatalf("EpochForRoad(150): %v", err) | ||
| } | ||
| if ep != current { | ||
| t.Fatalf("got %v, want current", ep) | ||
| } | ||
| } | ||
|
|
||
| func TestEpochForRoad_HitsPrevEpoch(t *testing.T) { | ||
| prev, current := testDuoEpochs(t) | ||
| w := types.EpochDuo{Prev: utils.Some(prev), Current: current} | ||
| ep, err := w.EpochForRoad(50) | ||
| if err != nil { | ||
| t.Fatalf("EpochForRoad(50): %v", err) | ||
| } | ||
| if ep != prev { | ||
| t.Fatalf("got %v, want prev", ep) | ||
| } | ||
| } | ||
|
|
||
| func TestEpochForRoad_OutsideWindowReturnsError(t *testing.T) { | ||
| _, current := testDuoEpochs(t) | ||
| w := types.EpochDuo{Current: current} | ||
| _, err := w.EpochForRoad(999) | ||
| if !errors.Is(err, types.ErrRoadAfterWindow) { | ||
| t.Fatalf("EpochForRoad(999) = %v, want ErrRoadAfterWindow", err) | ||
| } | ||
| _, err = w.EpochForRoad(50) | ||
| if !errors.Is(err, types.ErrRoadBeforeWindow) { | ||
| t.Fatalf("EpochForRoad(50) current-only = %v, want ErrRoadBeforeWindow", err) | ||
| } | ||
| } | ||
|
|
||
| func TestEpochForRoad_OpenRangePrevDoesNotMaskCurrent(t *testing.T) { | ||
| rng := utils.TestRng() | ||
| weights := map[types.PublicKey]uint64{types.GenSecretKey(rng).Public(): 1} | ||
| committee := utils.OrPanic1(types.NewCommittee(weights)) | ||
| openEpoch := types.NewEpoch(0, types.OpenRoadRange(), utils.GenTimestamp(rng), committee, 1) | ||
| current := types.NewEpoch(1, types.RoadRange{First: 100, Next: 200}, utils.GenTimestamp(rng), committee, 101) | ||
| w := types.EpochDuo{Prev: utils.Some(openEpoch), Current: current} | ||
| ep, err := w.EpochForRoad(150) | ||
| if err != nil { | ||
| t.Fatalf("EpochForRoad(150): %v", err) | ||
| } | ||
| if ep.EpochIndex() != current.EpochIndex() { | ||
| t.Fatalf("got epoch %d (Prev with OpenRoadRange masked Current), want current (%d)", | ||
| ep.EpochIndex(), current.EpochIndex()) | ||
| } | ||
| } | ||
|
|
||
| func TestEpochForRoad_AbsentPrevSkipped(t *testing.T) { | ||
| _, current := testDuoEpochs(t) | ||
| w := types.EpochDuo{Current: current} | ||
| _, err := w.EpochForRoad(50) | ||
| if !errors.Is(err, types.ErrRoadBeforeWindow) { | ||
| t.Fatalf("EpochForRoad(50) with absent Prev = %v, want ErrRoadBeforeWindow", err) | ||
| } | ||
| } | ||
|
|
||
| func TestEpochForRoad_BeforeAndAfterWithPrev(t *testing.T) { | ||
| prev, current := testDuoEpochs(t) | ||
| w := types.EpochDuo{Prev: utils.Some(prev), Current: current} | ||
| if _, err := w.EpochForRoad(50); err != nil { | ||
| t.Fatalf("EpochForRoad(50) in prev: %v", err) | ||
| } | ||
| if _, err := w.EpochForRoad(150); err != nil { | ||
| t.Fatalf("EpochForRoad(150) in current: %v", err) | ||
| } | ||
| _, err := w.EpochForRoad(200) | ||
| if !errors.Is(err, types.ErrRoadAfterWindow) { | ||
| t.Fatalf("EpochForRoad(200) = %v, want ErrRoadAfterWindow", err) | ||
| } | ||
| } | ||
|
|
||
| func TestWindowFirst_WithPrev(t *testing.T) { | ||
| prev, current := testDuoEpochs(t) | ||
| w := types.EpochDuo{Prev: utils.Some(prev), Current: current} | ||
| if got, want := w.WindowFirst(), prev.RoadRange().First; got != want { | ||
| t.Fatalf("WindowFirst() = %d, want %d", got, want) | ||
| } | ||
| } | ||
|
|
||
| func TestWindowFirst_CurrentOnly(t *testing.T) { | ||
| _, current := testDuoEpochs(t) | ||
| w := types.EpochDuo{Current: current} | ||
| if got, want := w.WindowFirst(), current.RoadRange().First; got != want { | ||
| t.Fatalf("WindowFirst() = %d, want %d", got, want) | ||
| } | ||
| } | ||
|
|
||
| func TestEpochOptForRoad(t *testing.T) { | ||
| prev, current := testDuoEpochs(t) | ||
| w := types.EpochDuo{Prev: utils.Some(prev), Current: current} | ||
| if ep, ok := w.EpochOptForRoad(50).Get(); !ok || ep != prev { | ||
| t.Fatalf("EpochOptForRoad(50) = %v, want prev", ep) | ||
| } | ||
| if w.EpochOptForRoad(999).IsPresent() { | ||
| t.Fatal("EpochOptForRoad(999) should be None") | ||
| } | ||
| } | ||
|
|
||
| func TestCurrentForRoad(t *testing.T) { | ||
| prev, current := testDuoEpochs(t) | ||
| w := types.EpochDuo{Prev: utils.Some(prev), Current: current} | ||
| if ep, ok := w.CurrentForRoad(150).Get(); !ok || ep != current { | ||
| t.Fatalf("CurrentForRoad(150) = %v, want current", ep) | ||
| } | ||
| if w.CurrentForRoad(50).IsPresent() { | ||
| t.Fatal("CurrentForRoad(50) must not admit Prev") | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] Codex flagged this line as a prompt-injection vector to be removed. I disagree: it's a benign, on-topic style guideline consistent with the comment cleanups elsewhere in this PR, with no instruction directed at a reviewer/agent. Recommend keeping it. (Flagging here only to record the resolution of Codex's point.)