mirrors/bisq
1
0
Fork 0
mirror of https://github.com/bisq-network/bisq.git synced 2025-02-24 15:10:44 +01:00
Code Issues Projects Releases Packages Wiki Activity

Update behavior of P2PDataStorage::addProtectedStorageEntry() on duplicates

Browse source 
Now returns false on duplicate sequence numbers. This matches more of
the expected behavior for an add() function when the element previously exists.

The only callers are either P2PService users that always increment the
sequence number or the onMessage() handler which doesn't verify the return
so there will be no visible change other than the increased readability
of the code and deduplication of the code paths.
This commit is contained in:
Julian Knutsen 2019-11-03 18:58:05 -08:00
parent 86c8c839d1
commit c1ad6b408b
No known key found for this signature in database
GPG key ID: D85F536DB3615B2D
2 changed files with 8 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java
View file

@ -390,28 +390,19 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
return false;
}
// TODO: Combine with hasSequenceNrIncreased check, but keep existing behavior for now
if(!isSequenceNrValid(protectedStorageEntry.getSequenceNumber(), hashOfPayload))
// If we have seen a more recent operation for this payload, we ignore the current one
if(!hasSequenceNrIncreased(protectedStorageEntry.getSequenceNumber(), hashOfPayload))
return false;
// Verify the ProtectedStorageEntry is well formed and valid for the add operation
if (!checkPublicKeys(protectedStorageEntry, true) || !checkSignature(protectedStorageEntry))
return false;
boolean containsKey = map.containsKey(hashOfPayload);
// If we have already seen an Entry with the same hash, verify the new Entry has the same owner
if (containsKey && !checkIfStoredDataPubKeyMatchesNewDataPubKey(protectedStorageEntry.getOwnerPubKey(), hashOfPayload))
// In a hash collision between two well formed ProtectedStorageEntry, the first item wins and will not be overwritten
if (map.containsKey(hashOfPayload) &&
!checkIfStoredDataPubKeyMatchesNewDataPubKey(protectedStorageEntry.getOwnerPubKey(), hashOfPayload)) {
return false;
boolean hasSequenceNrIncreased = hasSequenceNrIncreased(protectedStorageEntry.getSequenceNumber(), hashOfPayload);
// If we have seen a more recent operation for this payload, we ignore the current one
// TODO: I think we can return false here. All callers use the Client API (addProtectedStorageEntry(getProtectedStorageEntry())
// leaving only the onMessage() handler which doesn't look at the return value. It makes more intuitive sense that adds() that don't
// change state return false.
if (!hasSequenceNrIncreased)
return true;
}
// This is an updated entry. Record it and signal listeners.
map.put(hashOfPayload, protectedStorageEntry);

5
p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageTest.java
View file

@ -737,12 +737,11 @@ public class P2PDataStorageTest {
}
// TESTCASE: Adding duplicate payload w/ same sequence number
// TODO: Should adds() of existing sequence #s return false since they don't update state?
@Test
public void addProtectedStorageEntry_duplicateSeqNrGt0() throws CryptoException {
ProtectedStorageEntry entryForAdd = this.getProtectedStorageEntryForAdd(1);
doProtectedStorageAddAndVerify(entryForAdd, true, true);
doProtectedStorageAddAndVerify(entryForAdd, true, false);
doProtectedStorageAddAndVerify(entryForAdd, false, false);
}
// TESTCASE: Adding duplicate payload w/ 0 sequence number (special branch in code for logging)
@ -750,7 +749,7 @@ public class P2PDataStorageTest {
public void addProtectedStorageEntry_duplicateSeqNrEq0() throws CryptoException {
ProtectedStorageEntry entryForAdd = this.getProtectedStorageEntryForAdd(0);
doProtectedStorageAddAndVerify(entryForAdd, true, true);
doProtectedStorageAddAndVerify(entryForAdd, true, false);
doProtectedStorageAddAndVerify(entryForAdd, false, false);
}
// TESTCASE: Adding duplicate payload for w/ lower sequence number