From b6478026c4e8c5f03c7e6c6f726a60debb9bbfac Mon Sep 17 00:00:00 2001 From: Marie Gauthier Date: Wed, 18 May 2022 16:37:01 +0200 Subject: [PATCH] chore: store audit (#11987) ## Description Ref: https://github.com/cosmos/cosmos-sdk/issues/11362 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- store/cachekv/memiterator.go | 2 +- store/cachekv/store.go | 6 ++- store/iavl/store.go | 2 +- store/streaming/constructor_test.go | 65 ++++++++++++++++++++++++++-- store/streaming/file/service.go | 4 +- store/streaming/file/service_test.go | 4 +- 6 files changed, 71 insertions(+), 12 deletions(-) diff --git a/store/cachekv/memiterator.go b/store/cachekv/memiterator.go index 04df40ff56..e65e8a580f 100644 --- a/store/cachekv/memiterator.go +++ b/store/cachekv/memiterator.go @@ -8,7 +8,7 @@ import ( "github.com/cosmos/cosmos-sdk/store/types" ) -// Iterates over iterKVCache items. +// memIterator iterates over iterKVCache items. // if key is nil, means it was deleted. // Implements Iterator. type memIterator struct { diff --git a/store/cachekv/store.go b/store/cachekv/store.go index 68fe7213db..28063504b2 100644 --- a/store/cachekv/store.go +++ b/store/cachekv/store.go @@ -15,14 +15,16 @@ import ( "github.com/cosmos/cosmos-sdk/types/kv" ) -// If value is nil but deleted is false, it means the parent doesn't have the -// key. (No need to delete upon Write()) +// cValue represents a cached value. +// If dirty is true, it indicates the cached value is different from the underlying value. type cValue struct { value []byte dirty bool } // Store wraps an in-memory cache around an underlying types.KVStore. +// If a cached value is nil but deleted is defined for the corresponding key, +// it means the parent doesn't have the key. (No need to delete upon Write()) type Store struct { mtx sync.Mutex cache map[string]*cValue diff --git a/store/iavl/store.go b/store/iavl/store.go index 3b961e0ab1..c47e7171ad 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -380,7 +380,7 @@ func getProofFromTree(tree *iavl.MutableTree, key []byte, exists bool) *tmcrypto //---------------------------------------- -// Implements types.Iterator. +// iavlIterator implements types.Iterator. type iavlIterator struct { *iavl.Iterator } diff --git a/store/streaming/constructor_test.go b/store/streaming/constructor_test.go index 5f9d58016f..73d512e88b 100644 --- a/store/streaming/constructor_test.go +++ b/store/streaming/constructor_test.go @@ -1,13 +1,19 @@ -package streaming +package streaming_test import ( "testing" + "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec" codecTypes "github.com/cosmos/cosmos-sdk/codec/types" + serverTypes "github.com/cosmos/cosmos-sdk/server/types" + "github.com/cosmos/cosmos-sdk/simapp" + "github.com/cosmos/cosmos-sdk/store/streaming" "github.com/cosmos/cosmos-sdk/store/streaming/file" "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/tendermint/tendermint/libs/log" + dbm "github.com/tendermint/tm-db" "github.com/stretchr/testify/require" ) @@ -24,12 +30,12 @@ var ( ) func TestStreamingServiceConstructor(t *testing.T) { - _, err := NewServiceConstructor("unexpectedName") + _, err := streaming.NewServiceConstructor("unexpectedName") require.NotNil(t, err) - constructor, err := NewServiceConstructor("file") + constructor, err := streaming.NewServiceConstructor("file") require.Nil(t, err) - var expectedType ServiceConstructor + var expectedType streaming.ServiceConstructor require.IsType(t, expectedType, constructor) serv, err := constructor(mockOptions, mockKeys, testMarshaller) @@ -41,3 +47,54 @@ func TestStreamingServiceConstructor(t *testing.T) { require.True(t, ok) } } + +func TestLoadStreamingServices(t *testing.T) { + db := dbm.NewMemDB() + encCdc := simapp.MakeTestEncodingConfig() + keys := sdk.NewKVStoreKeys("mockKey1", "mockKey2") + bApp := baseapp.NewBaseApp("appName", log.NewNopLogger(), db) + + testCases := map[string]struct { + appOpts serverTypes.AppOptions + activeStreamersLen int + }{ + "empty app options": { + appOpts: simapp.EmptyAppOptions{}, + }, + "all StoreKeys exposed": { + appOpts: streamingAppOptions{keys: []string{"*"}}, + activeStreamersLen: 1, + }, + "some StoreKey exposed": { + appOpts: streamingAppOptions{keys: []string{"mockKey1"}}, + activeStreamersLen: 1, + }, + "not exposing anything": { + appOpts: streamingAppOptions{keys: []string{"mockKey3"}}, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + activeStreamers, _, err := streaming.LoadStreamingServices(bApp, tc.appOpts, encCdc.Codec, keys) + require.NoError(t, err) + require.Equal(t, tc.activeStreamersLen, len(activeStreamers)) + }) + } + +} + +type streamingAppOptions struct { + keys []string +} + +func (ao streamingAppOptions) Get(o string) interface{} { + switch o { + case "store.streamers": + return []string{"file"} + case "streamers.file.keys": + return ao.keys + default: + return nil + } +} diff --git a/store/streaming/file/service.go b/store/streaming/file/service.go index 02feb403e9..16c1b5c82b 100644 --- a/store/streaming/file/service.go +++ b/store/streaming/file/service.go @@ -39,7 +39,7 @@ type IntermediateWriter struct { outChan chan<- []byte } -// NewIntermediateWriter create an instance of an intermediateWriter that sends to the provided channel +// NewIntermediateWriter create an instance of an IntermediateWriter that sends to the provided channel func NewIntermediateWriter(outChan chan<- []byte) *IntermediateWriter { return &IntermediateWriter{ outChan: outChan, @@ -62,7 +62,7 @@ func NewStreamingService(writeDir, filePrefix string, storeKeys []types.StoreKey for _, key := range storeKeys { listeners[key] = append(listeners[key], listener) } - // check that the writeDir exists and is writeable so that we can catch the error here at initialization if it is not + // check that the writeDir exists and is writable so that we can catch the error here at initialization if it is not // we don't open a dstFile until we receive our first ABCI message if err := isDirWriteable(writeDir); err != nil { return nil, err diff --git a/store/streaming/file/service_test.go b/store/streaming/file/service_test.go index 1276b16364..db5b2137f9 100644 --- a/store/streaming/file/service_test.go +++ b/store/streaming/file/service_test.go @@ -372,7 +372,7 @@ func readInFile(name string) ([]byte, error) { return ioutil.ReadFile(path) } -// Returns all of the protobuf messages contained in the byte array as an array of byte arrays +// segmentBytes returns all of the protobuf messages contained in the byte array as an array of byte arrays // The messages have their length prefix removed func segmentBytes(bz []byte) ([][]byte, error) { var err error @@ -388,7 +388,7 @@ func segmentBytes(bz []byte) ([][]byte, error) { return segments, nil } -// Returns the bytes for the leading protobuf object in the byte array (removing the length prefix) and returns the remainder of the byte array +// getHeadSegment returns the bytes for the leading protobuf object in the byte array (removing the length prefix) and returns the remainder of the byte array func getHeadSegment(bz []byte) ([]byte, []byte, error) { size, prefixSize := binary.Uvarint(bz) if prefixSize < 0 {