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

2370 staketable update validator identification keys #2373

Merged
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
978cd8f
add stake table tests
alysiahuggins Dec 5, 2024
2bc6d75
remove stake types
alysiahuggins Dec 5, 2024
c8812ef
verify token allowance, balance and reprioritize verification order o…
alysiahuggins Dec 5, 2024
e33bf9b
set the fixed stake amount, added related tests, updated data types
alysiahuggins Dec 5, 2024
7bd7f4b
add more verification checks to the withdraw function
alysiahuggins Dec 5, 2024
2b2411c
updated errror types
alysiahuggins Dec 6, 2024
d7a352d
Merge branch 'main' into 2304-stake-table-registration
alysiahuggins Dec 6, 2024
584e320
added TODO statements in comments to be explicit about outdated funct…
alysiahuggins Dec 6, 2024
59c2109
a validator/node is identified by the msg.sender, blskey and schnorrk…
alysiahuggins Dec 6, 2024
23b0fb0
Merge branch 'main' into 2304-stake-table-registration
alysiahuggins Dec 6, 2024
ec4b02b
Merge branch '2304-stake-table-registration' into 2370-staketable-upd…
alysiahuggins Dec 6, 2024
9777b99
merged from the fixed stake branch and modified test to include the i…
alysiahuggins Dec 6, 2024
b9c7022
not required to get the blsSig for a withdrawal since we have the eth…
alysiahuggins Dec 6, 2024
9303444
add the ability to update consensus keys
alysiahuggins Dec 6, 2024
3a264c1
add the ability to update consensus keys
alysiahuggins Dec 6, 2024
955a69d
merge with main
alysiahuggins Dec 6, 2024
1f171dd
remove vscode settings
alysiahuggins Dec 6, 2024
f237074
add test or happy path of updating consensus keys
alysiahuggins Dec 9, 2024
8800893
added unhappy path tests on based on no key changes and changed the b…
alysiahuggins Dec 9, 2024
cee5a6f
added comment to newly added function on StakeTable and added tests t…
alysiahuggins Dec 9, 2024
f8c0210
change the lookup node and lookup stake functions to use their ethere…
alysiahuggins Dec 9, 2024
5124d9d
updated updateConsensusKeys function, enhance test coverage
alysiahuggins Dec 9, 2024
6a34dae
added todo
alysiahuggins Dec 10, 2024
548b28c
Merge branch 'main' into 2370-staketable-update-validator-identificat…
alysiahuggins Dec 10, 2024
5df564c
updated test to use the new seed so that a new schnorr key could be g…
alysiahuggins Dec 11, 2024
83f1a70
Update contracts/src/StakeTable.sol
alysiahuggins Jan 6, 2025
9d1b2b1
Merge branch 'main' into 2370-staketable-update-validator-identificat…
alysiahuggins Jan 7, 2025
e1b7e25
check for invalid bls and schnorr keys on register
alysiahuggins Jan 7, 2025
178e680
Merge branch 'main' into 2370-staketable-update-validator-identificat…
alysiahuggins Jan 7, 2025
48db5c4
update test comments so that they are clearer
alysiahuggins Jan 7, 2025
5f3a988
updateConsensusKeys reverts when the keys the same as the old ones, n…
alysiahuggins Jan 8, 2025
638ed78
updating consensus keys revert if any VK is a zero point VK
alysiahuggins Jan 9, 2025
0cc8946
clarifies that the bls sig proves
alysiahuggins Jan 9, 2025
2821408
Merge branch 'main' into 2370-staketable-update-validator-identificat…
alysiahuggins Jan 10, 2025
63fa46d
Merge branch 'main' into 2370-staketable-update-validator-identificat…
alysiahuggins Jan 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add the ability to update consensus keys
alysiahuggins committed Dec 6, 2024
commit 930344450c5cd881a3e70bc34159faa10d41e9f0
78 changes: 78 additions & 0 deletions contracts/src/StakeTable.sol
Original file line number Diff line number Diff line change
@@ -53,6 +53,9 @@ contract StakeTable is AbstractStakeTable {
// Error raised when the staker does not register with the correct stakeAmount
error InsufficientStakeAmount(uint256);

// Error raised when the staker does not provide a new key
error NoKeyChange();

/// Mapping from a hash of a BLS key to a node struct defined in the abstract contract.
mapping(bytes32 keyHash => Node node) public nodes;

@@ -419,6 +422,81 @@ contract StakeTable is AbstractStakeTable {
return balance;
}

/// @notice Update the consensus keys for a validator
/// @dev This function is used to update the consensus keys for a validator
/// @dev This function can only be called by the validator itself when it's not in the exit
/// queue
/// @dev The validator will need to give up either its old BLS key and/or old Schnorr key
/// @dev The validator will need to provide a BLS signature over the new BLS key
/// @param currBlsVK The current BLS verification key
/// @param newBlsVK The new BLS verification key
/// @param newSchnorrVK The new Schnorr verification key
/// @param newBlsSig The BLS signature that the account owns the new BLS key
/// TODO: This function should be tested
function updateConsensusKeys(
BN254.G2Point memory currBlsVK,
BN254.G2Point memory newBlsVK,
EdOnBN254.EdOnBN254Point memory newSchnorrVK,
BN254.G1Point memory newBlsSig
) external override {
bytes32 currBlsKey = _hashBlsKey(currBlsVK);
bytes32 newBlsKey = _hashBlsKey(newBlsVK);
Node memory node = nodes[currBlsKey];

// Verify that the msg.sender is the validator's staking address
if (msg.sender != node.account) {
revert Unauthenticated();
}

// Verify that the node is already registered.
if (node.account == address(0)) {
revert NodeNotRegistered();
}

// Verify that the node is not in the exit queue
if (node.exitEpoch != 0) {
revert ExitRequestInProgress();
}

// The staker does not provide a new key
if (
(newBlsKey == currBlsKey && EdOnBN254.isEqual(newSchnorrVK, node.schnorrVK))
|| (
newBlsKey == bytes32(0)
&& EdOnBN254.isEqual(newSchnorrVK, EdOnBN254.EdOnBN254Point(0, 0))
)
) {
revert NoKeyChange();
}

// Update the node's schnorr key
if (
!EdOnBN254.isEqual(newSchnorrVK, node.schnorrVK)
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the EdOnBn254 and BN254 types, can we use using ... for ... to make the code a bit more readable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e1b7e25 so i didn't add the isEqual function to the BN254 library since it was an external library but i did add it to the EdOnBn254 library so i added the using ... for ... syntax for that.

@philippecamacho shall I add isEqual and a zero point BN254 constant to the BN254 codebase? Also is it a good idea to add a zero point EdOnBN254 constant? Zero point checks have been added to the register and updateConsensusKeys functions.

&& !EdOnBN254.isEqual(newSchnorrVK, EdOnBN254.EdOnBN254Point(0, 0))
) {
node.schnorrVK = newSchnorrVK;
}

// Update the node's bls key
if (newBlsKey != currBlsKey && newBlsKey != bytes32(0)) {
// Verify that the validator can sign for that blsVK
bytes memory message = abi.encode(msg.sender);
BLSSig.verifyBlsSig(message, newBlsSig, newBlsVK);

// Update the node's bls key
node.blsVK = newBlsVK;
}

// Update the node in the stake table
nodes[newBlsKey] = node;

// Delete the old node from the stake table
delete nodes[currBlsKey];

// Emit the event
emit ConsensusKeysUpdated(msg.sender);
}

/// @notice Minimum stake amount
/// @return Minimum stake amount
/// TODO: This value should be a variable modifiable by admin
21 changes: 21 additions & 0 deletions contracts/src/interfaces/AbstractStakeTable.sol
Original file line number Diff line number Diff line change
@@ -39,6 +39,10 @@ abstract contract AbstractStakeTable {
/// @param amount amount of the deposit
event Deposit(bytes32 blsVKhash, uint256 amount);

/// @notice Signals a consensus key update for a validator
/// @param stakingAddr the address of the validator
event ConsensusKeysUpdated(address stakingAddr);

/// @dev (sadly, Solidity doesn't support type alias on non-primitive types)
// We avoid declaring another struct even if the type info helps with readability,
// extra layer of struct introduces overhead and more gas cost.
@@ -130,4 +134,21 @@ abstract contract AbstractStakeTable {
/// @param blsVK The BLS verification key to withdraw
/// @return The total amount withdrawn, equal to `Node.balance` associated with `blsVK`
function withdrawFunds(BN254.G2Point memory blsVK) external virtual returns (uint256);

/// @notice Update the consensus keys for a validator
/// @dev This function is used to update the consensus keys for a validator
/// @dev This function can only be called by the validator itself when it's not in the exit
/// queue
/// @dev The validator will need to give up either its old BLS key and/or old Schnorr key
/// @dev The validator will need to provide a BLS signature over the new BLS key
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above. The message signed is the address of the validator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

clarified the comment here 0cc8946

/// @param currBlsVK The current BLS verification key
/// @param newBlsVK The new BLS verification key
/// @param newSchnorrVK The new Schnorr verification key
/// @param newBlsSig The BLS signature that the account owns the new BLS key
function updateConsensusKeys(
BN254.G2Point memory currBlsVK,
BN254.G2Point memory newBlsVK,
EdOnBN254.EdOnBN254Point memory newSchnorrVK,
BN254.G1Point memory newBlsSig
) external virtual;
}
9 changes: 9 additions & 0 deletions contracts/src/libraries/EdOnBn254.sol
Original file line number Diff line number Diff line change
@@ -35,5 +35,14 @@ library EdOnBN254 {
return abi.encodePacked(Utils.reverseEndianness(point.x | mask));
}

/// @dev Check if two points are equal
function isEqual(EdOnBN254Point memory a, EdOnBN254Point memory b)
internal
pure
returns (bool)
{
return a.x == b.x && a.y == b.y;
}

// TODO: (alex) add `validatePoint` methods and tests
}