Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
CIP-0110? | Plutus v1 compatible script references #679
CIP-0110? | Plutus v1 compatible script references #679
Changes from 7 commits
1c1acf7
3c1de02
fca44d0
e5ab31a
db32e6c
9326d26
f000947
45c42f7
71c8739
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
My analysis showed that PlutusV1 scripts account for 41.3% and only 0.5% for PlutusV2 of the total size of blockchain since beginning of Alonzo, which means it is even lower that when taking Byron through Mary into consideration. So, 60% is definitely an overestimation. That being said 40% is still a lot.
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.
"bytes taken up by constantly re-publishing the same Plutus v1 scripts is nearly 60% of each block"
I am probably misunderstanding this, but this does not make sense to me. How can PlutusV1 take up 60% of a block if in total it takes up 40% of the blockchain size since PlutusV1 was introduced?
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.
It's 40% of the full chain (i.e. of the 151gb of on-disk storage for a fully sync'd node, at the time this was taken, 60gb of that was redundant scripts).
However, looking at an average over the last X time window (I think 1 month?), SMAUG found that:
So it's exactly as you said: considering the full chain history, it's around 40%, because of the time before smart contracts were even a thing; but on average, on a continuing basis, you can expect around 60% of each block to be taken up by a plutus v1 script.
These are just rough motivating numbers, and it may be closer to 50% or 70%, depending on which time frame you measure in; The point was to express the severity of the problem, and that it's not going away, not to give a high precision measurement.
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.
So, that is the statistics over 1 month. That should either not be present in this CIP or made it absolutely clear that it is just a statistic from an arbitrary period of in the past.
First of all it is 41.3% since smart contracts where a thing. See my analysis.
Second of all, this math makes no sense. "40% of blockchain consists of PlutusV1 scripts" literally means on average every block will be 40% PlutusV1 scripts. You can't cherry pick a small period with higher usage and use it as an average.
So, could you please remove that 60% statistic from the CIP, since it is not only confusing but also does not serve as a good representative of the overall usage pattern.
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.
So, what are we trying to achieve here?
Your analysis mentioned that 60gb of overhead was due to redundant plutus v1 scripts; The current on-disk storage for all eras was 151gb (I don't have an updated view). 60gb is 39.7% of 151gb, or about 40%.
It feels like you're picking a fight over me using the phrase "about 40%", instead of an exact 41.3%? What does the extra significant digits there help achieve? Does it materially change the point, or the motivation for adopting the CIP?
As for the 60%, my wording clearly says end of 2023; I'm happy to make it clearer, but I think a statistic that talks about the existing, ongoing usage of the chain, which normalizes for recent trends, is an important view of the picture. The point being that the problem isn't going away: for example, it would be one thing if the first 3 months of Alonzo produced 59gb, and then in all the time since, we've produced an additional gigabyte.
I'm not cherry-picking a small period, I just picked a recent period, and the implication that I'm being deceptive or dishonest is insulting, when your own analysis supports the same conclusion: that this is a problem, and needs to be addressed.
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.
I reworked the explanations to hopefully make it clearer.
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.
No, not at all. ~40% is fine. Also I am not trying to pick a fight. I am trying to make sure we have an accurate picture here.
It is like saying: "Last December the average temperature was -10C, so we can conclude that for the next few years that should be the average temperature every month."
So, what I am saying is the 60% average is wrong. There are no numbers backing up this "average" usage statistic, while there are clear numbers showing that the number is approximately 40%. I hope it is more clear now what I am trying to say.
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.
Also, I am not implying that you are being deceptive. There is no need to get defensive, because no one is trying to attack you here. We are all on the same team. All I am trying to do is point out a flaw in the CIP.
So, just to make it really clear, I am trying to say that using a small sample to infer a bigger picture is wrong, especially when there is a clear and accurate analysis available. That's all.
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.
Why do we need to fail on reference scripts in regular spending inputs?
Whenever a regular input includes a reference script it constitutes a usable reference script in that transaction, so it feels strange and inconsistent to disable just that functionality. Especially considering that the user will have to pay extra for spending that output starting with Conway, since it contains a reference script.
I would suggest we just translate the TxOut while omitting translation of the reference script. In other words it would look like a regular TxOut to PlutusV1 script.
I do agree about the inline datums, we should still fail translation on their presence in the inputs.
@michaelpj, @zliu41 and @Quantumplation any objections to allowing reference scripts in spending inputs?
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.
I'm fine with any relaxing of these rules; This part was written when I was unsure whether even the most gentle version of this was going to be accepted.
When I've raised this in the past, the response I got was that any hiding of information about the transaction when constructing the script context was considered dangerous and wouldn't be supported.
Hence, my original proposal was about including reference scripts in the witness set, which isn't directly visible to the script context.
It was quite a pleasant surprise when MPJ suggested that we just allow reference inputs like normal and leave them off the script context.
So, I have no objection to further relaxations if the ledger team themselves are ok with it, just understand that I was writing this from a posture of "what has the best chance of getting the ledger teams approval, and thus makes the minimal amount of semantic changes to plutus v1" 😅
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.
It's worth noting that I'm also fine with not relaxing this, as I can't think of any scenario where this buys us dApp developers any benefit. i.e. it's largely immaterial to me. Whereas the ability to satisfy a script witness via a reference input is hugely beneficial.
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.
😄
You got our full approval on this CIP.
I personally would prefer to have consistency. So, if we are enabling reference scripts for PlutusV1, then they should work in the same way as they do for PlutusV2 and PlutusV3
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.
Yeah, I appreciate the effort by @Quantumplation to make the minimal relaxation possible to allow the key usecase. Generally I do think that's the way to go.
However, I agree with @lehins that if we're going to allow reference scripts to be used in V1 we might as well put in the "whole feature". It's not much worse and otherwise it's a weird corner case.