Skip to content
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

Zk Rollup Milestone2 Modifying #248

Closed
wants to merge 1 commit into from

Conversation

ashWhiteHat
Copy link
Contributor

The substrate has ethereum compatibility because of frontier so it's not necessary to implement some milestone.
I modified some milestones.
Please confirm and merge.
Thank you!

@CLAassistant
Copy link

CLAassistant commented Feb 4, 2021

CLA assistant check
All committers have signed the CLA.

@mmagician
Copy link
Contributor

@noctrlz Before approving this PR, I would like to check the first milestone in more depth. I'm currently working on it and you should hear back from me in a couple of days.

@mmagician mmagician added the on hold There is an external blocker, such as another grant in progress. label Feb 11, 2021
@alxs
Copy link
Contributor

alxs commented Mar 12, 2021

Hi @noctrlz. This significantly reduces the amount of work for this milestone, which I think should be reflected in the price as well.

@mmagician
Copy link
Contributor

@noctrlz @alxs Let's first close the first milestone before we proceed with this one.

@mmagician
Copy link
Contributor

As briefly discussed in the evaluation notes for M1, I think it doesn't make much sense to continue with the grant in its current form.
The original intention was to implement the zksync components as substrate pallets, which:

  1. Clearly is a larger task than building a CLI as proposed in this amendment, as it involves re-writing the core of zksync's logic (operator, prover) to be substrate-runnable.
  2. Given the outcome of M1 which showcases that re-using zksync's setup works, I agree with this PR's description that it's good enough (and preferable) to keep using a well-tested & proven architecture, rather than re-implementing it all yourself. Right now, I also don't see any great advantages of native substrate integration.

Since this amendment would completely change the scope of the original contract, I am closing it.

If you wish to apply for a separate grant focusing on CLI and other improvements to the setup, you are welcome to do so. In such case, I'd please ask you to first submit an amendment to the original contract, removing milestones 2 & 3, just to keep things clean.

Thanks for your continued interest in the Polkadot ecosystem and congratulations again on completing the first milestone!

@mmagician mmagician closed this Mar 22, 2021
@SotaWatanabe
Copy link
Contributor

@mmagician I just came back from other work to ZK Rollups. I agree with you. In the current form, the scope is changed dramatically and it doesn't make sense. I will talk internally and get it back to you.

@SotaWatanabe
Copy link
Contributor

Hello @mmagician I am sorry for being late. During the implementation of the milestone1, we realized that implementing a ZK Rollups pallet was possible but it would be an over implementation. I am sorry. This was identified during the implementation of the milestone1.

At this moment, our best proposal is to implement CLI instead of a pallet. This option reduces implementation costs and makes it easier for Substrate developers to use ZK Rollups.

There are many missing pieces on ZK Rollups and it is originally designed to be implemented on Ethereum. For example, ZkSync (made by Matter Labs) supports geth but it can’t be used for Substrate because some dependencies haven’t been supported for Substrate. Hence, we would like to make the missing peaces instead of making this complicated.

I think the best way to do so it to terminate the milestone2 and resubmit another proposal separately.

It would be great if we could hear your opinion on this.

@mmagician
Copy link
Contributor

It's understandable that the requirements change as you dig deeper into the proposals, thanks for being open about it.
That sounds reasonable regarding M2 - I've prepared a PR that removes M2 from the application. We will look forward to your new proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold There is an external blocker, such as another grant in progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants