-
Notifications
You must be signed in to change notification settings - Fork 166
[hardfork] updating proof-systems to 0.2.0 and dropping support for setFeeWithSnarkCost #2657
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
base: develop-3.0
Are you sure you want to change the base?
Conversation
| setFeePerSnarkCost(newFeePerSnarkCost: number) { | ||
| let { totalTimeRequired } = getTotalTimeRequired(transaction.accountUpdates); | ||
| return this.setFee(new UInt64(Math.round(totalTimeRequired * newFeePerSnarkCost))); | ||
| }, |
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.
@Trivo25 Is it alright if I completely drop this function?
I'm afraid some people might be using it, but because we drop the concept of "snark cost" in the hard fork, supporting this is pretty impossible without it becoming a no-op.
I'd prefer for this to be a breaking change over a non-breaking one (maybe this is where changelog would be appropriate)
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.
hm ideally we keep it but replace it with a newer, easier way to calculate the fee - newFeePerSnarkCost * accountUpdate should be fine
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 did this here https://github.com/o1-labs/o1js/pull/2668/files
|
This also fixes: o1-labs/mina-rust#1676 |
3d1a790 to
4e402f7
Compare
b47a1fa to
3808513
Compare
|
CI is broken because this PR is missing from We reconfigured proof-systems to work properly, but now we aren't building it properly in |
title, this branch is temporary and meant to demonstrate the minimal subset of changes that need to happen to get a green build on o1js with mesa.
We're using:
mina#mesaand
proof-systems#0.2.0With a custom branch splicing the two together.