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

Add support for acknowledgments #130

Closed
wants to merge 3 commits into from
Closed

Conversation

emilbayes
Copy link
Contributor

I want to make a scheme where I can be reasonably certain that n-peers have received a block before I communicate any confidence that a block is safely "committed". This PR goes with the new flag in the protocol handshake and asks peers to acknowledge any blocks they receive. Of course this does not make any guarantees about byzantine peers, so you should only trust the ack if you trust the peer.

Still needs more tests.

@pfrazee
Copy link
Contributor

pfrazee commented Jan 8, 2018

I'm 👍 for the idea of this PR (haven't done code review).

(An aside on why I'm interested in this.) I'm personally still debating whether multiwriter is the right solution for handling multiple devices, or if we should only be using multiwriter for collaboration between multiple people. If we had a 'safe commit' protocol, we could have users reuse the same publishingkeys between devices, and then require checkins with managernodes before publishing updates to a log. This has both upsides and downsides.

Upsides:

  • Easier to communicate to users how to manage their keys; they can just copy the keyfiles from one device to another.
  • With multiwriter, there's a master hypercore which appoints new writers. If we use multiwriter to manage many users' devices, then we'll need a solution for what happens if the master-core is lost. (That's a downside for using multiwriter to manage user devices.)

Downsides:

  • You can make writes offline, but you can't publish/share the data until you've synced with your managernode. If you make writes that conflict, you'll need to roll them back and reapply them. (You can use a 'conflict' states like in multiwriter when writes conflict.)
  • More conceptual complexity, because there are now two syncing systems: safe commits and multiwriter.
  • Depends on managernodes, which hurts the automagic decentralization.

I'm not convinced of my argument but I wanted to record it.

@martinheidegger
Copy link
Contributor

Noting conversation with @mafintosh here:

@emilbayes didn’t want to to land it before adding more tests. There might be some functionality missing also.

bnewbold added a commit to pfrazee/DEPs that referenced this pull request Jan 20, 2019
@martinheidegger
Copy link
Contributor

@emilbayes what is missing here?

@emilbayes
Copy link
Contributor Author

@martinheidegger this looks pretty stale, but it is also lacking tests

@ghost ghost mentioned this pull request Jul 5, 2019
@mafintosh
Copy link
Contributor

landed in latest

@mafintosh mafintosh closed this Aug 7, 2019
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.

4 participants