[bisq-network/bisq] Implement proper remove-before-add behavior in P2PDataStorage (#3638)

[Julian Knutsen]

New commits are at f2c535b

According to the original [design doc](https://docs.google.com/document/d/1kdZf1ZaPuj8MmtueBhv0f8ztvnfd8hDJwNO1_TWH4MM/edit#), removes-before-adds were expected and should have the behavior that a remove with a larger sequence number will block a future add with a lower sequence number.

The idea is that you don't want out of order remove messages to get lost otherwise you may do more work then necessary by adding a stale `ProtectedStorageEntry` when you could have ignored it entirely. I'm not sure of any existing cases, but this may have been the cause of 'flapping' or 'undeleted' messages as nodes were starting.

I've left the existing behavior w.r.t. broadcasting. Even if this `remove()` is the first message we have seen for a payload, we don't rebroadcast it to our peers.  I'm not exactly sure what we want here, but rebroadcasting, in this case, could help network propagation and we may want to adopt the invariant that **_ANY_** state updates caused by a message are worth of rebroadcasting because the same state updates may be relevant to our peers.
You can view, comment on, or merge this pull request online at:

  https://github.com/bisq-network/bisq/pull/3638

-- Commit Summary --

  * [PR COMMENTS] Make maxSequenceNumberBeforePurge final
  * [TESTS] Clean up 'Analyze Code' warnings
  * [REFACTOR] HashMapListener::onAdded/onRemoved
  * [REFACTOR] removeFromMapAndDataStore can operate on Collections
  * Change removeFromMapAndDataStore to signal listeners at the end in a batch
  * Update removeExpiredEntries to remove all items in a batch
  * ProposalService::onProtectedDataRemoved signals listeners once on batch removes
  * Remove HashmapChangedListener::onBatch operations
  * [TESTS] Regression test for #3629
  * [BUGFIX] Reconstruct HashMap using 32-byte key
  * [BUGFIX] Use 32-byte key in requestData path
  * [DEAD CODE] Remove getProtectedDataStoreMap
  * [TESTS] Allow tests to validate SequenceNumberMap write separately
  * Implement remove-before-add message sequence behavior

-- File Changes --

    M core/src/main/java/bisq/core/alert/AlertManager.java (32)
    M core/src/main/java/bisq/core/dao/governance/proposal/ProposalListPresentation.java (49)
    M core/src/main/java/bisq/core/dao/governance/proposal/ProposalService.java (65)
    M core/src/main/java/bisq/core/filter/FilterManager.java (31)
    M core/src/main/java/bisq/core/offer/OfferBookService.java (37)
    M core/src/main/java/bisq/core/support/dispute/agent/DisputeAgentManager.java (23)
    A core/src/test/java/bisq/core/dao/governance/proposal/ProposalServiceP2PDataStorageListenerTest.java (127)
    M p2p/src/main/java/bisq/network/p2p/P2PModule.java (1)
    M p2p/src/main/java/bisq/network/p2p/P2PService.java (13)
    M p2p/src/main/java/bisq/network/p2p/peers/getdata/RequestDataHandler.java (2)
    M p2p/src/main/java/bisq/network/p2p/storage/HashMapChangedListener.java (14)
    M p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java (113)
    M p2p/src/test/java/bisq/network/p2p/storage/P2PDataStoragePersistableNetworkPayloadTest.java (1)
    M p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageProtectedStorageEntryTest.java (59)
    M p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRemoveExpiredTest.java (14)
    M p2p/src/test/java/bisq/network/p2p/storage/P2PDataStoreDisconnectTest.java (5)
    M p2p/src/test/java/bisq/network/p2p/storage/TestState.java (188)
    M p2p/src/test/java/bisq/network/p2p/storage/mocks/ProtectedStoragePayloadStub.java (2)

-- Patch Links --

https://github.com/bisq-network/bisq/pull/3638.patch
https://github.com/bisq-network/bisq/pull/3638.diff

-- 
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/3638
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191119/95c7b762/attachment.html>