-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix to issue #27 #29
Fix to issue #27 #29
Changes from 2 commits
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 | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -294,9 +294,15 @@ func withdraw(validator_address, delegator_address) | |||||||||||||||||||||||||||||
var delunbonds = {<start,end,amount> | amount = unbonds[delegator_address][validator_address].deltas[(start, end)] > 0 && end <= cur_epoch } | ||||||||||||||||||||||||||||||
//substract any pending slash before withdrawing | ||||||||||||||||||||||||||||||
forall (<start,end,amount> in selfunbonds) do | ||||||||||||||||||||||||||||||
var amount_after_slashing = amount | ||||||||||||||||||||||||||||||
forall (slash in slashes[validator_address] s.t. start <= slash.epoch && slash.epoch <= end) | ||||||||||||||||||||||||||||||
amount_after_slashing -= amount*slash.rate | ||||||||||||||||||||||||||||||
var updated_amount = amount | ||||||||||||||||||||||||||||||
var slashed_amount = 0 | ||||||||||||||||||||||||||||||
var previous_slash_epoch = -1 | ||||||||||||||||||||||||||||||
forall (slash in slashes[validator_address] in slash.epoch order s.t. start <= slash.epoch && slash.epoch <= end) | ||||||||||||||||||||||||||||||
if previous_slash_epoch == -1 || previous_slash_epoch + unbonding_length < slash.epoch do | ||||||||||||||||||||||||||||||
updated_amount = amount - slashed_amount | ||||||||||||||||||||||||||||||
slashed_amount += updated_amount*slash.rate | ||||||||||||||||||||||||||||||
previous_slash_epoch = slash.epoch | ||||||||||||||||||||||||||||||
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. I think a naive approach that works for more than 2 slashes (e.g. 3 slashes where no 2 consecutive slashes have
Suggested change
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.
I think it does not work. I tried to run it with 4 infractions that occurred at epochs
I think running your pseudocode, we get the slashed amount of infraction 4 wrong:
I may have made a mistake in the calculations (or maybe there is a bug in your pseudocode). I am proposing an alternative that I think works (maybe not the most efficient solution): forall (<start,end,amount> in selfunbonds) do
var computed_amounts = {}
forall (slash in slashes in slash.epoch order s.t. start <= slash.epoch && slash.epoch <= end) do
var updated_amount = amount
//Update amount with slashes that happened more than `unbonding_length` before this slash
forall (slashed_amount in computed_amounts s.t. slashed_amount.epoch + unbonding_length < slash.epoch) do
updated_amount -= slashed_amount.amount
computed_amounts = add(computed_amounts, SlashedAmount{epoch: slash.epoch, amount: updated_amount*slash.rate})
var amount_after_slashing = amount - sum({computed_amount.amount | computed_amount in computed_amounts})
balance[delegator_address] += amount_after_slashing
balance[pos] -= amount_after_slashing @tzemanovic what do you think? 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. A couple of minor changes that improve things a bit: var computed_amounts = {}
var updated_amount = amount
forall (slash in slashes in slash.epoch order s.t. start <= slash.epoch && slash.epoch <= end) do
//Update amount with slashes that happened more than `unbonding_length` before this slash
forall (slashed_amount in computed_amounts s.t. slashed_amount.epoch + unbonding_length < slash.epoch) do
updated_amount -= slashed_amount.amount
computed_amounts = computed_amounts \ {slashed_amount}
computed_amounts = computed_amounts \union {SlashedAmount{epoch: slash.epoch, amount: updated_amount*slash.rate}}
var amount_after_slashing = updated_amount - sum({computed_amount.amount | computed_amount in computed_amounts})
balance[delegator_address] += amount_after_slashing
balance[pos] -= amount_after_slashing 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! I think I was meaning to do
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.
Yes!
Awesome, I'll work on the TLA+ version :) |
||||||||||||||||||||||||||||||
var amount_after_slashing = amount - slashed_amount | ||||||||||||||||||||||||||||||
balance[delegator_address] += amount_after_slashing | ||||||||||||||||||||||||||||||
balance[pos] -= amount_after_slashing | ||||||||||||||||||||||||||||||
//remove unbond | ||||||||||||||||||||||||||||||
|
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.
Was this mistakenly removed? L305 below updates
amount_after_slashing
, except now this variable isn't declared explicitly anywhereThere 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.
My fault, the intention is that line 305 declares it (just missing the
var
keyword). Fixed. Thanks ;)