Skip to content

Commit

Permalink
[BUGFIX] Reconstruct HashMap using 32-byte key
Browse files Browse the repository at this point in the history
Addresses the first half of bisq-network#3629 by ensuring that the reconstructed
HashMap always has the 32-byte key for each payload.

I feel this fix is less risky than writing ondisk upgrade code throughout
the TempProposalStore and other disk-writing classes that are currently
untested and hard to test.

The end result is that the persistence code in P2PDataStorage still use
the 20-byte key, but the HashMap is reconstructed from disk using
the 32-byte key of the payload which minimizes the affected classes and
gives the same outward behavior.

Important to note that until all seednodes receive this update, nodes
will continue to have both the 20-byte and 32-byte keys in their HashMap.
  • Loading branch information
julianknutsen committed Nov 19, 2019
1 parent 849155a commit 68bc791
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,13 @@ public synchronized void readFromResources(String postFix) {
protectedDataStoreService.readFromResources(postFix);
resourceDataStoreService.readFromResources(postFix);

map.putAll(protectedDataStoreService.getMap());
map.putAll(
protectedDataStoreService.getMap().entrySet().stream().collect(
Collectors.toMap(
mapEntry -> get32ByteHashAsByteArray(mapEntry.getValue().getProtectedStoragePayload()),
Map.Entry::getValue)
)
);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,8 +575,7 @@ protected Class<ProtectedStorageEntry> getEntryClass() {

// Tests that just apply to PersistablePayload objects

// XXXBUG_3629XXX: Persisted ProtectedStorageEntries are saved to disk via their 20-byte hash. This causes
// the internal hash map to be reloaded with the 20-byte key instead of the 32-byte key.
// TESTCASE: Ensure the HashMap is the same before and after a restart
@Test
public void addProtectedStorageEntry_afterReadFromResourcesWithDuplicate_3629RegressionTest() {
ProtectedStorageEntry protectedStorageEntry = this.getProtectedStorageEntryForAdd(1);
Expand All @@ -585,9 +584,8 @@ public void addProtectedStorageEntry_afterReadFromResourcesWithDuplicate_3629Reg
Map<P2PDataStorage.ByteArray, ProtectedStorageEntry> beforeRestart = this.testState.mockedStorage.getMap();

this.testState.simulateRestart();

// Should be equal
Assert.assertNotEquals(beforeRestart, this.testState.mockedStorage.getMap());

Assert.assertEquals(beforeRestart, this.testState.mockedStorage.getMap());
}
}

Expand Down

0 comments on commit 68bc791

Please sign in to comment.