-
Notifications
You must be signed in to change notification settings - Fork 99
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
Changes from 25 commits
978cd8f
2bc6d75
c8812ef
e33bf9b
7bd7f4b
2b2411c
d7a352d
584e320
59c2109
23b0fb0
ec4b02b
9777b99
b9c7022
9303444
3a264c1
955a69d
1f171dd
f237074
8800893
cee5a6f
f8c0210
5124d9d
6a34dae
548b28c
5df564c
83f1a70
9d1b2b1
e1b7e25
178e680
48db5c4
5f3a988
638ed78
0cc8946
2821408
63fa46d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,8 +53,11 @@ 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; | ||
mapping(address account => Node node) public nodes; | ||
|
||
/// Total stake locked; | ||
uint256 public totalStake; | ||
|
@@ -103,33 +106,45 @@ contract StakeTable is AbstractStakeTable { | |
return keccak256(abi.encode(blsVK.x0, blsVK.x1, blsVK.y0, blsVK.y1)); | ||
} | ||
|
||
/// @dev Compares two BLS keys for equality | ||
/// @param a First BLS key | ||
/// @param b Second BLS key | ||
/// @return True if the keys are equal, false otherwise | ||
function _isEqualBlsKey(BN254.G2Point memory a, BN254.G2Point memory b) | ||
public | ||
pure | ||
returns (bool) | ||
{ | ||
return BN254.BaseField.unwrap(a.x0) == BN254.BaseField.unwrap(b.x0) | ||
&& BN254.BaseField.unwrap(a.x1) == BN254.BaseField.unwrap(b.x1) | ||
&& BN254.BaseField.unwrap(a.y0) == BN254.BaseField.unwrap(b.y0) | ||
&& BN254.BaseField.unwrap(a.y1) == BN254.BaseField.unwrap(b.y1); | ||
} | ||
|
||
/// TODO handle this logic more appropriately when epochs are re-introduced | ||
/// @dev Fetches the current epoch from the light client contract. | ||
/// @return current epoch (computed from the current block) | ||
function currentEpoch() public pure returns (uint64) { | ||
return 0; | ||
} | ||
|
||
/// @notice Look up the balance of `blsVK` | ||
/// @param blsVK BLS public key controlled by the user. | ||
/// @notice Look up the balance of `account` | ||
/// @param account account controlled by the user. | ||
/// @return Current balance owned by the user. | ||
/// TODO modify this according to the current spec | ||
function lookupStake(BN254.G2Point memory blsVK) external view override returns (uint256) { | ||
Node memory node = this.lookupNode(blsVK); | ||
function lookupStake(address account) external view override returns (uint256) { | ||
Node memory node = this.lookupNode(account); | ||
return node.balance; | ||
} | ||
|
||
/// @notice Look up the full `Node` state associated with `blsVK` | ||
/// @dev The lookup is achieved by hashing first the four field elements of blsVK using | ||
/// keccak256. | ||
/// @return Node indexed by blsVK | ||
/// TODO modify this according to the current spec | ||
function lookupNode(BN254.G2Point memory blsVK) external view override returns (Node memory) { | ||
return nodes[_hashBlsKey(blsVK)]; | ||
/// @notice Look up the full `Node` state associated with `account` | ||
/// @return Node indexed by account | ||
function lookupNode(address account) external view override returns (Node memory) { | ||
return nodes[account]; | ||
} | ||
|
||
/// @notice Get the next available epoch and queue size in that epoch | ||
/// TODO modify this according to the current spec | ||
/// TODO modify this according to the current spec | ||
function nextRegistrationEpoch() external view override returns (uint64, uint64) { | ||
uint64 epoch; | ||
uint64 queueSize; | ||
|
@@ -152,19 +167,22 @@ contract StakeTable is AbstractStakeTable { | |
// @param queueSize current size of the registration queue (after insertion of new element in | ||
// the queue) | ||
/// TODO modify this according to the current spec | ||
/// TODO modify this according to the current spec | ||
function appendRegistrationQueue(uint64 epoch, uint64 queueSize) private { | ||
firstAvailableRegistrationEpoch = epoch; | ||
_numPendingRegistrations = queueSize + 1; | ||
} | ||
|
||
/// @notice Get the number of pending registration requests in the waiting queue | ||
/// TODO modify this according to the current spec | ||
/// TODO modify this according to the current spec | ||
function numPendingRegistrations() external view override returns (uint64) { | ||
return _numPendingRegistrations; | ||
} | ||
|
||
/// @notice Get the next available epoch for exit and queue size in that epoch | ||
/// TODO modify this according to the current spec | ||
/// TODO modify this according to the current spec | ||
function nextExitEpoch() external view override returns (uint64, uint64) { | ||
uint64 epoch; | ||
uint64 queueSize; | ||
|
@@ -186,13 +204,15 @@ contract StakeTable is AbstractStakeTable { | |
// @param epoch next available exit epoch | ||
// @param queueSize current size of the exit queue (after insertion of new element in the queue) | ||
/// TODO modify this according to the current spec | ||
/// TODO modify this according to the current spec | ||
function appendExitQueue(uint64 epoch, uint64 queueSize) private { | ||
firstAvailableExitEpoch = epoch; | ||
_numPendingExits = queueSize + 1; | ||
} | ||
|
||
/// @notice Get the number of pending exit requests in the waiting queue | ||
/// TODO modify this according to the current spec | ||
/// TODO modify this according to the current spec | ||
function numPendingExits() external view override returns (uint64) { | ||
return _numPendingExits; | ||
} | ||
|
@@ -212,6 +232,7 @@ contract StakeTable is AbstractStakeTable { | |
/// @param node node which is assigned an exit escrow period. | ||
/// @return Number of epochs post exit after which funds can be withdrawn. | ||
/// TODO modify this according to the current spec | ||
/// TODO modify this according to the current spec | ||
function exitEscrowPeriod(Node memory node) public pure returns (uint64) { | ||
if (node.balance > 100) { | ||
return 10; | ||
|
@@ -254,8 +275,7 @@ contract StakeTable is AbstractStakeTable { | |
revert InsufficientStakeAmount(amount); | ||
} | ||
|
||
bytes32 key = _hashBlsKey(blsVK); | ||
Node memory node = nodes[key]; | ||
Node memory node = nodes[msg.sender]; | ||
|
||
// Verify that the node is not already registered. | ||
if (node.account != address(0x0)) { | ||
|
@@ -299,28 +319,28 @@ contract StakeTable is AbstractStakeTable { | |
// Create an entry for the node. | ||
node.account = msg.sender; | ||
node.balance = fixedStakeAmount; | ||
node.blsVK = blsVK; | ||
node.schnorrVK = schnorrVK; | ||
node.registerEpoch = registerEpoch; | ||
|
||
nodes[key] = node; | ||
nodes[msg.sender] = node; | ||
|
||
emit Registered(key, registerEpoch, fixedStakeAmount); | ||
emit Registered(msg.sender, registerEpoch, fixedStakeAmount); | ||
} | ||
|
||
/// @notice Deposit more stakes to registered keys | ||
/// @dev TODO this implementation will be revisited later. See | ||
/// https://github.com/EspressoSystems/espresso-sequencer/issues/806 | ||
/// @dev TODO modify this according to the current spec | ||
/// @param blsVK The BLS verification key | ||
/// @param amount The amount to deposit | ||
/// @return (newBalance, effectiveEpoch) the new balance effective at a future epoch | ||
function deposit(BN254.G2Point memory blsVK, uint256 amount) | ||
external | ||
override | ||
returns (uint256, uint64) | ||
{ | ||
bytes32 key = _hashBlsKey(blsVK); | ||
Node memory node = nodes[key]; | ||
function deposit(uint256 amount) external override returns (uint256, uint64) { | ||
Node memory node = nodes[msg.sender]; | ||
|
||
// if the node is not registered, revert | ||
if (node.account == address(0)) { | ||
revert NodeNotRegistered(); | ||
} | ||
|
||
// The deposit must come from the node's registered account. | ||
if (node.account != msg.sender) { | ||
|
@@ -338,23 +358,26 @@ contract StakeTable is AbstractStakeTable { | |
revert ExitRequestInProgress(); | ||
} | ||
|
||
nodes[key].balance += amount; | ||
nodes[msg.sender].balance += amount; | ||
SafeTransferLib.safeTransferFrom(ERC20(tokenAddress), msg.sender, address(this), amount); | ||
|
||
emit Deposit(_hashBlsKey(blsVK), uint256(amount)); | ||
emit Deposit(msg.sender, uint256(amount)); | ||
|
||
uint64 effectiveEpoch = _currentEpoch + 1; | ||
|
||
return (nodes[key].balance, effectiveEpoch); | ||
return (nodes[msg.sender].balance, effectiveEpoch); | ||
} | ||
|
||
/// @notice Request to exit from the stake table, not immediately withdrawable! | ||
/// | ||
/// @dev TODO modify this according to the current spec | ||
/// @param blsVK The BLS verification key to exit | ||
function requestExit(BN254.G2Point memory blsVK) external override { | ||
bytes32 key = _hashBlsKey(blsVK); | ||
Node memory node = nodes[key]; | ||
function requestExit() external override { | ||
Node memory node = nodes[msg.sender]; | ||
|
||
// if the node is not registered, revert | ||
alysiahuggins marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (node.account == address(0)) { | ||
revert NodeNotRegistered(); | ||
} | ||
|
||
// The exit request must come from the node's withdrawal account. | ||
if (node.account != msg.sender) { | ||
|
@@ -374,60 +397,134 @@ contract StakeTable is AbstractStakeTable { | |
|
||
// Prepare the node to exit. | ||
(uint64 exitEpoch, uint64 queueSize) = this.nextExitEpoch(); | ||
nodes[key].exitEpoch = exitEpoch; | ||
nodes[msg.sender].exitEpoch = exitEpoch; | ||
|
||
appendExitQueue(exitEpoch, queueSize); | ||
|
||
emit Exit(key, exitEpoch); | ||
emit Exit(msg.sender, exitEpoch); | ||
} | ||
|
||
/// @notice Withdraw from the staking pool. Transfers occur! Only successfully exited keys can | ||
/// withdraw past their `exitEpoch`. | ||
/// | ||
/// @param blsVK The BLS verification key to withdraw | ||
/// @param blsSig The BLS signature that authenticates the ethereum account this function is | ||
/// called from the caller | ||
/// @return The total amount withdrawn, equal to `Node.balance` associated with `blsVK` | ||
/// TODO: This function should be tested | ||
/// TODO modify this according to the current spec | ||
|
||
function withdrawFunds(BN254.G2Point memory blsVK, BN254.G1Point memory blsSig) | ||
external | ||
override | ||
returns (uint256) | ||
{ | ||
bytes32 key = _hashBlsKey(blsVK); | ||
Node memory node = nodes[key]; | ||
function withdrawFunds() external override returns (uint256) { | ||
Node memory node = nodes[msg.sender]; | ||
|
||
// Verify that the node is already registered. | ||
if (node.account == address(0)) { | ||
revert NodeNotRegistered(); | ||
} | ||
|
||
// The exit request must come from the node's withdrawal account. | ||
if (node.account != msg.sender) { | ||
revert Unauthenticated(); | ||
} | ||
|
||
// Verify that the balance is greater than zero | ||
uint256 balance = node.balance; | ||
if (balance == 0) { | ||
revert InsufficientStakeBalance(0); | ||
} | ||
|
||
// Verify that the validator can sign for that blsVK | ||
bytes memory message = abi.encode(msg.sender); | ||
BLSSig.verifyBlsSig(message, blsSig, blsVK); | ||
|
||
// Verify that the exit escrow period is over. | ||
if (currentEpoch() < node.exitEpoch + exitEscrowPeriod(node)) { | ||
revert PrematureWithdrawal(); | ||
} | ||
|
||
// Delete the node from the stake table. | ||
delete nodes[key]; | ||
delete nodes[msg.sender]; | ||
|
||
// Transfer the balance to the node's account. | ||
SafeTransferLib.safeTransfer(ERC20(tokenAddress), node.account, balance); | ||
|
||
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 | ||
philippecamacho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// @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: consider emitting the changed keys | ||
function updateConsensusKeys( | ||
philippecamacho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
BN254.G2Point memory newBlsVK, | ||
EdOnBN254.EdOnBN254Point memory newSchnorrVK, | ||
BN254.G1Point memory newBlsSig | ||
) external override { | ||
Node memory node = nodes[msg.sender]; | ||
|
||
// 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 key change | ||
if ( | ||
( | ||
_isEqualBlsKey(newBlsVK, node.blsVK) | ||
&& EdOnBN254.isEqual(newSchnorrVK, node.schnorrVK) | ||
) | ||
|| ( | ||
_isEqualBlsKey( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we verify the BLS sig, is it necessary to check that the key is non-zero? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see it's allowed to be zero if only the schnorr key is updated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. now any zero keys cause a revert 638ed78 |
||
newBlsVK, | ||
BN254.G2Point( | ||
BN254.BaseField.wrap(0), | ||
BN254.BaseField.wrap(0), | ||
BN254.BaseField.wrap(0), | ||
BN254.BaseField.wrap(0) | ||
) | ||
) && EdOnBN254.isEqual(newSchnorrVK, EdOnBN254.EdOnBN254Point(0, 0)) | ||
) | ||
) { | ||
revert NoKeyChange(); | ||
} | ||
|
||
// Update the node's schnorr key once it's not the same as the old one and it's nonzero | ||
if ( | ||
!EdOnBN254.isEqual(newSchnorrVK, node.schnorrVK) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e1b7e25 so i didn't add the isEqual function to the @philippecamacho shall I add |
||
&& !EdOnBN254.isEqual(newSchnorrVK, EdOnBN254.EdOnBN254Point(0, 0)) | ||
) { | ||
node.schnorrVK = newSchnorrVK; | ||
} | ||
|
||
// Update the node's bls key once it's not the same as the old one and it's nonzero | ||
if ( | ||
!_isEqualBlsKey(newBlsVK, node.blsVK) | ||
&& !_isEqualBlsKey( | ||
newBlsVK, | ||
BN254.G2Point( | ||
BN254.BaseField.wrap(0), | ||
BN254.BaseField.wrap(0), | ||
BN254.BaseField.wrap(0), | ||
BN254.BaseField.wrap(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[msg.sender] = node; | ||
|
||
// Emit the event | ||
emit UpdatedConsensusKeys(msg.sender); | ||
} | ||
|
||
/// @notice Minimum stake amount | ||
/// @return Minimum stake amount | ||
/// TODO: This value should be a variable modifiable by admin | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I understand correctly that we check that the keys are not zero when if consensus keys are updated but we don't check for that during registration? Is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we don't need to check the blsVk because we verify a signature but what about the schnorrVk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes you're right, there should be checks on the vk(s) here too!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added checks on zero point keys e1b7e25