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

[Julian Knutsen]

These patches changs some unexpected behavior when handling operations that have duplicate sequence numbers.

Now, if no internal state is changed the entry points will always return false.

It also fixes a subtle behavior of remove() where the code would allow duplicate add() operations if a
remove was processed with an equivalent sequence number.

Now, all operations require an increasing sequence number to update any internal state. I think this is a much easier way to think about the system, but I have no historical context for if/when these operations required equivalent sequence numbers between add() and remove().

I think it also closes a hole where a malicious node could rebroadcast an add() as a remove() and have it succeed. There are still other holes, but it moves it in the right direction.

This is the highest risk change in this stack.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * [TESTS] Add tests for P2PDataStorage in order to safely refactor
  * [BUGFIX] Call correct remove function in addMailboxData failure path.
  * [REFACTOR] P2PDataStorage::addPersistableNetworkPayload()
  * [REFACTOR] P2PDataStorage::addProtectedStorageEntry()
  * [REFACTOR] P2PDataStorage::refreshTTL()
  * [REFACTOR] P2PDataStorage::remove()
  * [REFACTOR] P2PDataStorage::removeMailboxData()
  * Update behavior of P2PDataStorage::addProtectedStorageEntry() on duplicates
  * Update behavior of P2PDataStorage::refreshTTL() on duplicates
  * Update behavior of P2PDataStorage::remove() & removeMailboxData() on duplicate sequence #s

-- File Changes --

    M build.gradle (1)
    M core/src/main/java/bisq/core/alert/Alert.java (2)
    M p2p/src/main/java/bisq/network/p2p/P2PService.java (10)
    M p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java (350)
    M p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageTest.java (1485)
    A p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java (42)
    A p2p/src/test/java/bisq/network/p2p/storage/mocks/DateTolerantPayloadStub.java (50)
    A p2p/src/test/java/bisq/network/p2p/storage/mocks/PersistableNetworkPayloadStub.java (43)
    A p2p/src/test/java/bisq/network/p2p/storage/mocks/ProtectedDataStoreServiceFake.java (45)

-- Patch Links --

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