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

simple gas simulations for rollups #284

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

simple gas simulations for rollups #284

wants to merge 4 commits into from

Conversation

johannbarbie
Copy link
Member

@johannbarbie johannbarbie commented Feb 25, 2020

rosolves #273

steps:

  • create a version of the bridge contract that accepts block data
  • measure gas costs for rolling up transaction of 154 bytes each (average size erc20 transfer, source)
  • measure gas cost for rolling up transactions of 408 bytes each (size of proof in tornado.cash transactions)

measurement results:
https://docs.google.com/spreadsheets/d/1ywhXffNw3sNzngvblu4hxE6d2ZbSQ_GjN2JFfl3vYFc/edit

TheReturnOfJan
TheReturnOfJan previously approved these changes Feb 27, 2020

require('./helpers/setup');

const Bridge = artifacts.require('RollupBridge');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next time call it RollupBridge hehe. I know this is not getting merged, but it is a good opportunity to write something and prove I actually reviewed this. 💯


const rsp = await bridge.submitPeriodWithData(prevPeriodHash, newPeriodHash, block.hex()).should.be.fulfilled;
console.log(rsp.receipt.gasUsed);
// assert.equal(rsp.receipt.gasUsed, 738140);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the number correct here? Why is it commented out?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I think I see. You just used these to collect data. Disregard.

@TheReturnOfJan
Copy link

Looks good. Just 2 comments:

  • I like the experimental approach, but just want to mention here that this could have probably been also achieved using just equations (at least the asymptotics).
  • Maybe mention in the doc, that the rollups you are referring to are optimistic rollups, in case this will be read by people that can not deduce it from context

But overall, very nice 👍

P.S.: Real man use matplotlib to plot data

@TheReturnOfJan
Copy link

Also one more thing: you could have probably pushed the data into an array during tests and then use something like plotly and have a plot automatically. Just FYI.

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

Successfully merging this pull request may close these issues.

2 participants