[bisq-network/bisq] Hash of ProtectedStoragePayload is non-deterministic (leading to duplication of elements in p2pstorage) (#3367)

[Justin Carter]

In [P2PDataStorage.java](https://github.com/bisq-network/bisq/blob/master/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java) there is a [hash map](https://github.com/bisq-network/bisq/blob/master/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java#L112) that should contain the up to date view of the data that gets gossiped across the network.

The keys to this map are [generated](https://github.com/bisq-network/bisq/blob/master/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java#L374-L375) from the hash of the [protobuf serialized](https://github.com/bisq-network/bisq/blob/master/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java#L842-L844) `ProtectedStoragePayload`.

Unfortunately using this hash as a key is not a good choice because it can differ for the same payload depending on how it gets serialized. Protobuf serialization is non-deterministic when it comes to [maps].(https://developers.google.com/protocol-buffers/docs/proto3#maps)

> Wire format ordering and map iteration ordering of map values is undefined, so you cannot rely on your map items being in a particular order.

And many payloads contain maps via the [extra_data field](https://github.com/bisq-network/bisq/blob/master/common/src/main/proto/pb.proto#L663).

This leads to payloads which should be considered unique ending up in the map multiple times. For example I have witnessed OfferPayloads that have the same OfferId inserted multiple times due to differing hashes due to different ordering of the map entries.

-- 
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/issues/3367
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191006/a642d7af/attachment.html>