Skip to content

Comments

feat: deposit_v4 with cheaper depositTopUp#2102

Closed
86667 wants to merge 0 commit intomainfrom
1991-incorrect-estimation-and-high-gas-costs-on-first-call-to-deposittopupdeposit
Closed

feat: deposit_v4 with cheaper depositTopUp#2102
86667 wants to merge 0 commit intomainfrom
1991-incorrect-estimation-and-high-gas-costs-on-first-call-to-deposittopupdeposit

Conversation

@86667
Copy link
Contributor

@86667 86667 commented Jan 7, 2025

Work done by updateLatestComputedEpoch() within functions called by delegators, ie depositTopUp(), unstake() and withdraw() is expensive in some circumstances, ie when a deposit has occurred and one or more epochs passed since. This is because the new staker key is written to future committees.

This work aims to remove costly writes to _committee[] from the above functions and instead have all writes done in deposit(). To achieve this i've changed how _committee works slightly such that we only write to _committee[] if there are changes and do not copy data if data hasnt changed.

Results

Before

deposit owner1: Gas used 410503

Rolling ahead 2 epochs
deposit owner2: Gas used 644369

Rolling ahead 1 epochs
1st top up owner 1: Gas used 179666
1st top up owner 2: Gas used 16149

Rolling ahead 2 epochs
2nd top up owner 1: Gas used 273779
2nd top up owner 2: Gas used 16158

After

deposit owner1: Gas used 334749

Rolling ahead 2 epochs
deposit owner2: Gas used 446585

Rolling ahead 1 epochs
1st top up owner 1: Gas used 14174
1st top up owner 2: Gas used 14174

Rolling ahead 2 epochs
2nd top up owner 1: Gas used 13987
2nd top up owner 2: Gas used 13990

@86667 86667 changed the base branch from main to deposit_v4 January 7, 2025 11:34
@86667 86667 force-pushed the 1991-incorrect-estimation-and-high-gas-costs-on-first-call-to-deposittopupdeposit branch 2 times, most recently from 6911fa3 to 58da64d Compare January 7, 2025 11:41
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2025

🐰 Bencher Report

Branch1991-incorrect-estimation-and-high-gas-costs-on-first-call-to-deposittopupdeposit
Testbedself-hosted

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results
BenchmarkLatencymilliseconds (ms)
process-empty/process-empty📈 view plot
⚠️ NO THRESHOLD
9.63
produce-full/produce-full📈 view plot
⚠️ NO THRESHOLD
1,964.70
🐰 View full continuous benchmarking report in Bencher

@86667 86667 force-pushed the 1991-incorrect-estimation-and-high-gas-costs-on-first-call-to-deposittopupdeposit branch 2 times, most recently from bfd580a to a73922a Compare January 8, 2025 18:05
@86667 86667 force-pushed the 1991-incorrect-estimation-and-high-gas-costs-on-first-call-to-deposittopupdeposit branch from 44900a5 to 575142b Compare January 9, 2025 09:17
Base automatically changed from deposit_v4 to main January 9, 2025 19:07
@86667 86667 closed this Jan 29, 2025
@86667 86667 force-pushed the 1991-incorrect-estimation-and-high-gas-costs-on-first-call-to-deposittopupdeposit branch from 3c8e9bd to 521ad2f Compare January 29, 2025 14:49
@86667
Copy link
Contributor Author

86667 commented Jan 29, 2025

Migrated along with another solution to a single draft PR - #2226

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.

Incorrect estimation and high gas costs of some calls to deposit::topupDeposit()

1 participant