[bisq-network/bisq] Limit max. nr. of PersistableNetworkPayload and ProtectedStorageEntries (#3562)

[Julian Knutsen]

julianknutsen commented on this pull request.



> @@ -504,6 +504,37 @@ public boolean remove(ProtectedStorageEntry protectedStorageEntry,
         return result;
     }
 
+
+    /**
+     * This method must be called only from client code not from network messages! We omit the ownership checks
+     * so we must apply it only if it comes from our trusted application code. It is used from client code which detects
+     * that the domain object violates specific domain rules.
+     * We could make it more generic by adding an Interface with a generic validation method.
+     *
+     * @param protectedStorageEntry     The entry to be removed
+     */
+    public void removeInvalidProtectedStorageEntry(ProtectedStorageEntry protectedStorageEntry) {

This leads to data structure inconsistency in the event that two consumers listen for the same Payload type and one consumer calls into this function to remove the data. It also breaks encapsulation having the consumers know about the internals of the P2PDataStore. This isn't perfect now, anyway, but moving towards a model where P2PDataStore is private to P2PService may be a good goal.

It seems like a cleaner way to do the same thing would be to have the P2PDataStore validate each stored entry prior to becoming "active" or signaling listeners. The ProtectedStorageEntry and/or payload could implement an interface that would do this check and remove the data before it was every available for consumers.

Is that what you were thinking with the "generic validation method"? Doing this would dovetail well with my existing refactoring and I can probably try it out and see how it looks. I think it would give a lot more flexibility for the future if there are certain Entrys or Payloads that we need to purge but made it past prior validation (see https://github.com/bisq-network/bisq/blob/0ab156f2496d1dda2dd0543b9653bdd126280f76/p2p/src/test/java/bisq/network/p2p/storage/payload/ProtectedMailboxStorageEntryTest.java#L96-L109 as an example).

-- 
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/3562#pullrequestreview-314565540
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191109/fd197ca7/attachment.html>