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

Change Executor.sol interface #780

Merged
merged 7 commits into from
Sep 9, 2024

Conversation

vladbochok
Copy link
Member

@vladbochok vladbochok commented Sep 7, 2024

What ❔

  • Change the Executor/ValidatorTimelock interface to be generic.
  • Small refactoring: move body of internal functions if only used in one external function.

Why ❔

  • No breaking changes to the ValidatorTimelock anymore.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.

Copy link

github-actions bot commented Sep 8, 2024

Changes to gas cost

Generated at commit: c5f42489d27d6b6ca16b797133c944040b58d7a4, compared to commit: cb42ae402af3e3f676003f44a49da4ea37a6811c

🧾 Summary (100% most significant diffs)

Contract Method Avg (+/-) %
TestExecutor commitBatchesSharedBridge
executeBatchesSharedBridge
proveBatchesSharedBridge
setPriorityTreeStartIndex
+2,533 ❌
-226 ✅
-198 ✅
-22 ✅
+9.03%
-2.82%
-2.47%
-0.10%
MerkleTest calculateRoot(bytes32[],bytes32[],uint256,bytes32[]) +419 ❌ +7.89%
DiamondProxy executeBatchesSharedBridge
proveBatchesSharedBridge
requestL2Transaction
setPriorityTreeStartIndex
+493 ❌
+82 ❌
+4 ❌
-22 ✅
+1.24%
+0.19%
+0.00%
-0.04%
ValidatorTimelock addValidator
commitBatchesSharedBridge
executeBatchesSharedBridge
getCommittedBatchTimestamp
proveBatchesSharedBridge
removeValidator
revertBatchesSharedBridge
setChainTypeManager
validators
-22 ✅
+326 ❌
+330 ❌
-22 ✅
+72 ❌
+44 ❌
-30 ✅
-22 ✅
+44 ❌
-0.04%
+0.57%
+1.01%
-1.34%
+0.22%
+0.14%
-0.11%
-0.05%
+2.65%
VerifierTest verificationKeyHash
verify
-33 ✅
-245 ✅
-3.20%
-0.18%
VerifierRecursiveTest verificationKeyHash
verify
-33 ✅
-248 ✅
-3.20%
-0.18%
Utils constructRollupL2DAValidatorOutputHash
createCommitBatchInfo
getExecutorSelectors
getGettersSelectors
getMailboxSelectors
makeInitializeDataForNewChain
randomBytes32
-22 ✅
-22 ✅
-22 ✅
-22 ✅
+44 ❌
+45 ❌
-22 ✅
-1.58%
-0.53%
-1.79%
-0.35%
+2.49%
+2.24%
-2.02%
PriorityTreeTest initFromCommitment
processBatch
push
-17 ✅
+826 ❌
-1 ✅
-0.03%
+2.26%
-0.00%
DummyBridgehubSetter addTokenAssetId
setCTM
setPendingAdmin
setZKChain
0 ➖
-1 ✅
0 ➖
-1 ✅
0.00%
-0.00%
0.00%
-0.00%
PermanentRestriction allowAdminImplementation
setAllowedData
-22 ✅
-93 ✅
-0.05%
-0.19%
ChainAdmin setUpgradeTimestamp -83 ✅ -0.18%
TransparentUpgradeableProxy createNewChain
setChainCreationParams
setNewVersionUpgrade
setUpgradeDiamondCut
+12 ❌
+12 ❌
+12 ❌
+12 ❌
+0.02%
+0.01%
+0.01%
+0.01%
DiamondCutTestContract diamondCut +2 ❌ +0.00%
Bridgehub createNewChain +12 ❌ +0.00%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
TestExecutor 3,484,949 (+392,229) commitBatchesSharedBridge
executeBatchesSharedBridge
proveBatchesSharedBridge
setPriorityTreeStartIndex
7,814 (-45)
7,791 (-226)
7,813 (-198)
22,310 (-22)
-0.57%
-2.82%
-2.47%
-0.10%
30,575 (+2,533)
7,791 (-226)
7,813 (-198)
22,310 (-22)
+9.03%
-2.82%
-2.47%
-0.10%
30,086 (+2,387)
7,791 (-226)
7,813 (-198)
22,310 (-22)
+8.62%
-2.82%
-2.47%
-0.10%
46,788 (+2,846)
7,791 (-226)
7,813 (-198)
22,310 (-22)
+6.48%
-2.82%
-2.47%
-0.10%
15 (0)
1 (0)
1 (0)
1 (0)
MerkleTest 520,004 (+20,869) calculateRoot(bytes32[],bytes32[],uint256,bytes32[])
calculateRoot(bytes32[],uint256,bytes32)
1,595 (+481)
565 (0)
+43.18%
0.00%
5,731 (+419)
2,930 (+1)
+7.89%
+0.03%
1,856 (+399)
2,982 (0)
+27.39%
0.00%
13,520 (+366)
3,026 (0)
+2.78%
0.00%
8 (0)
265 (0)
DiamondProxy 2,475,597 (+12) commitBatchesSharedBridge
executeBatchesSharedBridge
finalizeEthWithdrawal
proveBatchesSharedBridge
requestL2Transaction
setPriorityTreeStartIndex
util_setChainId
0 (0)
40,124 (+493)
37,568 (0)
42,767 (+82)
33,146 (0)
50,749 (-22)
28,906 (0)
+∞%
+1.24%
0.00%
+0.19%
0.00%
-0.04%
0.00%
35,116 (+1,631)
40,124 (+493)
76,235 (-122)
42,767 (+82)
129,613 (+4)
50,749 (-22)
33,776 (+43)
+4.87%
+1.24%
-0.16%
+0.19%
+0.00%
-0.04%
+0.13%
21,603 (+331)
40,124 (+493)
76,574 (0)
42,767 (+82)
166,356 (+60)
50,749 (-22)
33,718 (0)
+1.56%
+1.24%
0.00%
+0.19%
+0.04%
-0.04%
0.00%
91,803 (+4,074)
40,124 (+493)
76,874 (0)
42,767 (+82)
190,435 (0)
50,749 (-22)
34,090 (0)
+4.64%
+1.24%
0.00%
+0.19%
0.00%
-0.04%
0.00%
30 (0)
1 (0)
257 (0)
1 (0)
771 (0)
1 (0)
519 (0)
ValidatorTimelock 987,789 (-143,331) addValidator
chainTypeManager
commitBatchesSharedBridge
executeBatchesSharedBridge
executionDelay
getCommittedBatchTimestamp
proveBatchesSharedBridge
removeValidator
revertBatchesSharedBridge
setChainTypeManager
validators
29,438 (-22)
401 (+44)
30,410 (+565)
26,668 (+462)
357 (-44)
618 (-22)
28,098 (+70)
29,460 (+44)
23,916 (-33)
24,027 (-22)
702 (+44)
-0.07%
+12.32%
+1.89%
+1.76%
-10.97%
-3.44%
+0.25%
+0.15%
-0.14%
-0.09%
+6.69%
52,443 (-22)
1,401 (+44)
57,593 (+326)
33,061 (+330)
1,357 (-44)
1,618 (-22)
33,230 (+72)
30,889 (+44)
27,283 (-30)
44,530 (-22)
1,702 (+44)
-0.04%
+3.24%
+0.57%
+1.01%
-3.14%
-1.34%
+0.22%
+0.14%
-0.11%
-0.05%
+2.65%
53,355 (-22)
1,401 (+44)
63,028 (+278)
31,171 (+250)
1,357 (-44)
1,618 (-22)
33,230 (+72)
31,479 (+44)
23,928 (-33)
46,097 (-22)
1,702 (+44)
-0.04%
+3.24%
+0.44%
+0.81%
-3.14%
-1.34%
+0.22%
+0.14%
-0.14%
-0.05%
+2.65%
53,355 (-22)
2,401 (+44)
63,040 (+278)
41,344 (+277)
2,357 (-44)
2,618 (-22)
38,362 (+73)
31,730 (+44)
34,007 (-22)
46,097 (-22)
2,702 (+44)
-0.04%
+1.87%
+0.44%
+0.67%
-1.83%
-0.83%
+0.19%
+0.14%
-0.06%
-0.05%
+1.66%
50 (0)
2 (0)
6 (0)
3 (0)
2 (0)
2 (0)
2 (0)
3 (0)
3 (0)
25 (0)
10 (0)
VerifierTest 3,772,418 (-24,450) verificationKeyHash
verify
998 (-33)
3,966 (-245)
-3.20%
-5.82%
998 (-33)
135,204 (-245)
-3.20%
-0.18%
998 (-33)
3,966 (-245)
-3.20%
-5.82%
998 (-33)
360,630 (-245)
-3.20%
-0.07%
1 (0)
11 (0)
VerifierRecursiveTest 3,772,850 (-24,378) verificationKeyHash
verify
999 (-33)
4,276 (-248)
-3.20%
-5.48%
999 (-33)
137,522 (-248)
-3.20%
-0.18%
999 (-33)
4,276 (-248)
-3.20%
-5.48%
999 (-33)
374,006 (-248)
-3.20%
-0.07%
1 (0)
14 (0)
Utils 6,037,668 (-32,049) constructRollupL2DAValidatorOutputHash
createCommitBatchInfo
getExecutorSelectors
getGettersSelectors
getMailboxSelectors
makeInitializeDataForNewChain
randomBytes32
1,364 (-22)
4,131 (-22)
1,206 (-22)
6,290 (-22)
1,814 (+44)
2,050 (+45)
1,062 (-22)
-1.59%
-0.53%
-1.79%
-0.35%
+2.49%
+2.24%
-2.03%
1,370 (-22)
4,131 (-22)
1,206 (-22)
6,290 (-22)
1,814 (+44)
2,050 (+45)
1,066 (-22)
-1.58%
-0.53%
-1.79%
-0.35%
+2.49%
+2.24%
-2.02%
1,364 (-22)
4,131 (-22)
1,206 (-22)
6,290 (-22)
1,814 (+44)
2,050 (+45)
1,062 (-22)
-1.59%
-0.53%
-1.79%
-0.35%
+2.49%
+2.24%
-2.03%
1,522 (-22)
4,131 (-22)
1,206 (-22)
6,290 (-22)
1,814 (+44)
2,050 (+45)
1,144 (-22)
-1.42%
-0.53%
-1.79%
-0.35%
+2.49%
+2.24%
-1.89%
25 (0)
6 (0)
25 (0)
56 (0)
36 (0)
48 (0)
514 (0)
PriorityTreeTest 998,915 (-5,625) initFromCommitment
processBatch
push
54,070 (-17)
23,662 (+1,072)
90,757 (-1)
-0.03%
+4.75%
-0.00%
54,070 (-17)
37,372 (+826)
104,707 (-1)
-0.03%
+2.26%
-0.00%
54,070 (-17)
36,254 (+809)
109,056 (-1)
-0.03%
+2.28%
-0.00%
54,070 (-17)
53,319 (+614)
116,710 (-1)
-0.03%
+1.16%
-0.00%
1 (0)
4 (0)
14 (0)
DummyBridgehubSetter 5,300,040 (0) addTokenAssetId
proveL1ToL2TransactionStatus
proveL2LogInclusion
removeChainTypeManager
setCTM
setPendingAdmin
setZKChain
24,156 (0)
0 (0)
0 (0)
23,802 (0)
44,148 (0)
25,880 (-12)
111,315 (0)
0.00%
+∞%
+∞%
0.00%
0.00%
-0.05%
0.00%
36,601 (0)
1,363 (-18)
1,460 (-9)
25,426 (+6)
44,184 (-1)
41,562 (0)
111,351 (-1)
0.00%
-1.30%
-0.61%
+0.02%
-0.00%
0.00%
-0.00%
36,962 (+6)
969 (0)
1,060 (0)
25,774 (0)
44,184 (-12)
49,541 (0)
111,351 (-12)
+0.02%
0.00%
0.00%
0.00%
-0.03%
0.00%
-0.01%
47,583 (0)
3,593 (-14)
3,792 (0)
26,190 (0)
44,208 (0)
49,757 (0)
111,375 (0)
0.00%
-0.39%
0.00%
0.00%
0.00%
0.00%
0.00%
1,536 (0)
512 (0)
512 (0)
2,048 (0)
768 (0)
1,278 (0)
768 (0)
MailboxFacet 3,258,553 (0) finalizeEthWithdrawal 8,079 (0) 0.00% 48,949 (-124) -0.25% 49,357 (0) 0.00% 49,357 (0) 0.00% 257 (0)
PermanentRestriction 1,147,705 (0) allowAdminImplementation
setAllowedData
47,506 (+12)
48,233 (0)
+0.03%
0.00%
47,727 (-22)
49,197 (-93)
-0.05%
-0.19%
47,866 (0)
48,860 (-108)
0.00%
-0.22%
47,866 (0)
51,005 (-24)
0.00%
-0.05%
258 (0)
257 (0)
ChainAdmin 909,847 (0) setUpgradeTimestamp 25,361 (-19,912) -43.98% 45,318 (-83) -0.18% 45,339 (+12) +0.03% 45,645 (0) 0.00% 256 (0)
TransparentUpgradeableProxy 987,816 (+12) createNewChain
setChainCreationParams
setNewVersionUpgrade
setUpgradeDiamondCut
55,516 (+12)
96,005 (+12)
200,413 (+12)
96,788 (+12)
+0.02%
+0.01%
+0.01%
+0.01%
69,666 (+12)
96,005 (+12)
200,413 (+12)
96,788 (+12)
+0.02%
+0.01%
+0.01%
+0.01%
69,666 (+12)
96,005 (+12)
200,413 (+12)
96,788 (+12)
+0.02%
+0.01%
+0.01%
+0.01%
83,816 (+12)
96,005 (+12)
200,413 (+12)
96,788 (+12)
+0.01%
+0.01%
+0.01%
+0.01%
2 (0)
1 (0)
1 (0)
1 (0)
AccessControlRestriction 1,759,703 (0) grantRole
setRequiredRoleForCall
setRequiredRoleForFallback
51,036 (0)
48,605 (0)
47,940 (0)
0.00%
0.00%
0.00%
51,266 (-8)
49,426 (+7)
48,846 (-7)
-0.02%
+0.01%
-0.01%
51,408 (0)
48,977 (0)
48,312 (0)
0.00%
0.00%
0.00%
51,408 (0)
51,877 (0)
51,658 (0)
0.00%
0.00%
0.00%
1,024 (0)
1,280 (0)
1,280 (0)
MerkleTreeNoSort 583,114 (0) getProof 2,608 (0) 0.00% 32,752 (-4) -0.01% 33,207 (0) 0.00% 33,229 (0) 0.00% 277 (0)
DiamondCutTestContract 2,420,735 (0) diamondCut 23,653 (0) 0.00% 203,351 (+2) +0.00% 44,650 (0) 0.00% 1,437,433 (+12) +0.00% 20 (0)
Bridgehub 5,262,920 (0) createNewChain 4,072,617 (+12) +0.00% 4,072,617 (+12) +0.00% 4,072,617 (+12) +0.00% 4,072,617 (+12) +0.00% 15 (0)
AdminFacet 4,598,143 (+12)
DefaultUpgrade 1,671,236 (+12)
TestBaseFacet 219,676 (-12)

