Skip to content
This repository was archived by the owner on Jul 27, 2022. It is now read-only.

Problem: (CRO-503) Slashing rate is not proportional to number of failing validators#506

Closed
devashishdxt wants to merge 1 commit intocrypto-com:masterfrom
devashishdxt:proportional-slashing
Closed

Problem: (CRO-503) Slashing rate is not proportional to number of failing validators#506
devashishdxt wants to merge 1 commit intocrypto-com:masterfrom
devashishdxt:proportional-slashing

Conversation

@devashishdxt
Copy link
Copy Markdown
Contributor

Solution: Made slashing rates = (((3 * number of failing validators) / total vaildators) ^ 2) * slash rate

…ailing validators

Solution: Made slashing rates = (((3 * number of failing validators) / total vaildators) ^ 2) * slash rate
@devashishdxt devashishdxt changed the title Problem: (CRO-503) Slashing rates are not proportional to number of failing validators Problem: (CRO-503) Slashing rate is not proportional to number of failing validators Oct 23, 2019
total_validators: usize,
) -> SlashRatio {
let millis =
(((3.0 * failing_validators as f64) / total_validators as f64).powi(2) * 1000.0) as u64;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

3.0 can be constant if we need to adjust

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

3 comes from the fact that it can tolerate up to 1/3 of faulting nodes


#[inline]
fn mul(self, rhs: SlashRatio) -> Self::Output {
SlashRatio::new(self.0 * rhs.0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can it be assertion?

Copy link
Copy Markdown
Contributor Author

@devashishdxt devashishdxt Oct 23, 2019

Choose a reason for hiding this comment

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

I think having expect is similar to having runtime assertion. And in this case, we cannot have a compile time assertion.

Copy link
Copy Markdown
Contributor

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

ideally fixed point operations.

also, the formula for unresponsiveness and byzantine faults may be different -- e.g. https://research.web3.foundation/en/latest/polkadot/slashing/amounts/#unresponsiveness
the formula is `(3*(number of failing validators - 1) / total validators) * slash rate

The idea there is that it's 0 for one isolated case and goes up to the full rate if 1/3 are unresponsive.

Comment on lines +82 to +83
(((3.0 * failing_validators as f64) / total_validators as f64).powi(2) * 1000.0) as u64;
SlashRatio::new(Milli::from_millis(std::cmp::min(millis, 1000))).unwrap() * self
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this should use / preserve fixed-point operations rather than casting to floating points, otherwise it may not be deterministic (e.g. different rustc/llvm versions may optimizing floating point operations differently).
x.powi(2) can be rewritten as x*x

@tomtau tomtau requested a review from lezzokafka October 23, 2019 06:31
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 23, 2019

Codecov Report

Merging #506 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #506      +/-   ##
==========================================
+ Coverage   67.22%   67.24%   +0.02%     
==========================================
  Files         120      120              
  Lines       14114    14127      +13     
==========================================
+ Hits         9488     9500      +12     
- Misses       4626     4627       +1
Impacted Files Coverage Δ
chain-abci/src/app/slash_accounts.rs 97.5% <100%> (+0.2%) ⬆️
chain-core/src/tx/fee/mod.rs 81.88% <100%> (+0.43%) ⬆️
chain-core/src/init/config.rs 69.71% <100%> (+1.05%) ⬆️
chain-core/src/common/merkle_tree.rs 98.78% <0%> (-0.25%) ⬇️

@tomtau
Copy link
Copy Markdown
Contributor

tomtau commented Oct 24, 2019

I guess this PR is currently blocked by: #507

@devashishdxt
Copy link
Copy Markdown
Contributor Author

I guess this PR is currently blocked by: #507

That is correct. I'm closing this pull request for now and will reopen it once all the changes are done.

@devashishdxt devashishdxt deleted the proportional-slashing branch October 28, 2019 03:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants