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

feat(contracts): bridge costs optimization #1011

Merged
merged 15 commits into from
Jan 8, 2024

Conversation

zimpha
Copy link
Member

@zimpha zimpha commented Nov 9, 2023

Purpose or design rationale of this PR

This PR try to reduce the gas costs for bridging. The following changes are made:

  • change counterpart, router, messenger in gateway from storage variable to immutable variable
  • change rollup and messageQueue in L1ScrollMessenger from storage variable to immutable variable
  • change messageQueue in ScrollChain from storage variable to immutable variable
  • change messenger and scrollChain in L1MessageQueue from storage variable to immutable variable
  • add L1MessageQueueWithGasPriceOracle to merge L2GasPriceOracle and L1MessageQueue

PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

  • perf: A code change that improves performance
  • refactor: A code change that doesn't fix a bug, or add a feature, or improves performance

Deployment tag versioning

Has tag in common/version.go been updated or have you added bump-version label to this PR?

  • No, this PR doesn't involve a new deployment, git tag, docker image tag

Breaking change label

Does this PR have the breaking-change label?

  • No, this PR is not a breaking change

@zimpha zimpha self-assigned this Nov 9, 2023
@zimpha zimpha changed the title feat(contracts): bridge costs optimization feat(contracts): bridge costs optimization (draft) Nov 9, 2023
Copy link

github-actions bot commented Nov 9, 2023

LCOV of commit 0bb3065 during Contracts #144

Summary coverage rate:
  lines......: 52.8% (1054 of 1997 lines)
  functions..: 66.3% (236 of 356 functions)
  branches...: no data found

Files changed coverage rate: n/a

contracts/src/L1/rollup/L1MessageQueue.sol Outdated Show resolved Hide resolved
contracts/src/L1/rollup/ScrollChain.sol Outdated Show resolved Hide resolved
@zimpha zimpha requested a review from iczc November 26, 2023 16:05
@zimpha zimpha changed the title feat(contracts): bridge costs optimization (draft) feat(contracts): bridge costs optimization Nov 26, 2023
@zimpha
Copy link
Member Author

zimpha commented Nov 30, 2023

change it.skip to it in integration-test/GasOptimizationUpgrade.spec.ts and then run the following command.

npx hardhat test integration-test/GasOptimizationUpgrade.spec.ts

below are example results:

  GasOptimizationUpgrade.spec
    L1 upgrade
L1ETHGateway.depositETH before upgrade: GasUsed[271677]
L1GatewayRouter.depositETH before upgrade: GasUsed[295114]
L1ScrollMessenger.sendMessage before upgrade: GasUsed[195609]
L1ETHGateway.depositETH after upgrade: GasUsed[161598]
L1GatewayRouter.depositETH after upgrade: GasUsed[185035]
L1ScrollMessenger.sendMessage after upgrade: GasUsed[133910]
      ✔ should succeed on L1ETHGateway (23503ms)
L1WETHGateway.depositERC20 WETH before upgrade: GasUsed[317736]
L1GatewayRouter.depositERC20 WETH before upgrade: GasUsed[343644]
L1WETHGateway.depositERC20 WETH before upgrade: GasUsed[193254]
L1GatewayRouter.depositERC20 WETH before upgrade: GasUsed[219162]
      ✔ should succeed on L1WETHGateway (7843ms)
L1StandardERC20Gateway.depositERC20 USDT before upgrade: GasUsed[347763]
L1GatewayRouter.depositERC20 USDT before upgrade: GasUsed[375859]
L1StandardERC20Gateway.depositERC20 USDT before upgrade: GasUsed[203309]
L1GatewayRouter.depositERC20 USDT before upgrade: GasUsed[231405]
      ✔ should succeed on L1StandardERC20Gateway (6706ms)
L1CustomERC20Gateway.depositERC20 DAI before upgrade: GasUsed[313758]
L1GatewayRouter.depositERC20 DAI before upgrade: GasUsed[339666]
L1CustomERC20Gateway.depositERC20 DAI before upgrade: GasUsed[188412]
L1GatewayRouter.depositERC20 DAI before upgrade: GasUsed[214320]
      ✔ should succeed on L1CustomERC20Gateway (5778ms)
