Hash each address preimage once and reuse the address-hash map - #784
Hash each address preimage once and reuse the address-hash map#784151henry151 wants to merge 1 commit into
Conversation
Drop dual Hash160/SHA256 in AddressEntry::getHash and seed used AddressEntry objects from the warmed account hash map.
|
Hey, sorry about the delay. Having a horrible 2 weeks at work, I'll get to breathe a bit this weekend. I'll have time to review then. Thanks for your patience. |
All good, I'm a plumber full time, I get it. :) |
|
Your changeset removes the double computation when generating hashes. This is actually a feature. Paying to a mangled hash (or an address constructed from such hash) would black hole the coins. The additional cost is negligible. Your solution also pulls the hash from an AddressEntry object to then re-inject it into another AddressEntry object, introducing a method to the class to populate it with blind data. In this state, I cannot accept the PR. A better, middle ground solution would be to carry the whole AddressEntry object generated by Finally, a "perfect" solution, which would avoid the breach and mixing of duties, is to retire the address hash map from |
| getAddressHashMap / BDV registration. | ||
| ***/ | ||
|
|
||
| const_cast<AddressAccount*>(this)->updateAddressHashMap(); |
There was a problem hiding this comment.
AddressAccount isn't virtual, you can call updateAddressHashMap directly
It is fine to make getUserAddressMap non const.
| return type_; | ||
| } | ||
|
|
||
| void AddressEntry::setCachedPrefixedHash(BinaryData prefixed) |
There was a problem hiding this comment.
Don't add methods that allow injecting blind data into assets and/or addresses.
|
Thanks for the review; this weekend I'm traveling to play music at a family event, and then I'll have a busy week when I get back home -- but the following week, I'll have time to properly address the feedback. |
|
Sounds good. I still have to get RC2 to build on Windows so you should have time. Thanks for the effort, much apperciated! |
2 participants
Hash160 for used addresses was computed during walletToCapnp and again when building the BDV address map. AddressEntry::getHash also hashed each preimage twice.
Reuse the account address-hash map when building the used-address map, and hash each preimage once.
Tested with:
./gtest/WalletTests --gtest_filter=WalletsTest.UsedAddressMapReusesHashMap:WalletsTest.WalletDisplayNames— passed