-
Notifications
You must be signed in to change notification settings - Fork 6
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Simplify participant locking and expose current GPBFT progress
The host implementation needs to know the progress of the current instance for certificate storage and catch up purposes. As a result the current instance must be safe to read from multiple goroutines. This was achieved by a dedicated mutex that synchronised all access to the current instanceID. Separately, validation logic requires to know how far the current instance has progressed in order to effectively validate incoming messages. The changes here simplify the locking by unifying the logic for checking the progress of an instance: both validator and host can now get the latest progress from the participant that is safe for concurrent use. This is achieved by moving the progress observer mechanism out of validator and into the participant, while introducing the concept of `gpbft.Progress` a function that returns the current instance, round and phase. As part of this change, Lotus integration can now get the current round and phase as well as current instance, which is useful for self equivocation checking as well as debugging purposes. Fixes #658 Relates to #599
- Loading branch information
Showing
8 changed files
with
193 additions
and
106 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package gpbft | ||
|
|
||
| import "sync/atomic" | ||
|
|
||
| var ( | ||
| _ ProgressObserver = (*atomicProgression)(nil) | ||
| _ Progress = (*atomicProgression)(nil).Get | ||
| ) | ||
|
|
||
| // Progress gets the latest GPBFT instance progress. | ||
| type Progress func() (instance, round uint64, phase Phase) | ||
|
|
||
| // ProgressObserver defines an interface for observing and being notified about | ||
| // the progress of a GPBFT instance as it advances through different instance, | ||
| // rounds or phases. | ||
| type ProgressObserver interface { | ||
| // NotifyProgress is called to notify the observer about the progress of GPBFT | ||
| // instance, round or phase. | ||
| NotifyProgress(instance, round uint64, phase Phase) | ||
| } | ||
|
|
||
| type atomicProgression struct { | ||
| progression atomic.Pointer[progress] | ||
| } | ||
|
|
||
| type progress struct { | ||
| instance uint64 | ||
| round uint64 | ||
| phase Phase | ||
| } | ||
|
|
||
| func newAtomicProgression() *atomicProgression { | ||
| return &atomicProgression{} | ||
| } | ||
|
|
||
| func (a *atomicProgression) NotifyProgress(instance, round uint64, phase Phase) { | ||
| a.progression.Store(&progress{ | ||
| instance: instance, | ||
| round: round, | ||
| phase: phase, | ||
| }) | ||
| } | ||
|
|
||
| func (a *atomicProgression) Get() (instance, round uint64, phase Phase) { | ||
| if latest := a.progression.Load(); latest != nil { | ||
| instance, round, phase = latest.instance, latest.round, latest.phase | ||
| } | ||
| return | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package gpbft | ||
|
|
||
| import ( | ||
| "sync" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestAtomicProgression(t *testing.T) { | ||
| subject := newAtomicProgression() | ||
| t.Run("zero value", func(t *testing.T) { | ||
| instance, round, phase := subject.Get() | ||
| require.Equal(t, uint64(0), instance, "Expected initial instance to be 0") | ||
| require.Equal(t, uint64(0), round, "Expected initial round to be 0") | ||
| require.Equal(t, INITIAL_PHASE, phase, "Expected initial phase to be INITIAL_PHASE") | ||
| }) | ||
| t.Run("notify and get", func(t *testing.T) { | ||
| subject.NotifyProgress(1, 10, PREPARE_PHASE) | ||
| instance, round, phase := subject.Get() | ||
| require.Equal(t, uint64(1), instance, "Expected instance to be 1") | ||
| require.Equal(t, uint64(10), round, "Expected round to be 10") | ||
| require.Equal(t, PREPARE_PHASE, phase, "Expected phase to be PREPARE_PHASE") | ||
| }) | ||
| t.Run("notify and get progresses", func(t *testing.T) { | ||
| subject.NotifyProgress(2, 20, COMMIT_PHASE) | ||
| instance, round, phase := subject.Get() | ||
| require.Equal(t, uint64(2), instance, "Expected instance to be updated to 2") | ||
| require.Equal(t, uint64(20), round, "Expected round to be updated to 20") | ||
| require.Equal(t, COMMIT_PHASE, phase, "Expected phase to be updated to COMMIT_PHASE") | ||
| }) | ||
| t.Run("concurrent update", func(t *testing.T) { | ||
| var wg sync.WaitGroup | ||
| update := func(inst, rnd uint64, ph Phase) { | ||
| defer wg.Done() | ||
| subject.NotifyProgress(inst, rnd, ph) | ||
| } | ||
| wg.Add(2) | ||
| go update(3, 30, COMMIT_PHASE) | ||
| go update(4, 40, DECIDE_PHASE) | ||
| wg.Wait() | ||
|
|
||
| instance, round, phase := subject.Get() | ||
| require.True(t, instance == 3 || instance == 4, "Instance should match one of the updates") | ||
| require.True(t, round == 30 || round == 40, "Round should match one of the updates") | ||
| require.True(t, phase == COMMIT_PHASE || phase == DECIDE_PHASE, "Phase should match one of the updates") | ||
| }) | ||
| } |
Oops, something went wrong.