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

Deposit contract gas reduction #2226

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Deposit contract gas reduction #2226

wants to merge 6 commits into from

Conversation

86667
Copy link
Contributor

@86667 86667 commented Jan 24, 2025

This PR contains two options for mitigating the issue described in #1991.

Solution 1

The first solution found in file deposit_v4 is a general reduction in the gas used across the contract. These changes reduce the reading and writing to storage when writing future committees but the issue of uneven gas costs remains.

The effects of the changes can be seen in the below table:

Call Before After
Deposit owner1 348311 346633
Deposit owner2 584151 585667
depositTopUp owner1 337149 258079
depositTopUp owner2 13484 13484
2nd depositTopUp owner1 202925 34310
2nd depositTopUp owner2 13499 13499

The 2nd depositTopUp is so much cheaper because we do not need to write new keys to storage, only update the balances.

Solution 2

The second solution is in deposit_v5. This is an attempt to remove the uneven gas cost of the depositTopUp() call by removing the need to write futureCommittee all together and having the committee calculated on the fly whenever it is required.

The code's outline is complete but the solution is unfinished. TODO:

  • Run gas usage tests to understand how the cost of calculating committee() scales with number of depositors. Is it acceptable to pay this gas fee in all scenarios in which it is called?
  • The solution removed the need for StakerMoved. Figure out how we can continue to support the behavior expected by third parties and consensus

Copy link
Contributor

github-actions bot commented Jan 24, 2025

🐰 Bencher Report

Branchdeposit_v3_gas_reduction
Testbedself-hosted
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
full-blocks-erc20-transfers/full-blocks-erc20-transfers📈 view plot
🚷 view threshold
1,745.90
(+27.55%)
1,848.51
(94.45%)
full-blocks-evm-transfers/full-blocks-evm-transfers📈 view plot
🚷 view threshold
455.73
(-23.50%)
740.13
(61.57%)
full-blocks-zil-transfers/full-blocks-zil-transfers📈 view plot
🚷 view threshold
4,899.90
(+16.58%)
5,385.33
(90.99%)
process-empty/process-empty📈 view plot
🚷 view threshold
7.59
(-15.37%)
10.50
(72.25%)
🐰 View full continuous benchmarking report in Bencher

@86667 86667 changed the title Deposit v3 gas reduction Deposit contract gas reduction Jan 29, 2025
@86667 86667 force-pushed the deposit_v3_gas_reduction branch from fab6b5d to 138d5c7 Compare January 29, 2025 15:07
@86667 86667 marked this pull request as draft January 29, 2025 15:08
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.

1 participant