[bisq-network/bisq] (4/4) Update behavior of add/remove/refresh on duplicate sequence numbers (#3558)

[Julian Knutsen]

julianknutsen commented on this pull request.



>              return false;
-        }
 
         boolean hasSequenceNrIncreased = hasSequenceNrIncreased(protectedStorageEntry.getSequenceNumber(), hashOfPayload);
 
         // If we have seen a more recent operation for this payload, we ignore the current one
         // TODO: I think we can return false here. All callers use the get() public API which increments the sequence number
         // leaving only the onMessage() handler which doesn't look at the return value. It makes more intuitive sense that adds() that don't

Thanks again for looking through this.

I think my terminology may just be off here. When I refer to the "get() public API" I mean the P2PService callers that use the getProtectedStorageEntry/getRefreshTTLMessage/getMailboxDataWithSignedSeqNr APIs which grab an increasing sequence number before calling back into addProtectedStorageEntry/refreshTTL/remove/removeMailboxData. 

Maybe I should use the term "internal callers" or "P2PService users" or "non-onMessage callers" or something else to describe the code path that follows the pattern:

```
addProtectedStorageEntry(getProtectedStorageEntry());
```

What I am trying to communicate is that there are two paths through this code, the first is the onMessage handler which doesn't care about the return value so no behavior change will be noticed.

The second is the P2PService ("get() public API") which always uses an increasing sequence number so any failure paths that are due to sequence numbers won't trigger and the behavior will be the same as before.

I hope that helps communicate my intention on the deduplication of these checks.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3558#discussion_r342912009
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191105/e820a129/attachment.html>