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

Implement rollup protocol #2275

Closed
wants to merge 61 commits into from
Closed

Implement rollup protocol #2275

wants to merge 61 commits into from

Conversation

louismerlin
Copy link
Contributor

What this PR does

See commit message
This PR implements the rollup protocol to replace the collect protocol, in the context of our master semester project.


🙅‍ Friendly checklist:

  • 0. Code comments are added (or updated) when/where needed and explain the WHY of the code.
  • 1. Design choices, user documentation and any additional doc are added (or updated) in READMEs.
  • 2. Any new behaviour is tested and small units of code that can be are unit tested.
  • 3. Code comments are added on tests to explain what they do.
  • 4. Errors are systematically wrapped with a meaningful message using xerrors.Errorf and the %v verb.
  • 5. Hard limit of 80 chars is always respected.
  • 6. Changes are backward compatible.
  • 7. Indentation level does not exceed 5, although 4 is already suspicious.
  • 8. Functions, files, and packages are kept to a manageable size and decomposed into smaller units if needed.
  • 9. There are no magic values.

byzcoin/profile.tmp Outdated Show resolved Hide resolved
byzcoin/rollup_tx.go Outdated Show resolved Hide resolved
byzcoin/service.go Outdated Show resolved Hide resolved
byzcoin/service.go Outdated Show resolved Hide resolved
if err != nil {
return nil, xerrors.Errorf("Error starting the protocol", err)
}
if err := <-root.DoneChan; err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps it would be safer to select with a timeout, to avoid waiting forever

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was changed

ch := s.notifications.registerForBlocks()
defer s.notifications.unregisterForBlocks(ch)

//TODO : create new block if txBuffer is not empty directly after creating another one
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do :)

byzcoin/service.go Outdated Show resolved Hide resolved
byzcoin/viewchange_test.go Outdated Show resolved Hide resolved
byzcoin/tx_pipeline.go Outdated Show resolved Hide resolved
byzcoin/rollup_tx_test.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@louismerlin louismerlin left a comment

Choose a reason for hiding this comment

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

It seems like it wasn't enough...

@ineiti
Copy link
Member

ineiti commented Jun 24, 2020

Closed in favour of #2321

@ineiti ineiti closed this Jun 24, 2020
@louismerlin
Copy link
Contributor Author

Sorry about that ! We've lacked the motivation to finish the PR now that we're both at 100% employment... We hope the new one will get through :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants