Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core/consensus: rename ShouldRun #2463

Merged
merged 2 commits into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions core/consensus/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ type instanceIO struct {

// MarkParticipated marks the instance as participated.
// It returns an error if the instance was already marked as participated.
func (io *instanceIO) MarkParticipated() error {
func (io instanceIO) MarkParticipated() error {
select {
case <-io.participated:
return errors.New("already participated")
Expand All @@ -168,7 +168,7 @@ func (io *instanceIO) MarkParticipated() error {

// MarkProposed marks the instance as proposed.
// It returns an error if the instance was already marked as proposed.
func (io *instanceIO) MarkProposed() error {
func (io instanceIO) MarkProposed() error {
select {
case <-io.proposed:
return errors.New("already proposed")
Expand All @@ -179,10 +179,9 @@ func (io *instanceIO) MarkProposed() error {
return nil
}

// ShouldRun checks the current status of the instance.
// If the instance is not already running, it returns true and marks the instance as running.
// It returns false if the instance is already running.
func (io *instanceIO) ShouldRun() bool {
// MaybeStart returns true if the instance wasn't running and has been started by this call,
// otherwise it returns false if the instance was started in the past and is either running now or has completed.
func (io instanceIO) MaybeStart() bool {
select {
case <-io.running:
return false
Expand Down Expand Up @@ -367,7 +366,7 @@ func (c *Component) propose(ctx context.Context, duty core.Duty, value proto.Mes
}
}()

if !inst.ShouldRun() { // Participate was already called, instance is running.
if !inst.MaybeStart() { // Participate was already called, instance is running.
return <-inst.errCh
}

Expand All @@ -392,7 +391,7 @@ func (c *Component) Participate(ctx context.Context, duty core.Duty) error {
return errors.Wrap(err, "participate consensus", z.Any("duty", duty))
}

if !inst.ShouldRun() {
if !inst.MaybeStart() {
return nil // Instance already running.
}

Expand Down
20 changes: 10 additions & 10 deletions core/consensus/component_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,14 +454,14 @@ func TestComponentHandle(t *testing.T) {
}
}

func TestInstanceIO_ShouldRun(t *testing.T) {
t.Run("ShouldRun for new instance", func(t *testing.T) {
func TestInstanceIO_MaybeStart(t *testing.T) {
t.Run("MaybeStart for new instance", func(t *testing.T) {
inst1 := newInstanceIO()
require.True(t, inst1.ShouldRun())
require.False(t, inst1.ShouldRun())
require.True(t, inst1.MaybeStart())
require.False(t, inst1.MaybeStart())
})

t.Run("ShouldRun after handle", func(t *testing.T) {
t.Run("MaybeStart after handle", func(t *testing.T) {
var c Component
c.deadliner = testDeadliner{}
c.mutable.instances = make(map[core.Duty]instanceIO)
Expand All @@ -483,8 +483,8 @@ func TestInstanceIO_ShouldRun(t *testing.T) {

inst, ok := c.mutable.instances[duty]
require.True(t, ok)
require.True(t, inst.ShouldRun())
require.False(t, inst.ShouldRun())
require.True(t, inst.MaybeStart())
require.False(t, inst.MaybeStart())
})

t.Run("Call Propose after handle", func(t *testing.T) {
Expand Down Expand Up @@ -512,14 +512,14 @@ func TestInstanceIO_ShouldRun(t *testing.T) {

pubkey := testutil.RandomCorePubKey(t)

// Propose should internally mark instance as running by calling inst.ShouldRun().
// Propose should internally mark instance as running by calling inst.MaybeStart().
err = c.Propose(ctx, duty, core.UnsignedDataSet{pubkey: testutil.RandomCoreAttestationData(t)})
require.Error(t, err) // It should return an error as no peers are specified.

// Check if ShouldRun is called before.
// Check if MaybeStart is called before.
inst, ok := c.mutable.instances[duty]
require.True(t, ok)
require.False(t, inst.ShouldRun())
require.False(t, inst.MaybeStart())
})
}

Expand Down
Loading