Initial implementation and tests

[RangerMauve]

Fixes # .

Changes proposed in this pull request:

@consento-org/maintainer

[martinheidegger]

Shouldn't this be always n-1?

[RangerMauve]

If we do n-1 can we avoid ambiguity? Like, if we have A,B,C,D,E, if A,B,C have signed, and D/E aren't connected to each other / accept, they'll both think they're the final needed signature and cause a fork.

[martinheidegger]

If n-1 doesn't work in that case it doesn't work in the other case, as two devices can mutually remove each other, it would also create that ambiguity.

[martinheidegger]

We had a conversation about allowing forks but maybe resolving equal forks?! In example: D & E approve R1 creating Forks R1D and R1E and if they sync and notice equality they decide for the alphabetically lower one (R1D).

[martinheidegger]

Ugh, endless recursion possible, right? 😅

[RangerMauve]

Err, what's the condition that causes it?

[martinheidegger]

Nothing at the moment, but my eyes look with stress 💦 on the code and I would be constantly worried if it introduces endless recursions.

[RangerMauve]

Is there a different approach I should take here? The current structure recurses until all the feeds have been processed (more specifically, until no items are processable).

[martinheidegger]

There should be a validate template here to validate if the feed data that is added keeps being added. I think this is important to figure out even in an early stage because it allows us to figure out if we can identify a valid from a corrupt feed.

[RangerMauve]

Would this require the signvector stuff?

[martinheidegger]

The sign-vectors are part of this too. Yes, but there is a much simpler case: if there is a feed of a member that has been removed or was never added. Also, there is a thin possibility that the sign-vectors fail (and a fork is created). i.e. malfunctioning of a client: Fork detection should also be part of it.

Note: The central log may be a tool to make it easier/possible to detect member-feed forks.

[RangerMauve]

Hmm, mind pseudocodeing how the verification would look and what the test for forking would look like?

[martinheidegger]

function isIncomingFeedForked (localFeed, feed) {
  // Look at the merkle tree and see if the feed was forked at a previous step
}

function isIncomingFeedSignVectorCorrect (feed, startIndex) {
  let vector = createSignVectorFor(feed.get(startIndex))
  for (let i = startIndex+1; i<feed.length; i++) {
    if (!signVector.validate(feed.get(i))) return false
  }
  return true
}

function isIncomingFeedCorrect (feed) {
  // testing "correct-ness" of requests, i.e. proper msgpack/protobuf object; size limit not exceeded; removes feed that actually existed. Does the signature exist for the request? etc.
}

function verifyFeed (localFeed, feed, startIndex, peer) {
  if (isIncomingFeedForked(localFeed, feed)) {
    // Red alert
    requestRemoveMember({ cause: 'fork-detected' })
    return false
  }
  if (!isIncomingFeedSignVectorCorrect(feed, startIndex)) {
    addToBlackList(peer)
    return false
  }
  if(!isIncomingFeedCorrect(feed)) {
    // orange alert
    requestRemoveMember({ cause: 'incorrect-client' })
    return false
  }
  return true
}

[RangerMauve]

Sweet, thank you!

[martinheidegger]

Shouldn't processFeeds() be part of the syncing process? What advantage do we get of separating those?

[RangerMauve]

Main thing here was the auto-accept function meant that processing feeds would trigger member adds, and I was hoping to have more control of when that happened. I can refactor to work around that so that control of accepting could be outside of syncing feeds.

Other consideration was that I wanted a place for members to process requests and responses that they generated themselves 1. for the initial member case and 2. to remove the need for extra logic for that bit. Down to add proccesFeeds at the end of sync, though.

[martinheidegger]

I can refactor to work around that so that control of accepting could be outside of syncing feeds.

Structurally it is probably for the best, though I don't think we need to use manual confirmation in our first real scenario. The reason why I brought this up is because in the sync process it may be possible to avoid some chance of forks by having one member sync first and process the entries before syncing everything - including the need entries - with the other peer.

[RangerMauve]

Yeah, I'm thinking that I'll have a getPendingRequests() method and a acceptPendingRequests()

[martinheidegger]

Memo: This needs to be established in the final solution using a log statement.

[RangerMauve]

Could you elaborate on this point?

[martinheidegger]

Setting a finished = true statement works for this simulation but in an actual case we should probably have a log statement that looks like { request: 'abcd', finished: true } in order to recover the state from the log itself.

[RangerMauve]

Hmm, not 100% sure what you mean regarding recovering state from the log. If we don't persist request state anywhere we'd need to traverse the entire log again and that means that finished would be derived from there again.

[RangerMauve]

Working on refactoring to change how processFeeds works, and then I'll add more tests

[martinheidegger]

Without having a central feed its quite challenging for me to figure out if a fork happened. Is there a good way to verify that without a central log?

Another thing: When adding feeds, the information should not just contain their ID's but also the verification vector for the first block

[RangerMauve]

Without a central feed there is no forking.

Since everyone needs to agree on a history and you derive members by traversing each others feeds and verifying that signatures happened. If someone tires to fork their own member feed (assuming they bypass the signvector layer of security) then at most they can prevent new peers from getting a valid history (until the fork is detected and the invalid feed is purged).

Agreed on the verification vector stuff. I'll look at incorporating sign vectors after this initial refactor.

[RangerMauve]

TODO:

• Add sign vectors
• Verify sign vectors in sync
• When removing specify index to remove at so that they won't be processed after
• Add tests for forks
• Add tests fo rconflicting requests
• Add tests so that external peers always process feeds in the same way

[RangerMauve]

Added some sims, so far so good.

Talking with Martin last week, we determined we could be running into issues if we allow members to have multiple in-flight requests.

Gonna write up some thoughts on how that works and make some diagrams of what it would look like.

[martinheidegger]

We can merge it for now. more progress would be good to be made in issues.