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

combine UpdateL2Key and DepositToNew? #194

Open
0xmountaintop opened this issue Nov 15, 2021 · 3 comments
Open

combine UpdateL2Key and DepositToNew? #194

0xmountaintop opened this issue Nov 15, 2021 · 3 comments

Comments

@0xmountaintop
Copy link
Member

We only need one of them. And if having them both it will affect them performance.

So should we use
UpdateL2Key or DepositToNew?

@lispc
Copy link
Member

lispc commented Nov 15, 2021

?? i did not know we had updatel2key

@noel2004
Copy link
Member

noel2004 commented Nov 15, 2021

For data availability, the encoded public data may become too large if a deposit / transfer data need to comply with a L2 key (254bit). So it would be a better practice to divide a "big" tx into some separated piece if the guarantee of atomic is not needed. For depositToNew it is just that case, i.e. we can separate it into a "register L2 key" tx and a "deposit to existed address“ tx and do not require the two op is atomic.

Currently the code is still confused so we may need to do some renaming / refactoring work so they become more clearer and easier to be understood:

  • Naming "UpdateL2Key" to "registerL2Key" for we has no plan to make the L2 account mutable after being registered

  • Update DepositToNew to be a "dummy" depositing (0 amount has been deposited) so it is just an alias of register new key. This would be consistent with the action inside rollup manager in which it has replayed the "usermessage" msg (see here).

@0xmountaintop
Copy link
Member Author

  • rename DepositToNew to RegisterL2Key
  • remove amount in DepositToNew/RegisterL2Key

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants