-
Notifications
You must be signed in to change notification settings - Fork 206
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
Use latest cpu cost types #645
Conversation
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 looks like this breaks the tests. The testout has the log:
!!!!! Unable to upgrade Soroban Config Settings. Stopping all services. !!!!!
Yeah I ended up marking this as a draft. The number of cost types is dependent on protocol, so a change like this doesn't work if all of the images aren't on the same protocol version (latest is still on 21). |
We could make it so there can be different files for different protocol versions, that probably wouldn't be too difficult. But it also would be fine to wait to merge this until latest reaches 22 which is zero effort. |
Yeah I was considering a file for each protocol version available. The issue with waiting until 22 is that we'll just run into the same issue for 23. |
Oh. I guess for anyone running their network QuickStart on an old protocol version we'd want to keep files for old protocols too. Because you can specify the protocol version. So different files for each protocol since 20, or 21 at least makes sense 👍 |
We made some phase 1 cost type changes outside of a protocol boundary that don't get picked up by quickstart, so add them to the upgrade file. This should give testers more accurate instruction counts.