-
Notifications
You must be signed in to change notification settings - Fork 283
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
feat: lock to propose #9430
feat: lock to propose #9430
Conversation
@@ -154,7 +154,7 @@ contract GetProposalStateTest is ApellaBase { | |||
// We can overwrite the quorum to be 0 to hit an invalid case | |||
assertGt(apella.getProposal(proposalId).config.quorum, 0); | |||
bytes32 slot = | |||
bytes32(uint256(keccak256(abi.encodePacked(uint256(proposalId), uint256(1)))) + 4); | |||
bytes32(uint256(keccak256(abi.encodePacked(uint256(proposalId), uint256(1)))) + 6); |
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.
forge StdStorage would make this easier to update
@@ -0,0 +1,8 @@ | |||
ProposeWithLockTest | |||
├── when caller have insufficient power |
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.
typo: have -> has
|
||
return true; | ||
_initiateWithdraw(_to, amount, configuration.proposeConfig.lockDelay); |
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.
Perhaps we can make it more explicit that this delay is longer than votingDelay + votingDuration
to prevent someone from voting on their own emergency proposal. Right now this is the same delay as any other withdrawal.
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.
It is not the same delay as any other withdrawal, have renamed it to make it more clear as I could see the confusion.
3f07135
to
db70936
Compare
2727db9
to
adaf52d
Compare
Changes to public function bytecode sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
@@ -37,6 +37,10 @@ contract Apella is IApella { | |||
gerousia = _gerousia; | |||
|
|||
configuration = DataStructures.Configuration({ | |||
proposeConfig: DataStructures.ProposeConfiguration({ | |||
lockDelay: Timestamp.wrap(3600), |
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.
make an issue to update the delays here
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.
* really be using the proposal logic in Gerousia, which might have a similar | ||
* mechanism in place as well. | ||
* It is here for emergency purposes. | ||
* Using the lock should be a last resort if the Gerousia is broken. |
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.
Maybe mention that gerousia is broken if no proposals are made through it, its not immediately clear that this means validators are no longer able to make proposals
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.
The "Gerousia" can also be seen as broken if it is not agreeing with what token holders want, so it is also a way to bypass it 🤷.
Fixes #9348