Skip to content

Commit

Permalink
Fix cosmos#7640: tally calculation precision error
Browse files Browse the repository at this point in the history
Solution:
- change `(a / b) * c` to `a * b / c`
  • Loading branch information
yihuang committed Oct 31, 2020
1 parent bd6c16b commit 13e784d
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 7 deletions.
10 changes: 7 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

* (x/staking) [\#7499](https://github.com/cosmos/cosmos-sdk/pull/7499) `BondStatus` is now a protobuf `enum` instead of an `int32`, and JSON serialized using its protobuf name, so expect names like `BOND_STATUS_UNBONDING` as opposed to `Unbonding`.
* (x/evidence) [\#7538](https://github.com/cosmos/cosmos-sdk/pull/7538) The ABCI's `Result.Data` field of `MsgSubmitEvidence` does not contain the raw evidence's hash, but the encoded `MsgSubmitEvidenceResponse` struct.
* (x/upgrade) [#7697](https://github.com/cosmos/cosmos-sdk/pull/7697) Rename flag name "--time" to "--upgrade-time", "--info" to "--upgrade-info", to keep it consistent with help message.
* (x/upgrade) [#7697](https://github.com/cosmos/cosmos-sdk/pull/7697) Rename flag name "--time" to "--upgrade-time", "--info" to "--upgrade-info", to keep it consistent with help message.

### API Breaking

Expand All @@ -63,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

* (kvstore) [\#7415](https://github.com/cosmos/cosmos-sdk/pull/7415) Allow new stores to be registered during on-chain upgrades.
* (client) [\#7699](https://github.com/cosmos/cosmos-sdk/pull/7699) Fix panic in context when setting invalid nodeURI. `WithNodeURI` does not set the `Client` in the context.
* (x/gov) [#7641](https://github.com/cosmos/cosmos-sdk/pull/7641) Fix tally calculation precision error.


## [v0.40.0-rc0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.40.0-rc0) - 2020-10-13
Expand Down Expand Up @@ -653,6 +654,7 @@ generalized genesis accounts through the `GenesisAccount` interface.
* (sdk) [\#4758](https://github.com/cosmos/cosmos-sdk/issues/4758) update `x/genaccounts` to match module spec
* (simulation) [\#4824](https://github.com/cosmos/cosmos-sdk/issues/4824) `PrintAllInvariants` flag will print all failed invariants
* (simulation) [\#4490](https://github.com/cosmos/cosmos-sdk/issues/4490) add `InitialBlockHeight` flag to resume a simulation from a given block

* Support exporting the simulation stats to a given JSON file
* (simulation) [\#4847](https://github.com/cosmos/cosmos-sdk/issues/4847), [\#4838](https://github.com/cosmos/cosmos-sdk/pull/4838) and [\#4869](https://github.com/cosmos/cosmos-sdk/pull/4869) `SimApp` and simulation refactors:
* Implement `SimulationManager` for executing modules' simulation functionalities in a modularized way
Expand Down Expand Up @@ -966,6 +968,7 @@ that error is that the account doesn't exist.
* (simulation) PrintAllInvariants flag will print all failed invariants
* (simulation) Add `InitialBlockHeight` flag to resume a simulation from a given block
* (simulation) [\#4670](https://github.com/cosmos/cosmos-sdk/issues/4670) Update simulation statistics to JSON format

- Support exporting the simulation stats to a given JSON file
* [\#4775](https://github.com/cosmos/cosmos-sdk/issues/4775) Refactor CI config
* Upgrade IAVL to v0.12.4
Expand Down Expand Up @@ -1571,8 +1574,9 @@ BREAKING CHANGES
FEATURES

* Gaia REST API
* [\#2358](https://github.com/cosmos/cosmos-sdk/issues/2358) Add distribution module REST interface


* [\#2358](https://github.com/cosmos/cosmos-sdk/issues/2358) Add distribution module REST interface

* Gaia CLI (`gaiacli`)
* [\#3429](https://github.com/cosmos/cosmos-sdk/issues/3429) Support querying
for all delegator distribution rewards.
Expand Down
8 changes: 8 additions & 0 deletions types/decimal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,14 @@ func (s *decimalTestSuite) TestDecEncoding() {
}
}

// Showcase that different orders of operations causes different results.
func (s *decimalTestSuite) TestOperationOrders() {
n1 := sdk.NewDec(10)
n2 := sdk.NewDec(1000000010)
s.Require().Equal(n1.Mul(n2).Quo(n2), sdk.NewDec(10))
s.Require().NotEqual(n1.Mul(n2).Quo(n2), n1.Quo(n2).Mul(n2))
}

func BenchmarkMarshalTo(b *testing.B) {
bis := []struct {
in sdk.Dec
Expand Down
7 changes: 3 additions & 4 deletions x/gov/keeper/tally.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ func (keeper Keeper) Tally(ctx sdk.Context, proposal types.Proposal) (passes boo
val.DelegatorDeductions = val.DelegatorDeductions.Add(delegation.GetShares())
currValidators[valAddrStr] = val

delegatorShare := delegation.GetShares().Quo(val.DelegatorShares)
votingPower := delegatorShare.MulInt(val.BondedTokens)
// delegation shares * bonded / total shares
votingPower := delegation.GetShares().MulInt(val.BondedTokens).Quo(val.DelegatorShares)

results[vote.Option] = results[vote.Option].Add(votingPower)
totalVotingPower = totalVotingPower.Add(votingPower)
Expand All @@ -78,8 +78,7 @@ func (keeper Keeper) Tally(ctx sdk.Context, proposal types.Proposal) (passes boo
}

sharesAfterDeductions := val.DelegatorShares.Sub(val.DelegatorDeductions)
fractionAfterDeductions := sharesAfterDeductions.Quo(val.DelegatorShares)
votingPower := fractionAfterDeductions.MulInt(val.BondedTokens)
votingPower := sharesAfterDeductions.MulInt(val.BondedTokens).Quo(val.DelegatorShares)

results[val.Vote] = results[val.Vote].Add(votingPower)
totalVotingPower = totalVotingPower.Add(votingPower)
Expand Down

0 comments on commit 13e784d

Please sign in to comment.