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

Rotation in view change #46

Merged
merged 8 commits into from
May 15, 2020
Merged

Rotation in view change #46

merged 8 commits into from
May 15, 2020

Conversation

Gilthoniel
Copy link
Contributor

This implement a simple view change that rotates the leader every block.

@Gilthoniel Gilthoniel added the enhancement New feature or request label May 13, 2020
@Gilthoniel Gilthoniel self-assigned this May 13, 2020
@Gilthoniel Gilthoniel force-pushed the viewchange_rotation branch 4 times, most recently from 6fef1d1 to 0376f0e Compare May 14, 2020 12:52
@Gilthoniel Gilthoniel added the R4M 🚀 The PR is ready to be reviewed and merged label May 14, 2020
@Gilthoniel Gilthoniel requested a review from nkcr May 14, 2020 13:00
leader := int(prop.GetIndex()) % authority.Len()

iter := authority.AddressIterator()
iter.Seek(leader)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finally a way to reset an iterator \o/

cosi/mod.go Outdated
@@ -37,6 +37,11 @@ type Actor interface {
// CollectiveSigning is the interface that provides the primitives to sign a
// message by members of a network.
type CollectiveSigning interface {
// GetSigner returns the individual signer assigned to the instance. One
// should not used it to verify a collective signature but only for identity
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// should not used it to verify a collective signature but only for identity
// should not use it to verify a collective signature but only for identity

crypto/mod.go Outdated
Comment on lines 48 to 49
// GetNext returns the next public key only if a call to HasNext returns
// true.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// GetNext returns the next public key only if a call to HasNext returns
// true.
// GetNext returns the next public key in case HasNext returns true. If not
// no assumption can be done.

String() string
}

// AddressIterator is an iterator over the list of addresses of a membership.
type AddressIterator interface {
Seek(int)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be the same comments as in the PublicKeyIterator interface.

@@ -138,3 +176,31 @@ func (ops *operations) catchUp(target SkipBlock, addr mino.Address) error {
}
}
}

// waitBlock releases the catch up lock and wait for new blocks to be committed.
// It will return true if the expected block index exists before the timeout.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you changed the plan ^^

func (q *queue) Clear() {
q.Lock()
q.locked = false
q.items = nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't it dangerous to set it to nil? I would expect a clear to empty the list, not "invalidate" it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

learned something today

@nkcr
Copy link
Contributor

nkcr commented May 15, 2020

Will be able to merge once the tests pass again

@Gilthoniel Gilthoniel merged commit 3788a26 into master May 15, 2020
@Gilthoniel Gilthoniel deleted the viewchange_rotation branch May 15, 2020 16:39
bbjubjub2494 pushed a commit to bbjubjub2494/dela that referenced this pull request Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request R4M 🚀 The PR is ready to be reviewed and merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants