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

Allow Validator registration in L2 without BLS key #11181

Merged
merged 5 commits into from
Aug 19, 2024

Conversation

m-chrzan
Copy link
Contributor

@m-chrzan m-chrzan commented Aug 8, 2024

Description

BLS keys were used in CEL1 for PoS signatures. They're no longer needed in CEL2.

Other changes

  • Reallowed registerValidatorGroup, updateEcdsaPublicKey in L2.
  • isValidator check now checks the ECDSA key rather than the BLS key.
  • In MockElection, allow setAllowedToVoteOverMaxNumberOfGroups in L2.

Tested

Unit tests.

Related issues

Backwards compatibility

Introduces a new function that works only in L2.

@m-chrzan m-chrzan requested a review from a team as a code owner August 8, 2024 20:13
Copy link

openzeppelin-code bot commented Aug 8, 2024

Allow Validator registration in L2 without BLS key

Generated at commit: 7a413bfd3bd79548a6035044259d58b12534e3a9

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
2
3
0
15
41
61
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

Copy link
Contributor

@soloseng soloseng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 🚀

Left just one comment.

Copy link

gitguardian bot commented Aug 19, 2024

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
10538986 Triggered Generic High Entropy Secret 318e618 packages/protocol/scripts/foundry/constants.sh View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@martinvol martinvol merged commit 05dcde2 into feat/l2-epoch-system Aug 19, 2024
3 of 5 checks passed
@martinvol martinvol deleted the m-chrzan/validator-without-bls branch August 19, 2024 19:10
soloseng added a commit that referenced this pull request Oct 15, 2024
* first compiling draft
no working test.

Co-authored-by: Martín Volpe <[email protected]>

* Compiler fixes + finishNextEpochProcess

* allocateValidatorsRewards compilable

* First few tests

* test fix

* uisng registry instead

* updated registry ID var names

* typo

* ++ mock contracts in 0.8

* ++ passing test using mock

* Removal of using precompiles

* ScoreManager refactor

* Decouple epoch manager initializer from registry

* isReadyToStartEpoch fix

* startNextEpochProcess fixes

* rename score manager to score reader

* Rename transfer to release on CeloDistribution schedule

* Celo distribution schedule renamed to CeloUnreleasedTreasure

* conditions to getFirstBlockAtEpoch and getLastBlockAtEpoch

* ScoreManager update

* getFirstBlockOfEpoch in EpochManager initializer

* extra checks

* introduce new getValidatorsGroup

* systemAlreadyInitialized update

* Removal of IEpochManagerInterface

* Made CI run on feature branch

* merge fix

* Added target for the CI

* Fixed quote celo-monorepo.yml

