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

Julian Knutsen notifications at github.com
Mon Nov 11 17:15:23 UTC 2019


The request got stale and polluted with merge commits and has now been updated with master. Chimp's utAck outlined the risk of possible subtle changes to the client behavior since we may return false value to the client code now when it may previously have returned true.

After more review of the client caller code, I don't believe there is risk to client callers. All client code paths generate new Entrys that have sequence numbers greater than any previously seen sequence number (old + 1). This means that any new behavior in the P2PDataStore that requires increasing sequence numbers won't have any effect on those client callers since it isn't possible to generate a non-valid sequence number (with the old or new checks).

Instead, this new behavior changes what happens when messages are received from either peer nodes or seed nodes that have sequence numbers identical to those that have been seen before. It was possible to allow adds-after-removes in the old code which would have re-added data and signaled listeners.

Since this is a fundamental change to the P2P message handling logic. I encourage a merge when things are stable and there is no release pressure as suggested. I've done my best to add thorough tests, validate it in my regtest cluster, and run a production node with my changes and audit the logs to catch any glaring issues, but I've only been looking at this code for 3 weeks and don't have nearly the breadth of knowledge as the other developers who have touched this code recently.

Thanks for considering this request and I am happy to answer any other questions that come up.

-- 
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#issuecomment-552532480
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191111/cad32451/attachment.html>


More information about the bisq-github mailing list