feat: add stake table interfaces contract #743
Conversation
alxiong
requested review from
mrain,
sveitser,
philippecamacho,
nomaxg,
jbearer and
chancharles92
| function register( | ||
| BN254.G1Point calldata blsVK, | ||
| EdOnBN254.EdOnBN254Point calldata schnorrVK, | ||
| uint64 amount, |
There was a problem hiding this comment.
Is uint64 type sufficient for amount?
| /// @param balance The amount of token staked. | ||
| /// @param registerEpoch The starting epoch for the validator. | ||
| /// @param exitEpoch The ending epoch for the validator. | ||
| struct Node { |
There was a problem hiding this comment.
This struct should contain the BLS and Schnorr public keys right?
There was a problem hiding this comment.
Half correct, should contains Schnorr key (and other auxiliary info)
there would be a mapping in storage keeping track of mapping(BlsVerKey => Node), so no need to store BLS key again in the Node struct.
| function frozenStakeTableCommitment() external view returns (bytes32); | ||
|
|
||
| /// @notice Get the total stake of the registered keys in the latest stake table (Head) | ||
| function totalStake() external view returns (uint256); |
There was a problem hiding this comment.
Should we differentiate between native and restake?
There was a problem hiding this comment.
very interesting point! I see these options:
- keep as it is, let instantiations specify its own conversion, the output is neither native nor restaked total stake, but an interpreted/exchange-rate-weighted total stake
- returns
(uint256, uint256), first one for native, second one for restaked - returns
(StakeType[] memory, uint256[] memory)
Option 3 is the most flexible and future-proof API, since it allows StakeType to be extended to more types (e.g. EthRestaked, BtcRestaked, ..., but Option 2 is most straightforward if the assumption that we only support ETH and native token is valid for the foreseeable future.
WDYT?
| /// @notice Advance to the next epoch, the new `votingStakeTableCommitment` will be | ||
| /// `frozenStakeTableCommitment` and the new `frozenStakeTableCommitment` will be | ||
| /// `curEpochComm`, | ||
| function advanceEpoch(bytes32 curEpochComm) external returns (bool); |
There was a problem hiding this comment.
Who calls this function and why?
There was a problem hiding this comment.
Think of this API as the "setter for stake commitments" (as we have getter as votingStakeTableCommitment() and frozenStakeTableCommitment())
So light client prover will submit new states and call this function to update the new finalized stake table commitments.
|
I'm sorry I messed up and push to the wrong remote branch, somehow when I revert changes, Github recognize it as "merged" ... I will try to open a different PR to address comments above 😱 this is such weird behavior, I cannot submit a PR on this branch anymore. not sure why... |
* Bump HotShot --------- Co-authored-by: imabdulbasit <[email protected]>
Closes #673
note: I'm currently targeting
plonk-verifierbranch for easier git diff for the reviewers, and will change tomainonce #722 is merged