Closed andiflabs closed 4 months ago
This looks good to me 👍
I wonder if it's possible, or desirable, to enforce that we only skip validation when deserializing from stored values.
I think it may be hard to have a general rule like that for skipping validation, but I do wonder about how we can ensure that validation always happens somewhere.
Yes, I think it's both possible and desiderable. This could even be done using types, so that mistakes would be caught at build time.
Summary
Transactions that contains mints involve parsing assets, which in turn involve parsing public addresses of the owner for those assets. Parsing a public address is currently an expensive operation, because it involves elliptic curve point multiplications.
Transactions containing mints are therefore harmful for the performance of wallet scans.
To improve the performance of wallet scans, this commit:
PublicAddress::new_unchecked
(in Rust) constructor that speeds up construction of public addresses from trusted input;skipValidation
parameter toTransaction
(in TypeScript) to allow using the "unchecked" variants of constructors;skipValidation: true
.Note that, while the goal of this commit is to improve wallet scan performance, the impact of this commit is actually broader: it impacts all reads from the chain db. This is good for performance, but may be a concern for security and stability. The assumption is that if something is stored in the chain db, then it was previously validated and therefore it can be trusted.
Testing Plan
ironfish-rust-nodejs
withyarn build:stats
(see GH-5074)yarn start wallet:rescan
wallet:rescan
process. Observe that the number of note decryptions and the number of elliptic curve point multiplications match (i.e. there are no additional multiplications performed)Documentation
N/A
Breaking Change
N/A