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.

Sign up for GitHub

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

Jump to bottom

L1-L2-checks #11225

Merged

martinvol merged 8 commits into feat/l2-epoch-system from soloseng/L1-L2-checks

Sep 25, 2024

Contributor

soloseng commented Sep 24, 2024

Description

Check if onlyL1 && onlyL2 are appropriatly called

Other changes

updated test

Related issues

Fixes #[issue number here]


          ++ missing onlyL1 check in downtime & double signing slasher

968c62f

soloseng requested a review from a team as a code owner

September 24, 2024 22:23


          updated L1/L2 flags

7a623ce

soloseng commented

View reviewed changes

packages/protocol/contracts/common/Accounts.sol Show resolved Hide resolved

soloseng commented

View reviewed changes

packages/protocol/contracts/governance/test/MockValidators.sol Outdated

@@ @@ -38,7 +37,7 @@ contract MockValidators is IValidators, IsL2Check { @@
                   bytes calldata,
                   bytes calldata
                 ) external returns (bool) {
-                  allowOnlyL1();
+                  allowOnlyL1(); // BLS key no longer supported

Contributor Author

soloseng Sep 24, 2024

BLS key no longer supported

soloseng commented

View reviewed changes

packages/protocol/contracts/governance/test/MockValidators.sol Outdated

@@ @@ -84,20 +78,15 @@ contract MockValidators is IValidators, IsL2Check { @@
                 }
                 function halveSlashingMultiplier(address) external {
-                  allowOnlyL1(); // TODO remove
+                  allowOnlyL1(); // not used by governance slasher

Contributor Author

soloseng Sep 24, 2024

This function is not used by governance slasher.

soloseng commented

View reviewed changes

packages/protocol/contracts/governance/test/MockValidators.sol Outdated

                 }
                 function forceDeaffiliateIfValidator(address validator) external {
-                  allowOnlyL1(); // TODO remove
+                  allowOnlyL1(); // not used by governance slasher

Contributor Author

soloseng Sep 24, 2024

This function is not used by governance slasher. L1 check stays.

soloseng commented

View reviewed changes

packages/protocol/contracts/governance/test/MockValidators.sol Outdated

@@ @@ -119,7 +108,7 @@ contract MockValidators is IValidators, IsL2Check { @@
                 }
                 function getValidatorGroupSlashingMultiplier(address) external view returns (uint256) {
-                  allowOnlyL1();
+                  allowOnlyL1(); // not used by governance slasher

Contributor Author

soloseng Sep 24, 2024

This function is not used by governance slasher. L1 check stays

soloseng commented

View reviewed changes

packages/protocol/contracts/governance/DoubleSigningSlasher.sol

                   uint256 index,
                   bytes memory headerA,
                   bytes memory headerB
-                ) public view returns (uint256) {
+                ) public view onlyL1 returns (uint256) {

Contributor Author

soloseng Sep 24, 2024

Uses precompile. Added modifier for a clean revert message.

soloseng commented

View reviewed changes

packages/protocol/contracts/governance/DowntimeSlasher.sol

                 function getBitmapForInterval(
                   uint256 startBlock,
                   uint256 endBlock
-                ) public view returns (bytes32) {
+                ) public view onlyL1 returns (bytes32) {

Contributor Author

soloseng Sep 24, 2024

Uses precompile. Added modifier for a clean revert msg.

soloseng commented

View reviewed changes

packages/protocol/contracts/governance/DowntimeSlasher.sol

                   uint256 startBlock,
                   uint256 endBlock,
                   uint256 signerIndex
-                ) public view returns (bool) {
+                ) public view onlyL1 returns (bool) {

Contributor Author

soloseng Sep 24, 2024

Ditto

soloseng commented

View reviewed changes

packages/protocol/contracts/governance/DowntimeSlasher.sol

                   uint256[] memory startBlocks,
                   uint256[] memory endBlocks,
                   uint256[] memory signerIndices
-                ) public view returns (bool) {
+                ) public view onlyL1 returns (bool) {

Contributor Author

soloseng Sep 24, 2024

ditto.

downtime slashing not supported

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol Outdated

@@ @@ -219,7 +219,7 @@ contract Validators is @@
                  * @param uptime The Fixidity representation of the validator's uptime, between 0 and 1.
                  */
                 function updateValidatorScoreFromSigner(address signer, uint256 uptime) external virtual onlyVm {
-                  allowOnlyL1();
+                  allowOnlyL1(); // no longer using signer

Contributor Author

soloseng Sep 24, 2024

No longer using signers or uptime score

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol Outdated

@@ @@ -234,7 +234,7 @@ contract Validators is @@
                   address signer,
                   uint256 maxPayment
                 ) external virtual onlyVm returns (uint256) {
-                  allowOnlyL1();
+                  allowOnlyL1(); // no longer using signer

Contributor Author

soloseng Sep 24, 2024

No longer using signers

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol Outdated

@@ @@ -255,7 +255,7 @@ contract Validators is @@
                   bytes calldata blsPublicKey,
                   bytes calldata blsPop
                 ) external nonReentrant returns (bool) {
-                  allowOnlyL1();
+                  allowOnlyL1(); // no longer allow to register with a bls key

Contributor Author

soloseng Sep 24, 2024

No longer supporting BLS key.

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol Outdated

@@ @@ -385,7 +385,7 @@ contract Validators is @@
                   bytes calldata blsPublicKey,
                   bytes calldata blsPop
                 ) external returns (bool) {
-                  allowOnlyL1();
+                  allowOnlyL1(); // no longer allow to register with a bls key

Contributor Author

soloseng Sep 24, 2024

No longer supporting BLS key.

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol Outdated

@@ @@ -461,7 +461,7 @@ contract Validators is @@
                   bytes calldata blsPublicKey,
                   bytes calldata blsPop
                 ) external onlyRegisteredContract(ACCOUNTS_REGISTRY_ID) returns (bool) {
-                  allowOnlyL1();
+                  allowOnlyL1(); // no longer allow to register with a bls key

Contributor Author

soloseng Sep 24, 2024

No longer supporting BLS key.

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol Outdated

@@ @@ -611,6 +611,7 @@ contract Validators is @@
                  * @param validatorAccount The validator to deaffiliate from their affiliated validator group.
                  */
                 function forceDeaffiliateIfValidator(address validatorAccount) external nonReentrant onlySlasher {
+                  allowOnlyL1(); //Not used by governance slasher.

Contributor Author

soloseng Sep 24, 2024

Added since governance slasher does not use this function.

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol Outdated

@@ @@ -639,7 +640,7 @@ contract Validators is @@
                  * @param account The group being slashed.
                  */
                 function halveSlashingMultiplier(address account) external nonReentrant onlySlasher {
-                  allowOnlyL1();
+                  allowOnlyL1(); //Not used by governance slasher.

Contributor Author

soloseng Sep 24, 2024

This function is not used by governance slasher.

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol

                   require(isValidator(account), "Not a validator");
-                  require(epochNumber <= getEpochNumber(), "Epoch cannot be larger than current");
+                  require(epochNumber <= _getEpochNumber(), "Epoch cannot be larger than current");

Contributor Author

soloseng Sep 24, 2024

Updated to support L2 epoch number

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol Outdated

@@ @@ -992,7 +989,7 @@ contract Validators is @@
                   uint256 exponent,
                   uint256 adjustmentSpeed
                 ) public onlyOwner returns (bool) {
-                  allowOnlyL1();
+                  allowOnlyL1(); // score handled by score manager now
                   require(

Contributor Author

soloseng Sep 24, 2024

score now handled by score manager. Leaving as is.

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol Outdated

@@ @@ -1055,7 +1052,7 @@ contract Validators is @@
                  * @param value New reset period for slashing multiplier.
                  */
                 function setSlashingMultiplierResetPeriod(uint256 value) public nonReentrant onlyOwner {
-                  allowOnlyL1();
+                  allowOnlyL1(); // no used by governance slasher.

Contributor Author

soloseng Sep 24, 2024

This function is not used by governance slasher.

soloseng commented

View reviewed changes

packages/protocol/contracts-0.8/governance/Validators.sol Outdated

@@ @@ -1119,7 +1116,7 @@ contract Validators is @@
                  * @return Fixidity representation of the epoch score between 0 and 1.
                  */
                 function calculateEpochScore(uint256 uptime) public view returns (uint256) {
-                  allowOnlyL1();
+                  allowOnlyL1(); //score set by score manager.
                   require(uptime <= FixidityLib.fixed1().unwrap(), "Uptime cannot be larger than one");

Contributor Author

soloseng Sep 24, 2024

Not supported in L2. Score is now set by scoreManager.

soloseng and others added 3 commits

September 24, 2024 18:46


          removed comments

0cc126a


          updated tests

1d4149e


          Standardize Validators.sol

c41ae10

pahor167 approved these changes

View reviewed changes

martinvol and others added 3 commits

September 25, 2024 11:39


          Merge branch 'feat/l2-epoch-system' into soloseng/L1-L2-checks

708891b


          Removed duplicated code election.sol

0500b3d


          Merge branch 'feat/l2-epoch-system' into soloseng/L1-L2-checks

fec724b

martinvol merged commit b82b51d into feat/l2-epoch-system

37 of 38 checks passed

martinvol deleted the soloseng/L1-L2-checks branch

September 25, 2024 10:28

martinvol mentioned this pull request

Fix stCelo compatibility #11237

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet