Fix dedduped hashes #53
Conversation
Instead of dedupping hashes, return all paths mapped to inputs.
| continue; | ||
| } | ||
|
|
||
| paths.push(path); | ||
| } | ||
|
|
There was a problem hiding this comment.
Just to make sure that I understand, the fix here is to be certain that the resulting array is the same length as the mtx.inputs correct?
I also think that it will be the uncommon case that the cache hit happens, but it doesn't add very much complexity to use it.
There was a problem hiding this comment.
yes.
Cache hits will happen if you are reusing addresses. So it depends on the way users use the wallet, for some it will happen more frequently (that use same address, e.g. donations)
| path = table.get(hash); | ||
| } | ||
|
|
||
| if (!path) { |
There was a problem hiding this comment.
This code block is being explicit but I think that if await this.wallet.getPath is restricted to returning null or the path, then paths.push(path) will work exacty the same without this block that checks if (!path)
There was a problem hiding this comment.
There was a problem hiding this comment.
I think it's fine to have explicit logic, especially if falsyness logic changes in that function (I doubt but still)
There was a problem hiding this comment.
Makes sense to program defensively 👍
Instead of dedupping hashes, return all paths mapped to inputs.
Proper fix after this one will be to have specialized CoinView that has
redeemandpathsin it.See: handshake-org/hsd#145