[bisq-network/bisq] [BUGFIX] #3629 (Duplicate payloads w/ 20-byte and 32-byte keys) (#3636)

[Julian Knutsen]

New commits start at 849155a

fixes #3629

Hotfix Needed: No

There was a bug when reconstructing the in-memory HashMap for `ProtectedStoragePayload` objects implementing the `PersistablePayload` object. This had two related bugs:

1) The in-memory map would end up with duplicate Payloads if a node rebuilt a ProtectedStorageEntry from the 20-byte key, then received the 32-byte key from the seednode during initialization or from an update on the P2P network.

2) An initializing node would resync known payloads from the seednode since the `Payload` existed with both the 20-byte and 32-byte key.

This PR cleans up the usages so that all outward users see the 32-byte key and all internal users can use the existing 20-byte key for persistence. This design is much safer than transitioning all persistent users to 32-byte keys and writing on-disk upgrade code.

All external listeners of Payloads don't look at the key. So, although they will receive 2 onAdded() calls instead of 1, they do the right thing by excluding the second payload since it is identical. I don't think there is any reason to publish a hotfix for this.

## Upgrade behavior
**New Seednode <--> Old Client**
The seednode will only have 1 32-byte key for each unique payload, but the client node may still have a 20-byte key that it will send as a known key. The seednode will ignore this known key, send back the 32-byte key `Payload` and the client node will update its map with the 32-byte key. This mimics existing behavior.

**Old Seednode <--> New Client**
The seednode will have 1 20-byte key and 1 32-byte key for the same unique payload. The client will only have the 32-byte key and will send it is a known key. The seednode will transmit the 20-byte payload back to the client who will ignore it.

**New Seednode <--> New Client**
Both the seednode and the client will only have 1 32-byte key for each unique payload. The client will include its 32-byte key as a known key which will match the key in the seednode map. No duplicate `Payload` objects will be transmitted.
You can view, comment on, or merge this pull request online at:

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

-- 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

-- 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 (89)
    M p2p/src/test/java/bisq/network/p2p/storage/P2PDataStoragePersistableNetworkPayloadTest.java (1)
    M p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageProtectedStorageEntryTest.java (17)
    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 (184)
    M p2p/src/test/java/bisq/network/p2p/storage/mocks/ProtectedStoragePayloadStub.java (2)

-- Patch Links --

https://github.com/bisq-network/bisq/pull/3636.patch
https://github.com/bisq-network/bisq/pull/3636.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/3636
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191119/d7a6f5d9/attachment.html>