@vladbochok vladbochok marked this pull request as ready for review September 9, 2024 08:38
Copy link

github-actions bot commented Sep 9, 2024

Coverage after merging vb-generic-validator-interface into kl/sync-layer-reorg will be

87.17%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
../da-contracts/contracts
   RollupL1DAValidator.sol61.04%31.25%83.33%67.27%145, 148, 148, 148, 150, 183–184, 187–188, 27, 27–28, 30, 30–31, 34, 36–37, 41–42, 65, 67, 67, 67–68, 70, 80, 80–81
contracts/bridge
   BridgeHelper.sol100%100%100%100%
   L1AssetRouter.sol93.95%85.07%94.87%96.63%139–140, 155–156, 216, 236, 285, 338, 390, 550, 571, 613, 732–733, 754–755, 896
   L1ERC20Bridge.sol97.50%100%100%96.55%116
   L1NativeTokenVault.sol92.31%93.33%78.57%94.19%175–176, 256, 284, 289, 46–47
contracts/bridgehub
   Bridgehub.sol66.23%35.90%75.61%76.72%110, 110, 110, 115, 121, 126, 131, 163–164, 207, 207–208, 210–211, 211, 211–212, 218–219, 219, 219–220, 220, 220, 222–223, 223, 223–224, 232–233, 247–248, 295–296, 326–327, 329–330, 332–334, 336–337, 340–341, 344, 346–347, 349–350, 377–378, 388–389, 393, 393–394, 396, 424, 448–450, 453, 453–454, 498, 506–508, 512, 512–513, 515, 529–530, 542–543, 578, 597–598, 677, 680–681, 685–686, 716–717, 730, 775, 780, 785, 790, 799
   CTMDeploymentTracker.sol79.07%50%90%94.74%115, 119, 34, 41, 64, 91, 94, 96
   MessageRoot.sol91.07%63.64%100%96.97%116–117, 148, 69, 87