L1USDCGateway.depositERC20 USDC before upgrade: GasUsed[337606]
L1GatewayRouter.depositERC20 USDC before upgrade: GasUsed[365463]
L1USDCGateway.depositERC20 USDC before upgrade: GasUsed[212482]
L1GatewayRouter.depositERC20 USDC before upgrade: GasUsed[240339]
      ✔ should succeed on L1USDCGateway (8311ms)
    L2 upgrade
L2ETHGateway.withdrawETH before upgrade: GasUsed[168104]
L2GatewayRouter.withdrawETH before upgrade: GasUsed[191358]
L2ScrollMessenger.sendMessage before upgrade: GasUsed[133257]
L2ETHGateway.withdrawETH after upgrade: GasUsed[166831]
L2GatewayRouter.withdrawETH after upgrade: GasUsed[183040]
L2ScrollMessenger.sendMessage after upgrade: GasUsed[138302]
      ✔ should succeed on L2ETHGateway (284324ms)
L2WETHGateway.withdrawERC20 WETH before upgrade: GasUsed[203795]
L2GatewayRouter.withdrawERC20 WETH before upgrade: GasUsed[220681]
L2WETHGateway.withdrawERC20 WETH before upgrade: GasUsed[194408]
L2GatewayRouter.withdrawERC20 WETH before upgrade: GasUsed[217384]
      ✔ should succeed on L2WETHGateway (20490ms)
L2StandardERC20Gateway.withdrawERC20 USDT before upgrade: GasUsed[185916]
L2GatewayRouter.withdrawERC20 USDT before upgrade: GasUsed[204972]
L2StandardERC20Gateway.withdrawERC20 USDT before upgrade: GasUsed[176553]
L2GatewayRouter.withdrawERC20 USDT before upgrade: GasUsed[201699]
      ✔ should succeed on L2StandardERC20Gateway (22963ms)
L2CustomERC20Gateway.withdrawERC20 DAI before upgrade: GasUsed[190416]
L2GatewayRouter.withdrawERC20 DAI before upgrade: GasUsed[207302]
L2CustomERC20Gateway.withdrawERC20 DAI before upgrade: GasUsed[181053]
L2GatewayRouter.withdrawERC20 DAI before upgrade: GasUsed[204029]
      ✔ should succeed on L2CustomERC20Gateway (22519ms)
L2USDCGateway.withdrawERC20 USDC before upgrade: GasUsed[212736]
L2GatewayRouter.withdrawERC20 USDC before upgrade: GasUsed[229555]
L2USDCGateway.withdrawERC20 USDC before upgrade: GasUsed[203373]
L2GatewayRouter.withdrawERC20 USDC before upgrade: GasUsed[226282]
      ✔ should succeed on L2USDCGateway (31009ms)

Copy link

codecov bot commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b96e877) 55.78% compared to head (45e1305) 55.84%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1011      +/-   ##
===========================================
+ Coverage    55.78%   55.84%   +0.05%     
===========================================
  Files          144      145       +1     
  Lines        11067    11031      -36     
===========================================
- Hits          6174     6160      -14     
+ Misses        4471     4449      -22     
  Partials       422      422              
Flag Coverage Δ
contracts 55.27% <100.00%> (+0.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zimpha zimpha mentioned this pull request Dec 22, 2023
3 tasks
contracts/docs/apis/L1ScrollMessenger.md Outdated Show resolved Hide resolved
contracts/docs/apis/L1ERC1155Gateway.md Outdated Show resolved Hide resolved
contracts/integration-test/GasOptimizationUpgrade.spec.ts Outdated Show resolved Hide resolved
contracts/src/L1/rollup/IL1MessageQueueWithOracle.sol Outdated Show resolved Hide resolved
contracts/src/L1/rollup/IL1MessageQueueWithOracle.sol Outdated Show resolved Hide resolved
contracts/src/L1/rollup/ScrollChain.sol Outdated Show resolved Hide resolved
contracts/src/libraries/IScrollMessenger.sol Outdated Show resolved Hide resolved
Thegaram
Thegaram previously approved these changes Jan 4, 2024
@zimpha zimpha merged commit aaea3cc into develop Jan 8, 2024
11 checks passed
@zimpha zimpha deleted the feat/bridge_cost_optimization branch January 8, 2024 09:25
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.

5 participants