Skip to content

Conversation

@riccardo-ssvlabs
Copy link
Contributor

@MatheusFranco99 please, could we research what kind of minimal values would be ideal for our config to set?

@github-actions
Copy link

github-actions bot commented May 15, 2025

Changes to gas cost

Generated at commit: cc063f7149a5075e6738e494a98cf50e41924eef, compared to commit: 5b2de18370b6f6c00fec8178a506249d5790afb9

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
ProtocolManager 564,217 (+127,752) updateFeeExpireTime
updateFeeTimelockPeriod
updateMaxFeeIncrement
updateMaxShares
updateObligationExpireTime
updateObligationTimelockPeriod
updateTokenUpdateTimelockPeriod
updateWithdrawalExpireTime
updateWithdrawalTimelockPeriod
364 (-3,368)
365 (-6,160)
365 (-6,168)
326 (-6,127)
387 (-6,168)
386 (-6,168)
344 (-6,157)
343 (-6,168)
342 (-6,168)
-90.25%
-94.41%
-94.41%
-94.95%
-94.10%
-94.11%
-94.71%
-94.73%
-94.75%
2,064 (-1,668)
4,493 (-2,032)
3,465 (-3,068)
3,402 (-3,051)
3,487 (-3,068)
3,486 (-3,068)
3,438 (-3,063)
3,443 (-3,068)
3,442 (-3,068)
-44.69%
-31.14%
-46.96%
-47.28%
-46.80%
-46.81%
-47.12%
-47.12%
-47.13%
2,064 (-1,668)
6,557 (+32)
3,465 (-3,068)
3,402 (-3,051)
3,487 (-3,068)
3,486 (-3,068)
3,438 (-3,063)
3,443 (-3,068)
3,442 (-3,068)
-44.69%
+0.49%
-46.96%
-47.28%
-46.80%
-46.81%
-47.12%
-47.12%
-47.13%
3,764 (+32)
6,557 (+32)
6,565 (+32)
6,479 (+26)
6,587 (+32)
6,586 (+32)
6,533 (+32)
6,543 (+32)
6,542 (+32)
+0.86%
+0.49%
+0.49%
+0.40%
+0.49%
+0.49%
+0.49%
+0.49%
+0.49%
2 (+1)
3 (+1)
2 (+1)
2 (+1)
2 (+1)
2 (+1)
2 (+1)
2 (+1)
2 (+1)
SSVBasedApps 2,842,167 (+133,978) initialize
updateFeeExpireTime
updateFeeTimelockPeriod
updateMaxFeeIncrement
updateMaxShares
updateObligationExpireTime
updateObligationTimelockPeriod
updateTokenUpdateTimelockPeriod
updateWithdrawalExpireTime
updateWithdrawalTimelockPeriod
3,183 (0)
2,678 (0)
2,699 (0)
2,654 (0)
2,631 (0)
2,678 (0)
2,654 (0)
2,635 (0)
2,678 (0)
2,698 (0)
0.00%
0.00%
0.00%
0.00%
0.00%
0.00%
0.00%
0.00%
0.00%
0.00%
169,701 (+560)
7,390 (+345)
9,822 (-562)
8,300 (-122)
8,236 (-123)
8,339 (-118)
8,314 (-118)
8,264 (-123)
8,310 (-125)
8,329 (-125)
+0.33%
+4.90%
-5.41%
-1.45%
-1.47%
-1.40%
-1.40%
-1.47%
-1.48%
-1.48%
171,284 (+1,338)
8,049 (+1,004)
11,165 (-3,062)
8,026 (-396)
7,964 (-395)
8,072 (-385)
8,047 (-385)
7,986 (-401)
8,028 (-407)
8,047 (-407)
+0.79%
+14.25%
-21.52%
-4.70%
-4.73%
-4.55%
-4.57%
-4.78%
-4.83%
-4.81%
191,184 (+1,338)
11,445 (+32)
14,259 (+32)
14,222 (+32)
14,113 (+26)
14,268 (+32)
14,243 (+32)
14,171 (+32)
14,224 (+32)
14,243 (+32)
+0.70%
+0.28%
+0.22%
+0.23%
+0.18%
+0.22%
+0.23%
+0.23%
+0.23%
+0.23%
535 (+15)
3 (+1)
4 (+1)
3 (+1)
3 (+1)
3 (+1)
3 (+1)
3 (+1)
3 (+1)
3 (+1)
NonCompliantBApp 485,061 (0) slash 85,979 (-12) -0.01% 85,986 (-5) -0.01% 85,991 (0) 0.00% 85,991 (0) 0.00% 5 (0)

@MatheusFranco99
Copy link
Contributor

@riccardo-ssvlabs
To be honest, I don't have good numbers for minimum reasonable values right now, but I think the ones you used are fine (1 day for the time lock period, and 1 hour for the expiration time).
For maxShares, ofc it should be less than $2^{128} \approx 10^{38}$, but, for the minimum value, if we mint in the initial deposit amount * 10^18 shares, then I think a minimum MaxShares of $1\text{M} \cdot 10^{18} = 10^{24}$ is a reasonable number.

@riccardo-ssvlabs riccardo-ssvlabs marked this pull request as ready for review May 16, 2025 15:05
@mtabasco mtabasco merged commit 90ede9d into release/v0.1.1 May 28, 2025
3 checks passed
@mtabasco mtabasco deleted the feat/security-checks-config branch May 28, 2025 12:34
mtabasco pushed a commit that referenced this pull request Jun 17, 2025
* fix(strategy-manager): change slashing and obligationUpdate event order
* fix(strategy): revert slashing if strategy not opted in
* Add Security Checks for Config (#49)
* feat: add and enforce basic checks for config vars
* Enrich SSVBasedApps interface (#51)
* Remove IERC165 Interface Check (#52)
* feat: check bApp registered during OptIn (#53)
* Use `ICore.TokenConfig` for `registerBApp()` (#50)
* chore: update prettier dependency (#62)
* chore: add Renovate config
* chore: sepolia deployment, bump version
* Example ECDSA verifier (#72)
* Fix: Propose Obligation Update storage ref (#74)
* feat: add script for implementation update and solidity 0.8.30
mtabasco added a commit that referenced this pull request Jun 18, 2025
* fix(strategy-manager): change slashing and obligationUpdate event order
* fix(strategy): revert slashing if strategy not opted in
* test(strategy): revert slash not opted in strategy
* Add Security Checks for Config (#49)
* Enrich SSVBasedApps interface (#51)
* Remove IERC165 Interface Check (#52)
* feat: check bApp registered during OptIn (#53)
* Use `ICore.TokenConfig` for `registerBApp()` (#50)
* chore: add Renovate config
* Example ECDSA verifier (#72)
* Fix: Propose Obligation Update storage ref (#74)
* feat: add script for implementation update and solidity 0.8.30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants