From 799dad97cca66065118f322fb89edda9c5b9cc02 Mon Sep 17 00:00:00 2001 From: jianguo Date: Wed, 25 May 2022 12:21:28 +0800 Subject: [PATCH 1/3] merge cosmos sdk pr 6811 and 10339,fix for removeZeroCoins mutates original slice;merge cosmos sdk pr 8811, use typed types/kv.List instead of container/list.List --- libs/cosmos-sdk/store/cachekv/memiterator.go | 13 +++-- libs/cosmos-sdk/store/cachekv/store.go | 15 +++--- libs/cosmos-sdk/types/coin.go | 15 +++--- libs/cosmos-sdk/types/coin_test.go | 52 ++++++++++++++++++++ libs/cosmos-sdk/types/dec_coin.go | 15 +++--- libs/cosmos-sdk/types/dec_coin_test.go | 51 +++++++++++++++++++ 6 files changed, 129 insertions(+), 32 deletions(-) diff --git a/libs/cosmos-sdk/store/cachekv/memiterator.go b/libs/cosmos-sdk/store/cachekv/memiterator.go index 9c2d1a266c..0c40f870a5 100644 --- a/libs/cosmos-sdk/store/cachekv/memiterator.go +++ b/libs/cosmos-sdk/store/cachekv/memiterator.go @@ -1,10 +1,8 @@ package cachekv import ( - "container/list" "errors" - - tmkv "github.com/okex/exchain/libs/tendermint/libs/kv" + kv "github.com/okex/exchain/libs/cosmos-sdk/types/kv" dbm "github.com/okex/exchain/libs/tm-db" ) @@ -13,15 +11,16 @@ import ( // Implements Iterator. type memIterator struct { start, end []byte - items []*tmkv.Pair + items []*kv.Pair ascending bool } -func newMemIterator(start, end []byte, items *list.List, ascending bool) *memIterator { - itemsInDomain := make([]*tmkv.Pair, 0) +func newMemIterator(start, end []byte, items *kv.List, ascending bool) *memIterator { + itemsInDomain := make([]*kv.Pair, 0, items.Len()) + var entered bool for e := items.Front(); e != nil; e = e.Next() { - item := e.Value.(*tmkv.Pair) + item := e.Value if !dbm.IsKeyInDomain(item.Key, start, end) { if entered { break diff --git a/libs/cosmos-sdk/store/cachekv/store.go b/libs/cosmos-sdk/store/cachekv/store.go index c13101b4e0..5ef69c44d6 100644 --- a/libs/cosmos-sdk/store/cachekv/store.go +++ b/libs/cosmos-sdk/store/cachekv/store.go @@ -2,7 +2,7 @@ package cachekv import ( "bytes" - "container/list" + //"github.com/okex/exchain/libs/cosmos-sdk/types/kv" "io" "reflect" "sort" @@ -13,11 +13,12 @@ import ( "github.com/tendermint/go-amino" - tmkv "github.com/okex/exchain/libs/tendermint/libs/kv" + //tmkv "github.com/okex/exchain/libs/tendermint/libs/kv" dbm "github.com/okex/exchain/libs/tm-db" "github.com/okex/exchain/libs/cosmos-sdk/store/tracekv" "github.com/okex/exchain/libs/cosmos-sdk/store/types" + kv "github.com/okex/exchain/libs/cosmos-sdk/types/kv" ) // If value is nil but deleted is false, it means the parent doesn't have the @@ -35,7 +36,7 @@ type Store struct { dirty map[string]cValue readList map[string][]byte unsortedCache map[string]struct{} - sortedCache *list.List // always ascending sorted + sortedCache *kv.List // always ascending sorted parent types.KVStore preChangesHandler PreChangesHandler @@ -48,7 +49,7 @@ func NewStore(parent types.KVStore) *Store { dirty: make(map[string]cValue), readList: make(map[string][]byte), unsortedCache: make(map[string]struct{}), - sortedCache: list.New(), + sortedCache: kv.NewList(), parent: parent, } } @@ -309,13 +310,13 @@ func byteSliceToStr(b []byte) string { // Constructs a slice of dirty items, to use w/ memIterator. func (store *Store) dirtyItems(start, end []byte) { - unsorted := make([]*tmkv.Pair, 0) + unsorted := make([]*kv.Pair, 0) n := len(store.unsortedCache) for key := range store.unsortedCache { if dbm.IsKeyInDomain(strToByte(key), start, end) { cacheValue := store.dirty[key] - unsorted = append(unsorted, &tmkv.Pair{Key: []byte(key), Value: cacheValue.value}) + unsorted = append(unsorted, &kv.Pair{Key: []byte(key), Value: cacheValue.value}) } } @@ -335,7 +336,7 @@ func (store *Store) dirtyItems(start, end []byte) { for e := store.sortedCache.Front(); e != nil && len(unsorted) != 0; { uitem := unsorted[0] - sitem := e.Value.(*tmkv.Pair) + sitem := e.Value comp := bytes.Compare(uitem.Key, sitem.Key) switch comp { case -1: diff --git a/libs/cosmos-sdk/types/coin.go b/libs/cosmos-sdk/types/coin.go index 225173feae..47dd7f39ad 100644 --- a/libs/cosmos-sdk/types/coin.go +++ b/libs/cosmos-sdk/types/coin.go @@ -558,18 +558,15 @@ func (coins Coins) IsAnyGTE(coinsB Coins) bool { // removeZeroCoins removes all zero coins from the given coin set in-place. func removeZeroCoins(coins Coins) Coins { - i, l := 0, len(coins) - for i < l { - if coins[i].IsZero() { - // remove coin - coins = append(coins[:i], coins[i+1:]...) - l-- - } else { - i++ + result := make([]Coin, 0, len(coins)) + + for _, coin := range coins { + if !coin.IsZero() { + result = append(result, coin) } } - return coins[:i] + return result } //----------------------------------------------------------------------------- diff --git a/libs/cosmos-sdk/types/coin_test.go b/libs/cosmos-sdk/types/coin_test.go index b14afa343e..9b89851f62 100644 --- a/libs/cosmos-sdk/types/coin_test.go +++ b/libs/cosmos-sdk/types/coin_test.go @@ -212,6 +212,58 @@ func TestCoinIsZero(t *testing.T) { require.False(t, res) } +func TestFilteredZeroCoins(t *testing.T) { + cases := []struct { + name string + input Coins + original string + expected string + }{ + { + name: "all greater than zero", + input: Coins{ + NewInt64Coin("testa", 1), + NewInt64Coin("testb", 2), + NewInt64Coin("testc", 3), + NewInt64Coin("testd", 4), + NewInt64Coin("teste", 5), + }, + original: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + expected: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + }, + { + name: "zero coin in middle", + input: Coins{ + NewInt64Coin("testa", 1), + NewInt64Coin("testb", 2), + NewInt64Coin("testc", 0), + NewInt64Coin("testd", 4), + NewInt64Coin("teste", 5), + }, + original: "1.000000000000000000testa,2.000000000000000000testb,0.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + expected: "1.000000000000000000testa,2.000000000000000000testb,4.000000000000000000testd,5.000000000000000000teste", + }, + { + name: "zero coin end (unordered)", + input: Coins{ + NewInt64Coin("teste", 5), + NewInt64Coin("testc", 3), + NewInt64Coin("testa", 1), + NewInt64Coin("testd", 4), + NewInt64Coin("testb", 0), + }, + original: "5.000000000000000000teste,3.000000000000000000testc,1.000000000000000000testa,4.000000000000000000testd,0.000000000000000000testb", + expected: "1.000000000000000000testa,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + }, + } + + for _, tt := range cases { + undertest := NewCoins(tt.input...) + require.Equal(t, tt.expected, undertest.String(), "NewCoins must return expected results") + require.Equal(t, tt.original, tt.input.String(), "input must be unmodified and match original") + } +} + // ---------------------------------------------------------------------------- // Coins tests diff --git a/libs/cosmos-sdk/types/dec_coin.go b/libs/cosmos-sdk/types/dec_coin.go index 500cf91857..f6dd9fa0e1 100644 --- a/libs/cosmos-sdk/types/dec_coin.go +++ b/libs/cosmos-sdk/types/dec_coin.go @@ -659,18 +659,15 @@ func (coins DecCoins) IsAllPositive() bool { } func removeZeroDecCoins(coins DecCoins) DecCoins { - i, l := 0, len(coins) - for i < l { - if coins[i].IsZero() { - // remove coin - coins = append(coins[:i], coins[i+1:]...) - l-- - } else { - i++ + result := make([]DecCoin, 0, len(coins)) + + for _, coin := range coins { + if !coin.IsZero() { + result = append(result, coin) } } - return coins[:i] + return result } //----------------------------------------------------------------------------- diff --git a/libs/cosmos-sdk/types/dec_coin_test.go b/libs/cosmos-sdk/types/dec_coin_test.go index 39e89bc030..a36d3a9cea 100644 --- a/libs/cosmos-sdk/types/dec_coin_test.go +++ b/libs/cosmos-sdk/types/dec_coin_test.go @@ -101,6 +101,57 @@ func TestAddDecCoins(t *testing.T) { } } +func TestFilteredZeroDecCoins(t *testing.T) { + cases := []struct { + name string + input DecCoins + original string + expected string + }{ + { + name: "all greater than zero", + input: DecCoins{ + {"testa", NewDec(1)}, + {"testb", NewDec(2)}, + {"testc", NewDec(3)}, + {"testd", NewDec(4)}, + {"teste", NewDec(5)}, + }, + original: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + expected: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + }, + { + name: "zero coin in middle", + input: DecCoins{ + {"testa", NewDec(1)}, + {"testb", NewDec(2)}, + {"testc", NewDec(0)}, + {"testd", NewDec(4)}, + {"teste", NewDec(5)}, + }, + original: "1.000000000000000000testa,2.000000000000000000testb,0.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + expected: "1.000000000000000000testa,2.000000000000000000testb,4.000000000000000000testd,5.000000000000000000teste", + }, + { + name: "zero coin end (unordered)", + input: DecCoins{ + {"teste", NewDec(5)}, + {"testc", NewDec(3)}, + {"testa", NewDec(1)}, + {"testd", NewDec(4)}, + {"testb", NewDec(0)}, + }, + original: "5.000000000000000000teste,3.000000000000000000testc,1.000000000000000000testa,4.000000000000000000testd,0.000000000000000000testb", + expected: "1.000000000000000000testa,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + }, + } + + for _, tt := range cases { + undertest := NewDecCoins(tt.input...) + require.Equal(t, tt.expected, undertest.String(), "NewDecCoins must return expected results") + require.Equal(t, tt.original, tt.input.String(), "input must be unmodified and match original") + } +} func TestIsValid(t *testing.T) { tests := []struct { coin DecCoin From 667691c41c54c8bad46d7e7d53a643f3872469f1 Mon Sep 17 00:00:00 2001 From: jianguo Date: Wed, 25 May 2022 13:02:42 +0800 Subject: [PATCH 2/3] merge cosmos sdk pr 6811 and 10339,fix for removeZeroCoins mutates original slice; --- libs/cosmos-sdk/types/coin.go | 13 +++++++++++-- libs/cosmos-sdk/types/dec_coin.go | 13 +++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/libs/cosmos-sdk/types/coin.go b/libs/cosmos-sdk/types/coin.go index 47dd7f39ad..947bf17b50 100644 --- a/libs/cosmos-sdk/types/coin.go +++ b/libs/cosmos-sdk/types/coin.go @@ -558,14 +558,23 @@ func (coins Coins) IsAnyGTE(coinsB Coins) bool { // removeZeroCoins removes all zero coins from the given coin set in-place. func removeZeroCoins(coins Coins) Coins { - result := make([]Coin, 0, len(coins)) + for i := 0; i < len(coins); i++ { + if coins[i].IsZero() { + break + } else if i == len(coins)-1 { + return coins + } + } + var result []Coin + if len(coins) > 0 { + result = make([]Coin, 0, len(coins)-1) + } for _, coin := range coins { if !coin.IsZero() { result = append(result, coin) } } - return result } diff --git a/libs/cosmos-sdk/types/dec_coin.go b/libs/cosmos-sdk/types/dec_coin.go index f6dd9fa0e1..149a230467 100644 --- a/libs/cosmos-sdk/types/dec_coin.go +++ b/libs/cosmos-sdk/types/dec_coin.go @@ -659,14 +659,23 @@ func (coins DecCoins) IsAllPositive() bool { } func removeZeroDecCoins(coins DecCoins) DecCoins { - result := make([]DecCoin, 0, len(coins)) + for i := 0; i < len(coins); i++ { + if coins[i].IsZero() { + break + } else if i == len(coins)-1 { + return coins + } + } + var result []DecCoin + if len(coins) > 0 { + result = make([]DecCoin, 0, len(coins)-1) + } for _, coin := range coins { if !coin.IsZero() { result = append(result, coin) } } - return result } From 42bd84e5080ecbc6c2e5cbfcb3fa1ade4bfd4a1d Mon Sep 17 00:00:00 2001 From: jianguo Date: Wed, 25 May 2022 13:16:22 +0800 Subject: [PATCH 3/3] delete invalid comments --- libs/cosmos-sdk/store/cachekv/store.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/libs/cosmos-sdk/store/cachekv/store.go b/libs/cosmos-sdk/store/cachekv/store.go index 5ef69c44d6..397f83d1ea 100644 --- a/libs/cosmos-sdk/store/cachekv/store.go +++ b/libs/cosmos-sdk/store/cachekv/store.go @@ -2,7 +2,6 @@ package cachekv import ( "bytes" - //"github.com/okex/exchain/libs/cosmos-sdk/types/kv" "io" "reflect" "sort" @@ -10,11 +9,8 @@ import ( "unsafe" "github.com/okex/exchain/libs/iavl" - - "github.com/tendermint/go-amino" - - //tmkv "github.com/okex/exchain/libs/tendermint/libs/kv" dbm "github.com/okex/exchain/libs/tm-db" + "github.com/tendermint/go-amino" "github.com/okex/exchain/libs/cosmos-sdk/store/tracekv" "github.com/okex/exchain/libs/cosmos-sdk/store/types"