contracts/common
   ReentrancyGuard.sol90%66.67%100%92.86%78–79
contracts/common/libraries
   DataEncoding.sol80%100%100%66.67%72, 80
   DynamicIncrementalMerkle.sol74.42%100%80%72.22%67–70, 72–74, 76–78
   FullMerkle.sol100%100%100%100%
   L2ContractHelper.sol47.06%0%60%54.17%100, 104, 29–30, 35–36, 39–40, 54, 56, 56–57, 61, 61–62, 70
   Merkle.sol96.61%90.91%100%97.67%80–81
   MessageHashing.sol100%100%100%100%
   SemVer.sol100%100%100%100%
   UncheckedMath.sol100%100%100%100%
   UnsafeBytes.sol84.21%100%83.33%84.62%35–36
contracts/governance
   AccessControlRestriction.sol100%100%100%100%
   ChainAdmin.sol95.12%80%100%96.15%27–28
   Governance.sol98.15%94.74%100%98.55%45–46
   PermanentRestriction.sol87.32%78.57%100%87.23%142, 142–143, 146, 148, 148–149, 176–177
contracts/state-transition
   ChainTypeManager.sol67.28%33.33%60%77.67%108, 135–136, 138–139, 141–142, 144–145, 200–201, 245, 252, 270, 276, 283, 295, 302, 309, 317, 324, 332, 339, 357, 359, 424, 443, 443, 443, 446, 446, 446, 448, 461, 466, 491, 74, 87–88
   TestnetVerifier.sol77.78%66.67%100%75%16, 28
   ValidatorTimelock.sol95.08%83.33%100%95.24%200, 82–83
   Verifier.sol89.90%40%96.30%90.93%1674–1675, 287–302, 305–308, 311–318, 321–328, 331–332, 335–336, 339, 383–384, 394–395, 405–406, 416–417, 427–428, 443–444, 453, 453–454, 905–906
