-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Automatic reference script #449
Conversation
aab87b6
to
785cc17
Compare
src/Cooked/Skeleton.hs
Outdated
-- reference scripts and compare their hashes, thus will slightly reduce | ||
-- performance, especially when handling a lot of utxos. | ||
-- | ||
-- Defaut is 'True'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be False
by default if this really slows down things by default?
Or is there an optimization to be found such as setting a flag or adding an entry in a map of known ref scripts (script hash to tx out ref) in the chain state when a utxo with a ref script is produced? Could this improve performance by not going through everything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From our current test suite, the overhead is minimal, also it takes a slightly bit more time. I have thought about such optimization, but it has drawbacks:
- it duplicates information
- This map would be hard to maintain because utxos disappear when consumed. So whenever a transaction is validated, we would have to check whether utxos from this map have been consumed to remove them, but only do that in case of successful validation of the transaction. We would also have to manually add them to the map when they are created.
Overall I believe the current solution is better (because simpler) unless we discover at some point that is really hurts performance.
Also, if we want to address possible future performance issues, we know that this is an area that can offer some improvement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this is debatable. This optimization is clearly doable in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, duplicated information is not a problem. The overhead in terms of space is minimal (there will never be more than a handful of entries in that map and even if we had millions, what is a few MB of ram). However we already often find execution time to be a bit long. Not on one single trace, but in a test suite / audit test suite it quickly adds up to annoying seconds.
On the one hand, space is not really an issue, and there are never a lot of ref scripts anyways so the map will always be small. On the other hand, having a lot of utxos is not uncommon at all: big initial distributions, or sometimes we even do stress tests with a huge number of utxos during certain specific attack attempts.
Therefore I think in terms of scaling, the alternative implementation with a map of known ref scripts and their location is way better.
I would be in favor of either:
- switch to the second implementation, even if it involves a few additions during validation to update the map
- OR keep the current implementation for the time being but turn it off by default and explain the tradeoff in the documentation. I don't think this feature is needed right away. It could be an experimental feature that is disabled by default until we implement the optimized version.
This PR will close #447 but allowing to implicit search for correct reference scripts in known utxos. In the process, this will also close #438.
In practice, this means that putting any script on utxos will be sufficient for cooked to find those scripts and use them as reference whenever required. In particular, putting reference scripts in the initial distribution will be even more useful.
A drawback of this approach is that there will be no visual difference between redeeming something with of without a reference script. To compensate, a new log entry is generated whenever cooked assigns a reference script by itself.
This automated addition of reference scripts can be turned off and it will still be possible to manually provide a reference input with a script attached when redeeming a script.
This is rebase on #450