Feat/delay logout #56
Conversation
|
@ChihChengLiang @karlfloersch @vbuterin Parameterizing logout delay is ready for review |
| self.base_penalty_factor = _base_penalty_factor | ||
| self.min_deposit_size = _min_deposit_size | ||
|
|
||
| # helper contracts |
There was a problem hiding this comment.
like the reorder of these variables ❤️
casper/contracts/simple_casper.v.py
Outdated
| self.next_dynasty_wei_delta -= reward | ||
| if current_dynasty == end_dynasty - 2: | ||
| self.second_next_dynasty_wei_delta -= reward | ||
| if end_dynasty < self.default_end_dynasty: |
There was a problem hiding this comment.
This would happen in the logout, right? A comment would be nice to have here.
There was a problem hiding this comment.
yes, if the validator is in logout period. I'll make a comment!
tests/test_unit_tests.py
Outdated
| (-1), | ||
| ] | ||
| ) | ||
| def test_dynasty_wei_delta_defaults(casper, dynasty): |
There was a problem hiding this comment.
Not sure the purpose of this test
There was a problem hiding this comment.
Was a sanity check when I added the new var early in dev. Will remove
|
ahah... sorry this needs to upgrade to new Vyper style too. |
|
Yeah, I'm waiting to merge this before vyper 0.0.4 gets merged. Will port
in changes from master then.
…On Thursday, March 29, 2018, Chih Cheng Liang ***@***.***> wrote:
ahah... sorry this needs to upgrade to new style too.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#56 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABXf-2DwbWNlxSg7CbQfXhDJl_DFUeY9ks5tjH_7gaJpZM4S2dgE>
.
|
|
Also waiting on a review from @vbuterin
…On Thursday, March 29, 2018, danny ryan ***@***.***> wrote:
Yeah, I'm waiting to merge this before vyper 0.0.4 gets merged. Will port
in changes from master then.
On Thursday, March 29, 2018, Chih Cheng Liang ***@***.***>
wrote:
> ahah... sorry this needs to upgrade to new style too.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#56 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/ABXf-2DwbWNlxSg7CbQfXhDJl_DFUeY9ks5tjH_7gaJpZM4S2dgE>
> .
>
|
|
@ChihChengLiang That merge was surprisingly not fun. But it's done |
|
Also, I fixed #67 here. |
I'm so sorry for that merge. will wait longer after the review. |
This PR adds
dynasty_logout_delayas a parameter in the casper contract to address #51. We can now set the dynasty delay when deploying the contract rather than having to use the built in 2 dynasties.This PR:
_dynasty_logout_delayto contract params anddynasty_logout_delayas a constant in the contractdynasty_wei_deltamap as variable to the contract as a replacement tonext_dynasty_wei_deltaandsecond_next_dynasty_wei_delta. This maps from a dynasty number to the +/- wei to change in total deposits for that dynasty number.default_end_dynastyas a constant to the contract because we use the huge number in a conditional as well as the original assignment for a validatorlogouttestsSome things to note:
dynasty_wei_deltafrom a 0 pays 20000 gas as opposed to subsequent writings costing 4000. This can make certain operations like logging out cost slightly different gas costs depending on if you are the first one to do something for that end_dynasty.2for the logout delay can be used as the param if we happened to decide not to make it a longer delay.dynasty_wei_deltato update the current dynasty deposits inincrement_dynasty, we could potentially 0 this out if that helps with the state in the EVM. I'm not 100% sure if this actually frees up space. This operation occurs inincrement_dynastywithininitialize_epochso it wouldn't cost any user for that write operation.Note all tests in the corresponding pyethapp repo currently fail. The contract currently fails to deploy over there because of the additional
dynasty_logout_delayparameter. I'll add a PR to fix that soon. That said, all tests pass in this local repo and the contract changes are ready for review.