Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow slashing only on current era #3411

Merged
merged 4 commits into from
Aug 16, 2019
Merged

Allow slashing only on current era #3411

merged 4 commits into from
Aug 16, 2019

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Aug 15, 2019 •
edited
Loading

Offences from a prior era are discarded, and we track how much of the current era's exposure we've already slashed to avoid over-slashing.

@rphmeier rphmeier added the A0-please_review Pull request needs code review. label Aug 15, 2019
@rphmeier rphmeier requested a review from pepyakin August 15, 2019 16:56
pepyakin
pepyakin reviewed Aug 15, 2019
View reviewed changes
srml/staking/src/lib.rs Show resolved Hide resolved
srml/staking/src/lib.rs

// Handle the rest of imbalances
T::Slash::on_unbalanced(remaining_imbalance);
}
}

/// Filter historical offences out and only allow those from the current era.
pub struct FilterHistoricalOffences<T, R> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not hook it up in node-runtime right away?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to, but I didn't see that ReportOffence was used anywhere

srml/staking/src/lib.rs Show resolved Hide resolved
tomusdrw
tomusdrw reviewed Aug 16, 2019
View reviewed changes
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sensible to me, left couple of questions though.

srml/staking/src/lib.rs
// otherwise, these funds may have already been slashed due to something
// reported from a prior era.
let already_slashed_own = journal.iter()
.filter(|entry| &entry.who == stash)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose stash does not change, right? You can't move to another stash account without unbonding/bonding?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct

srml/staking/src/lib.rs Show resolved Hide resolved
srml/staking/src/lib.rs
@@ -1178,6 +1213,10 @@ impl<T: Trait> Module<T> {

// Increment current era.
let current_era = CurrentEra::mutate(|s| { *s += 1; *s });

// prune journal for last era.
<EraSlashJournal<T>>::remove(current_era - 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we prune era's with some offset (X) and accept reports for last X eras?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but it wouldn't be of any use with this runtime. Historical slashing just isn't doable without more work beyond this milestone.

@gavofyork
Copy link
Member

@rphmeier just waiting on answers to the final queries before i can merge this.

tomusdrw
tomusdrw approved these changes Aug 16, 2019
View reviewed changes
srml/staking/src/lib.rs Show resolved Hide resolved
@tomusdrw tomusdrw merged commit 2d0fe68 into retreat-slashing Aug 16, 2019
@tomusdrw tomusdrw deleted the rh-retreat-slashing-only-current branch August 16, 2019 12:02
gavofyork pushed a commit that referenced this pull request Aug 16, 2019
@tomusdrw @gavofyork
Offences reporting and slashing (#3322)
5d992d8 
* Remove offline slashing logic from staking.

* Initial version of reworked offence module, can report offences

* Clean up staking example.

* Commit SlashingOffence

* Force new era on slash.

* Add offenders in the SlashingOffence trait.

* Introduce the ReportOffence trait.

* Rename `Offence`.

* Add on_before_session_ending handler.

* Move offence related stuff under sr-primitives.

* Fix cargo check.

* Import new im-online implementation.

* Adding validator count to historical session storage as it's needed for slash calculations

* Add a comment about offence.

* Add BabeEquivocationOffence

* GrandpaEquivocationOffence

* slash_fraction and fix

* current_era_start_session_index

* UnresponsivnessOffence

* Finalise OnOffenceHandler traits, and stub impl for staking.

* slash_fraction doesn't really need &self

* Note that offenders count is greater than 0

* Add a test to ensure that I got the math right

* Use FullIdentification in offences.

* Use FullIndentification.

* Hook up the offences module.

* Report unresponsive validators

* Make sure eras have the same length.

* Slashing and rewards.

* Fix compilation.

* Distribute rewards.

* Supply validators_count

* Use identificationTuple in Unresponsivness report

* Fix merge.

* Make sure we don't slash if amount is zero.

* We don't return an error from report_offence anymo

* We actually can use vec!

* Prevent division by zero if the reporters is empty

* offence_forces_new_era/nominators_also_get_slashed

* advance_session

* Fix tests.

* Update srml/staking/src/lib.rs

Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>

* slashing_performed_according_exposure

* Check that reporters receive their slice.

* Small clean-up.

* invulnerables_are_not_slashed

* Minor clean ups.

* Improve docs.

* dont_slash_if_fraction_is_zero

* Remove session dependency from offences.

* Introduce sr-staking-primitives

* Move offence under sr_staking_primitives

* rename session_index

* Resolves todos re using SessionIndex

* Fix staking tests.

* Properly scale denominator.

* Fix UnresponsivnessOffence

* Fix compilation.

* Tests for offences.

* Clean offences tests.

* Fix staking doc test.

* Bump spec version

* Fix aura tests.

* Fix node_executor

* Deposit an event on offence.

* Fix compilation of node-runtime

* Remove aura slashing logic.

* Remove HandleReport

* Update docs for timeslot.

* rename with_on_offence_fractions

* Add should_properly_count_offences

* Replace ValidatorIdByIndex with CurrentElectedSet

ValidatorIdByIndex was querying the current_elected set in each call, doing loading (even though its from cache), deserializing and cloning of element.

Instead of this it is more efficient to use `CurrentElectedSet`. As a small bonus, the invariant became a little bit easier: now we just rely on the fact that `keys` and `current_elected` set are of the same length rather than relying on the fact that `validator_id_by_index` would work similar to `<[T]>::get`.

* Clarify babe equivocation

* Fix offences.

* Rename validators_count to validator_set_count

* Fix squaring.

* Update core/sr-staking-primitives/src/offence.rs

Co-Authored-By: Gavin Wood <gavin@parity.io>

* Docs for CurrentElectedSet.

* Don't punish only invulnerables

* Use `get/insert` instead of `mutate`.

* Fix compilation

* Update core/sr-staking-primitives/src/offence.rs

Co-Authored-By: Gavin Wood <gavin@parity.io>

* Update srml/offences/src/lib.rs

Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>

* Update srml/im-online/src/lib.rs

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Update srml/im-online/src/lib.rs

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Update srml/im-online/src/lib.rs

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Update srml/babe/src/lib.rs

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Update core/sr-staking-primitives/src/offence.rs

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Update core/sr-staking-primitives/src/offence.rs

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Update core/sr-staking-primitives/src/offence.rs

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Update core/sr-staking-primitives/src/offence.rs

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Update core/sr-staking-primitives/src/offence.rs

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Add aura todo.

* Allow multiple reports for single offence report.

* Fix slash_fraction calculation.

* Fix typos.

* Fix compilation and tests.

* Fix staking tests.

* Update srml/im-online/src/lib.rs

Co-Authored-By: Logan Saether <x@logansaether.com>

* Fix doc on time_slot

* Allow slashing only on current era (#3411)

* only slash in current era

* prune journal for last era

* comment own_slash

* emit an event when old slashing events are discarded

* Pave the way for pruning

* Address issues.

* Try to refactor collect_offence_reports

* Other fixes.

* More fixes.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gavofyork gavofyork gavofyork approved these changes

@pepyakin pepyakin pepyakin approved these changes

@tomusdrw tomusdrw tomusdrw approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rphmeier @gavofyork @tomusdrw @pepyakin @devops-parity