contracts/state-transition/chain-deps
   DiamondInit.sol78%45.45%100%86.49%39–40, 42–43, 45–46, 48–49, 51–52, 77
   DiamondProxy.sol92.31%75%100%100%16, 27
contracts/state-transition/chain-deps/facets
   Admin.sol72.69%36.21%90.91%85.29%104–105, 115–116, 130, 130–131, 133–134, 157, 157, 157–158, 158, 158, 160, 239, 241, 254–255, 261, 263, 266, 266, 266, 284, 295–296, 301, 313, 313, 315, 315, 315, 321, 321, 321–322, 322, 322–324, 324, 324–325, 325, 325–327, 354, 356, 360, 369, 379, 383, 40, 40
   Executor.sol75.48%55.84%92%80.77%120–121, 173, 178, 183, 188, 193, 198, 202–203, 208, 208–209, 209–210, 212, 212–213, 223–225, 227, 227–228, 246–247, 268, 271, 317, 317–318, 322, 326, 328–329, 335–336, 355–358, 360, 38, 416, 416, 416–419, 421, 424, 427–428, 441, 444–445, 447–449, 462–464, 520–521, 525–526, 548–549, 559–560, 611–612, 638–639
   Getters.sol86.32%66.67%86.67%87.88%115, 118, 129, 132, 227, 232, 236, 70, 85, 90
   Mailbox.sol82.52%70.59%90%83.80%153, 188, 198, 207–208, 233, 237, 237, 237, 240, 242–243, 245, 250, 252, 255–257, 262–265, 267–268, 274, 274, 274, 276, 279–280, 372–373, 54
   ZKChainBase.sol70.59%66.67%75%70.59%59, 59–60, 66, 66–67, 73, 76
contracts/state-transition/data-availability
   

@vladbochok vladbochok merged commit 49868af into kl/sync-layer-reorg Sep 9, 2024
24 checks passed
@vladbochok vladbochok deleted the vb-generic-validator-interface branch September 9, 2024 12:17
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