Fix bug with seed restore and open offers #4507
Conversation
In dev testing I got the case of duplicated address entries and thus incorrect balance. I could not reproduce it later but looking in the AddressEntry code it was unsafe using the add operation on list without a contains check. Better instead to use a HashSet and avoid possibility of duplicated entries by the chosen data structure. Note that the protobuf representation is still a list and get converted to a HashSet.
…tModificationExceptions - Make entrySet final - Avoind unneeded wrapping - Remove visibility of entrySet - Add getAddressEntriesAsListImmutable method. This is the only access for the hashSet so we ensure it cannot be changed from outside. - Remove stream() method - Remove unused return types - Improve some stream structures - Renaming, improve comments
- Add SharedPresentation class for methods in GUIUtil which are no utility methods. - Move restoreSeedWords to SharedPresentation
at restore from seed and create a backup copy. I don't know what was the motivation of removing the original file (probably the use case that the seeds are different and thus there is no match of existing entries with the new addresses) as that breaks the internal association of addresses with trades/offers and the reserved balances display. This should be tested well though with the related use cases (diff. offer and trade states, wallet restore with same seed of with foreign seed -> in that case we need to recreated the AddressEntryList but that should be handled in the related domain) - Add copyFile method
an open offer. Improve handling of restore and add more checks to avoid invalid entries.
|
ACK My dev/regtest/dao mode test scenarios:
|
Fixes #4499
When restoring from same seeds and the user had open offers the reserved balance was not updated as the addressEntry list got created newly. It is not the normal use case that a user applies the same seed to a data directory containing open offers but we should handle such cases correctly as well.
We also check now for open offers and enforce that those get removed at restore from seeds.
There have been potential bugs as well in the addressEntryList class which are hopefully eliminated by that PR. Some might have been responsible for out-of-sync addressList with wallet data issues. Those cases are hard to reproduce and mostly happen at rather un-usual situations.
Lots of refactoring as well...
I tested quite a bit but it will require good testing before deployment.