add sorted check for the coins sub/add fun parameter (#9240)

* add sorted check for the coins sub/add fun parameter

* adding internal tests

* fix tests

* docs update

* add self sorted check

* add unit test for self sorted check

* adding a comment

* review updates

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This commit is contained in:
Robert Zaremba 2021-05-05 17:55:28 +02:00 committed by GitHub
parent 42a4e4b432
commit 68d461052b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 82 additions and 21 deletions

View File

@ -243,6 +243,15 @@ func (coins Coins) Validate() error {
}
}
func (coins Coins) isSorted() bool {
for i := 1; i < len(coins); i++ {
if coins[i-1].Denom > coins[i].Denom {
return false
}
}
return true
}
// IsValid calls Validate and returns true when the Coins are sorted, have positive amount, with a
// valid and unique denomination (i.e no duplicates).
func (coins Coins) IsValid() bool {
@ -260,6 +269,7 @@ func (coins Coins) IsValid() bool {
//
// CONTRACT: Add will never return Coins where one Coin has a non-positive
// amount. In otherwords, IsValid will always return true.
// The function panics if `coins` or `coinsB` are not sorted (ascending).
func (coins Coins) Add(coinsB ...Coin) Coins {
return coins.safeAdd(coinsB)
}
@ -269,7 +279,17 @@ func (coins Coins) Add(coinsB ...Coin) Coins {
// other set is returned. Otherwise, the coins are compared in order of their
// denomination and addition only occurs when the denominations match, otherwise
// the coin is simply added to the sum assuming it's not zero.
// The function panics if `coins` or `coinsB` are not sorted (ascending).
func (coins Coins) safeAdd(coinsB Coins) Coins {
// probably the best way will be to make Coins and interface and hide the structure
// definition (type alias)
if !coins.isSorted() {
panic("Coins (self) must be sorted")
}
if !coinsB.isSorted() {
panic("Wrong argument: coins must be sorted")
}
sum := ([]Coin)(nil)
indexA, indexB := 0, 0
lenA, lenB := len(coins), len(coinsB)
@ -354,6 +374,7 @@ func (coins Coins) Sub(coinsB Coins) Coins {
// SafeSub performs the same arithmetic as Sub but returns a boolean if any
// negative coin amount was returned.
// The function panics if `coins` or `coinsB` are not sorted (ascending).
func (coins Coins) SafeSub(coinsB Coins) (Coins, bool) {
diff := coins.safeAdd(coinsB.negative())
return diff, diff.IsAnyNegative()

View File

@ -0,0 +1,37 @@
package types
import (
"testing"
"github.com/stretchr/testify/suite"
)
func TestCoinTestSuite(t *testing.T) {
suite.Run(t, new(coinInternalSuite))
}
type coinInternalSuite struct {
suite.Suite
}
func (s *coinInternalSuite) TestIsSorted() {
v := NewInt(1)
cases := []struct {
coins Coins
expected bool
}{
{Coins{}, true},
{Coins{{"1", v}}, true},
{Coins{{"1", v}, {"1", v}}, true},
{Coins{{"1", v}, {"2", v}}, true},
{Coins{{"1", v}, {"2", v}, {"2", v}}, true},
{Coins{{"1", v}, {"0", v}}, false},
{Coins{{"1", v}, {"0", v}, {"2", v}}, false},
{Coins{{"1", v}, {"1", v}, {"0", v}}, false},
}
assert := s.Assert()
for i, tc := range cases {
assert.Equal(tc.expected, tc.coins.isSorted(), "testcase %d failed", i)
}
}

View File

@ -18,6 +18,7 @@ var (
type coinTestSuite struct {
suite.Suite
ca0, ca1, ca2, cm0, cm1, cm2 sdk.Coin
}
func TestCoinTestSuite(t *testing.T) {
@ -26,6 +27,11 @@ func TestCoinTestSuite(t *testing.T) {
func (s *coinTestSuite) SetupSuite() {
s.T().Parallel()
zero := sdk.NewInt(0)
one := sdk.OneInt()
two := sdk.NewInt(2)
s.ca0, s.ca1, s.ca2 = sdk.Coin{testDenom1, zero}, sdk.Coin{testDenom1, one}, sdk.Coin{testDenom1, two}
s.cm0, s.cm1, s.cm2 = sdk.Coin{testDenom2, zero}, sdk.Coin{testDenom2, one}, sdk.Coin{testDenom2, two}
}
// ----------------------------------------------------------------------------
@ -409,20 +415,16 @@ func (s *coinTestSuite) TestEqualCoins() {
}
func (s *coinTestSuite) TestAddCoins() {
zero := sdk.NewInt(0)
one := sdk.OneInt()
two := sdk.NewInt(2)
cases := []struct {
inputOne sdk.Coins
inputTwo sdk.Coins
expected sdk.Coins
}{
{sdk.Coins{{testDenom1, one}, {testDenom2, one}}, sdk.Coins{{testDenom1, one}, {testDenom2, one}}, sdk.Coins{{testDenom1, two}, {testDenom2, two}}},
{sdk.Coins{{testDenom1, zero}, {testDenom2, one}}, sdk.Coins{{testDenom1, zero}, {testDenom2, zero}}, sdk.Coins{{testDenom2, one}}},
{sdk.Coins{{testDenom1, two}}, sdk.Coins{{testDenom2, zero}}, sdk.Coins{{testDenom1, two}}},
{sdk.Coins{{testDenom1, one}}, sdk.Coins{{testDenom1, one}, {testDenom2, two}}, sdk.Coins{{testDenom1, two}, {testDenom2, two}}},
{sdk.Coins{{testDenom1, zero}, {testDenom2, zero}}, sdk.Coins{{testDenom1, zero}, {testDenom2, zero}}, sdk.Coins(nil)},
{sdk.Coins{s.ca1, s.cm1}, sdk.Coins{s.ca1, s.cm1}, sdk.Coins{s.ca2, s.cm2}},
{sdk.Coins{s.ca0, s.cm1}, sdk.Coins{s.ca0, s.cm0}, sdk.Coins{s.cm1}},
{sdk.Coins{s.ca2}, sdk.Coins{s.cm0}, sdk.Coins{s.ca2}},
{sdk.Coins{s.ca1}, sdk.Coins{s.ca1, s.cm2}, sdk.Coins{s.ca2, s.cm2}},
{sdk.Coins{s.ca0, s.cm0}, sdk.Coins{s.ca0, s.cm0}, sdk.Coins(nil)},
}
for tcIndex, tc := range cases {
@ -433,31 +435,32 @@ func (s *coinTestSuite) TestAddCoins() {
}
func (s *coinTestSuite) TestSubCoins() {
zero := sdk.NewInt(0)
one := sdk.OneInt()
two := sdk.NewInt(2)
testCases := []struct {
inputOne sdk.Coins
inputTwo sdk.Coins
expected sdk.Coins
shouldPanic bool
}{
{sdk.Coins{{testDenom1, two}}, sdk.Coins{{testDenom1, one}, {testDenom2, two}}, sdk.Coins{{testDenom1, one}, {testDenom2, two}}, true},
{sdk.Coins{{testDenom1, two}}, sdk.Coins{{testDenom2, zero}}, sdk.Coins{{testDenom1, two}}, false},
{sdk.Coins{{testDenom1, one}}, sdk.Coins{{testDenom2, zero}}, sdk.Coins{{testDenom1, one}}, false},
{sdk.Coins{{testDenom1, one}, {testDenom2, one}}, sdk.Coins{{testDenom1, one}}, sdk.Coins{{testDenom2, one}}, false},
{sdk.Coins{{testDenom1, one}, {testDenom2, one}}, sdk.Coins{{testDenom1, two}}, sdk.Coins{}, true},
// denoms are not sorted - should panic
{sdk.Coins{s.ca2}, sdk.Coins{s.cm2, s.ca1}, sdk.Coins{}, true},
{sdk.Coins{s.cm2, s.ca2}, sdk.Coins{s.ca1}, sdk.Coins{}, true},
// test cases for sorted denoms
{sdk.Coins{s.ca2}, sdk.Coins{s.ca1, s.cm2}, sdk.Coins{s.ca1, s.cm2}, true},
{sdk.Coins{s.ca2}, sdk.Coins{s.cm0}, sdk.Coins{s.ca2}, false},
{sdk.Coins{s.ca1}, sdk.Coins{s.cm0}, sdk.Coins{s.ca1}, false},
{sdk.Coins{s.ca1, s.cm1}, sdk.Coins{s.ca1}, sdk.Coins{s.cm1}, false},
{sdk.Coins{s.ca1, s.cm1}, sdk.Coins{s.ca2}, sdk.Coins{}, true},
}
assert := s.Assert()
for i, tc := range testCases {
tc := tc
if tc.shouldPanic {
s.Require().Panics(func() { tc.inputOne.Sub(tc.inputTwo) })
assert.Panics(func() { tc.inputOne.Sub(tc.inputTwo) })
} else {
res := tc.inputOne.Sub(tc.inputTwo)
s.Require().True(res.IsValid())
s.Require().Equal(tc.expected, res, "sum of coins is incorrect, tc #%d", i)
assert.True(res.IsValid())
assert.Equal(tc.expected, res, "sum of coins is incorrect, tc #%d", i)
}
}
}