* Allow Validator registration in L2 without BLS key (#11181)

* Allow validator registration in L2 without BLS key

* Reallow ECDSA key change in L2

---------

Co-authored-by: Martín Volpe <[email protected]>

* Fix CI

* Fixed interfaces and imports

* Build fix

* lint

* prettify

* prettify 2

* Attempt to fix truffle build and migrations l2 epoch (#11190)

* Added Blocking to LockedGold, Election (#11186)

* add EpochManager to abis

* force deploy abis

* build fix

* Move Validators.sol to 0.8 (#11192)

* WIP

* Validator test WIP, forge doesn't compile yet

* buildable

* most of the foundry tests working

* Validator related tests fixed

* truffle build working

* cache bump

* lint + prettify+ migrations

* Enable optimization for Solidity 0.8

* prettify

* Ci bump

* CI bump 2

* Validators to 0.8 config

* CI bump

* foundry fix

* Truffle migrations are partly fixed

* Added import for ValidatorsMock08 in EpochManager.t.sol, I think it was removed by mistake

* Added yarn.lock

* Changes to mock with full implementation

* Attempt to fix linking libraries not working with deployCodeTo foundry-rs/foundry#4049

* truffle migrations fixed

* CI bump

* lint

* forge test fixes

* artifacts test fix

* lint

* update of foundry version

* add ProxyFactory import to tests

* library linking fix

* Foundry migrations fix

* migration tests fix

* CI bump

* Little cleanup + retrigger CI

* forgot to commit Validators.sol

* Fixed the ABI encoded

* lint

* Fix contract versions

* add Adapter to ignored contracts

* revert of ReentrancyGuard change

* lint fix

* remove adapters from check

* storage layout fix

---------

Co-authored-by: pahor167 <[email protected]>

* force release of canary abi

* force new canary version

* npm tag revert

* Soloseng/update-l2-getepochnumber-logic (#11195)

* disable getEpochSize on L2

* update registry

* update relevant interfaces

* update contracts with L2 `getEpochNumber()` logic

* update tests with L2 `getEpochNumber()` logic

* ++ TODO

* moved constants to constants file

* updated allocated supply function to handle L1 & L2 cases

* made `epochManager.currenEpochNumber()` private, to avoid returning 0 when not initialized.

* PR feedback

* Passing validators test using mockEpochManager for L2 tests

* clean up

* fixed other failing tests

* using mockEpochManager instead of interface.

Fixed failing tests.

* happy linter

* revert npm tag ∆

* ++ TODO and comment

* add score manager to abis

* Split initEpochManager Function (#11199)

* ++ contract function

* ++ comment

* startNextEpochProcess unit & integration test (#11191)

* unit test with mocks

* ++ integration tests

* clean up

* -- logging

* removed duplicate interface

* using `MockCeloToken` to get test to pass.

Fails when it hits a precompile in `EpochRewards.sol`

* removed endEpochTimestamp

* moved IEpochManager to 0.5 folder

* added L2 conditions for EpochRewards functions using precompiles

Still missing tests

* renamed EpochManagerInitializer due to name conflict

* ++ more unit test

* setup anvil migration
fix name conflict

* compiles

* ++ require fund in unreleased treasury

* Updated regex

* ++ registry 0.8 for testing only

* clean up

* ++ unit test

* initial integration test using L1 devchain

* ++ comment

* -- forge based integration test

* ++ to const

* happy linter

* update contract name

* ++ PR feedback

* ++ checks

* updated carbon address

* proxy stableToken mint call via Validators contract

* -- duplicate imports

* removed registry08. replaced with vm call

* PR feedback

* -- coment

* passing unit tests

* clean up

* ++ mintStable test

* -- TODO; compiles test when filtering

* PR feedback

* updated migration script to add more validators

* passing integration test

* removed test for zero amount

* yarn build fix

* clean up comments && TODO

* revert change as out of scope

* E2E EpochManager test + Epoch truffle migrations & Anvil L2 migration build fix (#11198)

* Soloseng/dynamically-fetch-epochmanagerenabler-address (#11207)

* dynamically fetch epochManagerEnabler && carbonOffsettingPartner

++ to registry

* PR feedback

* removed onlyL1 modifier in setter functions

* updated unit test to reflect changes

* fixing tests

* fix test

* fixed migration data

* fixed migration script error

* removed unused modifier

* removed duplicate or unused code

* Implement sending of allocated validator payments (#11197)

* EpochManager fixes (#11208)

* truffle build fix

* build fix

* PR comments

* prettify

* rename of registerValidator overload

* bug fix

* extensing epochManager e2e test

* yarn lint

* Reset pending payment to 0 after sending (#11209)

Reset pending payment to 0 when sending

* Deleted duplicated import

* Make captureEpochAndValidators work in constant time (#11210)

* Use more general interface for token transfer (#11216)

* Epoch manager enabler tests (#11213)

* ++ basic test

* -- celoToken balance check

* cleanup comments

* use celoToken instead of native token for `initializeSystem` balance check

* ++ more test

* removed additional epochs

* Make sendValidatorPayment nonReentrant (#11217)

* Martinvol/return account instead of signer (#11215)

* EpochManager e2e tests add/remove validators between ecpochs (#11214)

* Send validator payment on actions that would modify reward distribution (#11211)

* Rename `CeloUnreleasedTreasure` to `CeloUnreleasedTreasury` (#11220)

* updated `CeloUnrealeasedTreasure` to `CeloUnreleasedTreasury`

* update `CeloUnreleasedTreasury` initial balance to use defined constant

* renamed file

* prevent CELO transfers to `CeloUnreleasedTreasury`

* ++ comment

* unused import

* using internal accounting for released amount

* updated integration test

* PR feedback

* PR feedback

* reverted due to gas consideration

* more revert

---------

Co-authored-by: Martín Volpe <[email protected]>
Co-authored-by: pahor167 <[email protected]>
Co-authored-by: Martín Volpe <[email protected]>
Co-authored-by: Martín Volpe <[email protected]>
Co-authored-by: Martin <[email protected]>
Co-authored-by: pahor167 <[email protected]>
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.

3 participants