From 0a59cb3864c31cea984ce4db4e5854bf3a4bf0d0 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Tue, 17 Aug 2021 17:33:42 -0400 Subject: [PATCH 01/27] add TODOs to identify where block imported channel is handled --- dot/digest/digest.go | 1 + dot/rpc/subscription/websocket.go | 4 ++++ lib/grandpa/grandpa.go | 1 + lib/grandpa/message_tracker.go | 1 + 4 files changed, 7 insertions(+) diff --git a/dot/digest/digest.go b/dot/digest/digest.go index ebd5dec899..d463bde93d 100644 --- a/dot/digest/digest.go +++ b/dot/digest/digest.go @@ -74,6 +74,7 @@ type resume struct { // NewHandler returns a new Handler func NewHandler(blockState BlockState, epochState EpochState, grandpaState GrandpaState) (*Handler, error) { + // TODO (ed) imported channel is unregistered and closed in handler.Stop imported := make(chan *types.Block, 16) finalised := make(chan *types.FinalisationInfo, 16) iid, err := blockState.RegisterImportedChannel(imported) diff --git a/dot/rpc/subscription/websocket.go b/dot/rpc/subscription/websocket.go index f53e51f952..d1aad3a4d6 100644 --- a/dot/rpc/subscription/websocket.go +++ b/dot/rpc/subscription/websocket.go @@ -233,6 +233,8 @@ func (c *WSConn) unsubscribeStorageListener(reqID float64, l Listener, _ interfa } func (c *WSConn) initBlockListener(reqID float64, _ interface{}) (Listener, error) { + // TODO (ed) here the imported Channel get unregistered and closed in http.Stop + // there doesn't seem to be an un-subscribe for this bl := &BlockListener{ Channel: make(chan *types.Block, DEFAULT_BUFFER_SIZE), wsconn: c, @@ -305,6 +307,8 @@ func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener return nil, err } + // TODO (ed) the importedChan does not seem to get closed, or deleted + // there doesn't seem to be an un-subscribe for this // listen for built blocks esl := &ExtrinsicSubmitListener{ importedChan: make(chan *types.Block, DEFAULT_BUFFER_SIZE), diff --git a/lib/grandpa/grandpa.go b/lib/grandpa/grandpa.go index 15c40799fc..bc47f84ae8 100644 --- a/lib/grandpa/grandpa.go +++ b/lib/grandpa/grandpa.go @@ -377,6 +377,7 @@ func (s *Service) initiate() error { func (s *Service) waitForFirstBlock() error { ch := make(chan *types.Block) + // TODO (ed) import channel is unregistered by defer, doesn't seem to be closed id, err := s.blockState.RegisterImportedChannel(ch) if err != nil { return err diff --git a/lib/grandpa/message_tracker.go b/lib/grandpa/message_tracker.go index c0006a218f..eb2436c871 100644 --- a/lib/grandpa/message_tracker.go +++ b/lib/grandpa/message_tracker.go @@ -37,6 +37,7 @@ type tracker struct { func newTracker(bs BlockState, out chan<- *networkVoteMessage) (*tracker, error) { in := make(chan *types.Block, 16) + // TODO (ed) import channel is unregistered and closed is tracker.stop() id, err := bs.RegisterImportedChannel(in) if err != nil { return nil, err From 30338f71e8c12bdb3e8238113623c02460543934 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Wed, 18 Aug 2021 15:34:13 -0400 Subject: [PATCH 02/27] added comments for imported channels --- dot/digest/digest.go | 1 + dot/rpc/subscription/websocket.go | 3 +-- lib/grandpa/message_tracker.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dot/digest/digest.go b/dot/digest/digest.go index d463bde93d..d518899f29 100644 --- a/dot/digest/digest.go +++ b/dot/digest/digest.go @@ -75,6 +75,7 @@ type resume struct { // NewHandler returns a new Handler func NewHandler(blockState BlockState, epochState EpochState, grandpaState GrandpaState) (*Handler, error) { // TODO (ed) imported channel is unregistered and closed in handler.Stop + // imported := make(chan *types.Block, 16) finalised := make(chan *types.FinalisationInfo, 16) iid, err := blockState.RegisterImportedChannel(imported) diff --git a/dot/rpc/subscription/websocket.go b/dot/rpc/subscription/websocket.go index 1d9ddecb60..51ab57f8cb 100644 --- a/dot/rpc/subscription/websocket.go +++ b/dot/rpc/subscription/websocket.go @@ -287,8 +287,7 @@ func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener return nil, err } - // TODO (ed) the importedChan does not seem to get closed, or deleted - // there doesn't seem to be an un-subscribe for this + // TODO (ed) the importedChan get deleted by unregister imported channel in defer in Listen (listeners.go line 236) // listen for built blocks esl := &ExtrinsicSubmitListener{ importedChan: make(chan *types.Block, DEFAULT_BUFFER_SIZE), diff --git a/lib/grandpa/message_tracker.go b/lib/grandpa/message_tracker.go index eb2436c871..fad86326ee 100644 --- a/lib/grandpa/message_tracker.go +++ b/lib/grandpa/message_tracker.go @@ -37,7 +37,7 @@ type tracker struct { func newTracker(bs BlockState, out chan<- *networkVoteMessage) (*tracker, error) { in := make(chan *types.Block, 16) - // TODO (ed) import channel is unregistered and closed is tracker.stop() + // TODO (ed) import channel is unregistered and closed in tracker.stop() id, err := bs.RegisterImportedChannel(in) if err != nil { return nil, err From 6ed9d6330583ac7988d87f3ed1f8356150896612 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 19 Aug 2021 15:27:37 -0400 Subject: [PATCH 03/27] create constructor for listeners --- dot/rpc/subscription/listeners.go | 26 ++++++++++++++++++++++++++ dot/rpc/subscription/websocket.go | 18 ++---------------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/dot/rpc/subscription/listeners.go b/dot/rpc/subscription/listeners.go index 01a4ad896d..cefc55ff8e 100644 --- a/dot/rpc/subscription/listeners.go +++ b/dot/rpc/subscription/listeners.go @@ -123,6 +123,18 @@ type BlockListener struct { cancelTimeout time.Duration } +// NewBlockListener constructor for creating BlockListener +func NewBlockListener(conn *WSConn) *BlockListener { + bl := &BlockListener{ + Channel: make(chan *types.Block, DEFAULT_BUFFER_SIZE), + wsconn: conn, + cancel: make(chan struct{}, 1), + cancelTimeout: defaultCancelTimeout, + done: make(chan struct{}, 1), + } + return bl +} + // Listen implementation of Listen interface to listen for importedChan changes func (l *BlockListener) Listen() { go func() { @@ -228,6 +240,20 @@ type ExtrinsicSubmitListener struct { cancelTimeout time.Duration } +// NewExtrinsicSubmitListener constructor to build new ExtrinsicSubmitListener +func NewExtrinsicSubmitListener(conn *WSConn, extBytes []byte) *ExtrinsicSubmitListener { + esl := &ExtrinsicSubmitListener{ + importedChan: make(chan *types.Block, DEFAULT_BUFFER_SIZE), + wsconn: conn, + extrinsic: types.Extrinsic(extBytes), + finalisedChan: make(chan *types.FinalisationInfo), + cancel: make(chan struct{}, 1), + done: make(chan struct{}, 1), + cancelTimeout: defaultCancelTimeout, + } + return esl +} + // Listen implementation of Listen interface to listen for importedChan changes func (l *ExtrinsicSubmitListener) Listen() { // listen for imported blocks with extrinsic diff --git a/dot/rpc/subscription/websocket.go b/dot/rpc/subscription/websocket.go index 51ab57f8cb..286bc017e7 100644 --- a/dot/rpc/subscription/websocket.go +++ b/dot/rpc/subscription/websocket.go @@ -215,13 +215,7 @@ func (c *WSConn) initStorageChangeListener(reqID float64, params interface{}) (L func (c *WSConn) initBlockListener(reqID float64, _ interface{}) (Listener, error) { // TODO (ed) here the imported Channel get unregistered and closed in http.Stop // there doesn't seem to be an un-subscribe for this - bl := &BlockListener{ - Channel: make(chan *types.Block, DEFAULT_BUFFER_SIZE), - wsconn: c, - cancel: make(chan struct{}, 1), - cancelTimeout: defaultCancelTimeout, - done: make(chan struct{}, 1), - } + bl := NewBlockListener(c) if c.BlockAPI == nil { c.safeSendError(reqID, nil, "error BlockAPI not set") @@ -289,15 +283,7 @@ func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener // TODO (ed) the importedChan get deleted by unregister imported channel in defer in Listen (listeners.go line 236) // listen for built blocks - esl := &ExtrinsicSubmitListener{ - importedChan: make(chan *types.Block, DEFAULT_BUFFER_SIZE), - wsconn: c, - extrinsic: types.Extrinsic(extBytes), - finalisedChan: make(chan *types.FinalisationInfo), - cancel: make(chan struct{}, 1), - done: make(chan struct{}, 1), - cancelTimeout: defaultCancelTimeout, - } + esl := NewExtrinsicSubmitListener(c, extBytes) if c.BlockAPI == nil { return nil, fmt.Errorf("error BlockAPI not set") From 2ae9572176383560994a8baa5fd6779c90285fb4 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Mon, 23 Aug 2021 15:37:33 -0400 Subject: [PATCH 04/27] added close channel to defer in listen --- dot/rpc/subscription/listeners.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dot/rpc/subscription/listeners.go b/dot/rpc/subscription/listeners.go index cefc55ff8e..28b2889bae 100644 --- a/dot/rpc/subscription/listeners.go +++ b/dot/rpc/subscription/listeners.go @@ -141,6 +141,7 @@ func (l *BlockListener) Listen() { defer func() { l.wsconn.BlockAPI.UnregisterImportedChannel(l.ChanID) close(l.done) + close(l.Channel) }() for { @@ -192,6 +193,7 @@ func (l *BlockFinalizedListener) Listen() { defer func() { l.wsconn.BlockAPI.UnregisterFinalisedChannel(l.chanID) close(l.done) + close(l.channel) }() for { @@ -262,6 +264,8 @@ func (l *ExtrinsicSubmitListener) Listen() { l.wsconn.BlockAPI.UnregisterImportedChannel(l.importedChanID) l.wsconn.BlockAPI.UnregisterFinalisedChannel(l.finalisedChanID) close(l.done) + close(l.importedChan) + close(l.finalisedChan) }() for { From 2b16fa643f44a8d45987379d286fa2b43a696fb3 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 26 Aug 2021 17:14:50 -0400 Subject: [PATCH 05/27] move imported chan to block_notify --- dot/core/interface.go | 7 +- dot/core/mocks/block_state.go | 68 ++++++-------- dot/digest/digest.go | 20 +++-- dot/digest/interface.go | 7 +- dot/rpc/http.go | 4 +- dot/rpc/modules/api.go | 7 +- dot/rpc/modules/api_mocks.go | 11 ++- .../mocks/{BlockAPI.go => block_api.go} | 84 ++++++++--------- dot/rpc/modules/system_test.go | 2 +- dot/rpc/subscription/listeners.go | 23 +++-- dot/rpc/subscription/listeners_test.go | 12 +-- dot/rpc/subscription/websocket.go | 12 ++- dot/rpc/subscription/websocket_test.go | 4 +- dot/state/block.go | 18 ++-- dot/state/block_notify.go | 90 +++++++++++++------ dot/state/block_notify_test.go | 23 +++-- lib/grandpa/grandpa.go | 9 +- lib/grandpa/message_tracker.go | 12 +-- lib/grandpa/state.go | 7 +- 19 files changed, 251 insertions(+), 169 deletions(-) rename dot/rpc/modules/mocks/{BlockAPI.go => block_api.go} (74%) diff --git a/dot/core/interface.go b/dot/core/interface.go index 83fe0ab21f..1013ebbbc6 100644 --- a/dot/core/interface.go +++ b/dot/core/interface.go @@ -41,8 +41,11 @@ type BlockState interface { GetSlotForBlock(common.Hash) (uint64, error) GetFinalisedHeader(uint64, uint64) (*types.Header, error) GetFinalisedHash(uint64, uint64) (common.Hash, error) - RegisterImportedChannel(ch chan<- *types.Block) (byte, error) - UnregisterImportedChannel(id byte) + // todo ed remove + //RegisterImportedChannel(ch chan<- *types.Block) (byte, error) + //UnregisterImportedChannel(id byte) + GetNotifierChannel() (chan *types.Block, error) + FreeNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) HighestCommonAncestor(a, b common.Hash) (common.Hash, error) diff --git a/dot/core/mocks/block_state.go b/dot/core/mocks/block_state.go index 7686c23dab..1227bcd645 100644 --- a/dot/core/mocks/block_state.go +++ b/dot/core/mocks/block_state.go @@ -143,6 +143,11 @@ func (_m *MockBlockState) BestBlockStateRoot() (common.Hash, error) { return r0, r1 } +// FreeNotifierChannel provides a mock function with given fields: ch +func (_m *MockBlockState) FreeNotifierChannel(ch chan *types.Block) { + _m.Called(ch) +} + // GenesisHash provides a mock function with given fields: func (_m *MockBlockState) GenesisHash() common.Hash { ret := _m.Called() @@ -267,6 +272,29 @@ func (_m *MockBlockState) GetFinalisedHeader(_a0 uint64, _a1 uint64) (*types.Hea return r0, r1 } +// GetNotifierChannel provides a mock function with given fields: +func (_m *MockBlockState) GetNotifierChannel() (chan *types.Block, error) { + ret := _m.Called() + + var r0 chan *types.Block + if rf, ok := ret.Get(0).(func() chan *types.Block); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(chan *types.Block) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetRuntime provides a mock function with given fields: _a0 func (_m *MockBlockState) GetRuntime(_a0 *common.Hash) (runtime.Instance, error) { ret := _m.Called(_a0) @@ -369,41 +397,6 @@ func (_m *MockBlockState) RegisterFinalizedChannel(ch chan<- *types.Finalisation return r0, r1 } -// RegisterImportedChannel provides a mock function with given fields: ch -func (_m *MockBlockState) RegisterImportedChannel(ch chan<- *types.Block) (byte, error) { - ret := _m.Called(ch) - - var r0 byte - if rf, ok := ret.Get(0).(func(chan<- *types.Block) byte); ok { - r0 = rf(ch) - } else { - r0 = ret.Get(0).(byte) - } - - var r1 error - if rf, ok := ret.Get(1).(func(chan<- *types.Block) error); ok { - r1 = rf(ch) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// SetFinalisedHash provides a mock function with given fields: _a0, _a1, _a2 -func (_m *MockBlockState) SetFinalisedHash(_a0 common.Hash, _a1 uint64, _a2 uint64) error { - ret := _m.Called(_a0, _a1, _a2) - - var r0 error - if rf, ok := ret.Get(0).(func(common.Hash, uint64, uint64) error); ok { - r0 = rf(_a0, _a1, _a2) - } else { - r0 = ret.Error(0) - } - - return r0 -} - // StoreRuntime provides a mock function with given fields: _a0, _a1 func (_m *MockBlockState) StoreRuntime(_a0 common.Hash, _a1 runtime.Instance) { _m.Called(_a0, _a1) @@ -436,8 +429,3 @@ func (_m *MockBlockState) SubChain(start common.Hash, end common.Hash) ([]common func (_m *MockBlockState) UnregisterFinalisedChannel(id byte) { _m.Called(id) } - -// UnregisterImportedChannel provides a mock function with given fields: id -func (_m *MockBlockState) UnregisterImportedChannel(id byte) { - _m.Called(id) -} diff --git a/dot/digest/digest.go b/dot/digest/digest.go index d518899f29..16a4f22abe 100644 --- a/dot/digest/digest.go +++ b/dot/digest/digest.go @@ -48,7 +48,8 @@ type Handler struct { // block notification channels imported chan *types.Block - importedID byte + // todo remove + //importedID byte finalised chan *types.FinalisationInfo finalisedID byte @@ -76,12 +77,17 @@ type resume struct { func NewHandler(blockState BlockState, epochState EpochState, grandpaState GrandpaState) (*Handler, error) { // TODO (ed) imported channel is unregistered and closed in handler.Stop // - imported := make(chan *types.Block, 16) - finalised := make(chan *types.FinalisationInfo, 16) - iid, err := blockState.RegisterImportedChannel(imported) + // todo ed remove + //imported := make(chan *types.Block, 16) + imported, err := blockState.GetNotifierChannel() if err != nil { return nil, err } + finalised := make(chan *types.FinalisationInfo, 16) + //iid, err := blockState.RegisterImportedChannel(imported) + //if err != nil { + // return nil, err + //} fid, err := blockState.RegisterFinalizedChannel(finalised) if err != nil { @@ -97,7 +103,7 @@ func NewHandler(blockState BlockState, epochState EpochState, grandpaState Grand epochState: epochState, grandpaState: grandpaState, imported: imported, - importedID: iid, + //importedID: iid, finalised: finalised, finalisedID: fid, }, nil @@ -113,7 +119,9 @@ func (h *Handler) Start() error { // Stop stops the Handler func (h *Handler) Stop() error { h.cancel() - h.blockState.UnregisterImportedChannel(h.importedID) + // todo ed remove + //h.blockState.UnregisterImportedChannel(h.importedID) + h.blockState.FreeNotifierChannel(h.imported) h.blockState.UnregisterFinalisedChannel(h.finalisedID) close(h.imported) close(h.finalised) diff --git a/dot/digest/interface.go b/dot/digest/interface.go index bd42d9b883..01d0b2699d 100644 --- a/dot/digest/interface.go +++ b/dot/digest/interface.go @@ -26,8 +26,11 @@ import ( // BlockState interface for block state methods type BlockState interface { BestBlockHeader() (*types.Header, error) - RegisterImportedChannel(ch chan<- *types.Block) (byte, error) - UnregisterImportedChannel(id byte) + // todo ed remove + //RegisterImportedChannel(ch chan<- *types.Block) (byte, error) + //UnregisterImportedChannel(id byte) + GetNotifierChannel() (chan *types.Block, error) + FreeNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) } diff --git a/dot/rpc/http.go b/dot/rpc/http.go index 250d615890..286aa73f39 100644 --- a/dot/rpc/http.go +++ b/dot/rpc/http.go @@ -191,7 +191,9 @@ func (h *HTTPServer) Stop() error { case *subscription.StorageObserver: h.serverConfig.StorageAPI.UnregisterStorageObserver(v) case *subscription.BlockListener: - h.serverConfig.BlockAPI.UnregisterImportedChannel(v.ChanID) + // todo ed remove, remove close channel? + //h.serverConfig.BlockAPI.UnregisterImportedChannel(v.ChanID) + h.serverConfig.BlockAPI.FreeNotifierChannel(v.Channel) close(v.Channel) } } diff --git a/dot/rpc/modules/api.go b/dot/rpc/modules/api.go index 1184b13251..dbecfa6759 100644 --- a/dot/rpc/modules/api.go +++ b/dot/rpc/modules/api.go @@ -35,8 +35,11 @@ type BlockAPI interface { GetHighestFinalisedHash() (common.Hash, error) HasJustification(hash common.Hash) (bool, error) GetJustification(hash common.Hash) ([]byte, error) - RegisterImportedChannel(ch chan<- *types.Block) (byte, error) - UnregisterImportedChannel(id byte) + // todo ed remove + //RegisterImportedChannel(ch chan<- *types.Block) (byte, error) + //UnregisterImportedChannel(id byte) + GetNotifierChannel() (chan *types.Block, error) + FreeNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) SubChain(start, end common.Hash) ([]common.Hash, error) diff --git a/dot/rpc/modules/api_mocks.go b/dot/rpc/modules/api_mocks.go index fa1d6c732a..bb4daec242 100644 --- a/dot/rpc/modules/api_mocks.go +++ b/dot/rpc/modules/api_mocks.go @@ -2,6 +2,7 @@ package modules import ( modulesmocks "github.com/ChainSafe/gossamer/dot/rpc/modules/mocks" + "github.com/ChainSafe/gossamer/dot/types" "github.com/ChainSafe/gossamer/lib/common" runtimemocks "github.com/ChainSafe/gossamer/lib/runtime/mocks" "github.com/stretchr/testify/mock" @@ -21,15 +22,19 @@ func NewMockStorageAPI() *modulesmocks.MockStorageAPI { } // NewMockBlockAPI creates and return an rpc BlockAPI interface mock -func NewMockBlockAPI() *modulesmocks.BlockAPI { - m := new(modulesmocks.BlockAPI) +func NewMockBlockAPI() *modulesmocks.MockBlockAPI { + m := new(modulesmocks.MockBlockAPI) m.On("GetHeader", mock.AnythingOfType("common.Hash")).Return(nil, nil) m.On("BestBlockHash").Return(common.Hash{}) m.On("GetBlockByHash", mock.AnythingOfType("common.Hash")).Return(nil, nil) m.On("GetBlockHash", mock.AnythingOfType("*big.Int")).Return(nil, nil) m.On("GetFinalisedHash", mock.AnythingOfType("uint64"), mock.AnythingOfType("uint64")).Return(common.Hash{}, nil) m.On("GetHighestFinalisedHash").Return(common.Hash{}, nil) - m.On("RegisterImportedChannel", mock.AnythingOfType("chan<- *types.Block")).Return(byte(0), nil) + // todo ed remave + //m.On("RegisterImportedChannel", mock.AnythingOfType("chan<- *types.Block")).Return(byte(0), nil) + //m.On("UnregisterImportedChannel", mock.AnythingOfType("uint8")) + m.On("GetNotifierChannel").Return(make(chan *types.Block, 5), nil) + m.On("FreeNotifierChannel", mock.AnythingOfType("chan *types.Block")) m.On("UnregisterImportedChannel", mock.AnythingOfType("uint8")) m.On("RegisterFinalizedChannel", mock.AnythingOfType("chan<- *types.FinalisationInfo")).Return(byte(0), nil) m.On("UnregisterFinalizedChannel", mock.AnythingOfType("uint8")) diff --git a/dot/rpc/modules/mocks/BlockAPI.go b/dot/rpc/modules/mocks/block_api.go similarity index 74% rename from dot/rpc/modules/mocks/BlockAPI.go rename to dot/rpc/modules/mocks/block_api.go index dd6e6db19b..922e42e3a9 100644 --- a/dot/rpc/modules/mocks/BlockAPI.go +++ b/dot/rpc/modules/mocks/block_api.go @@ -1,4 +1,4 @@ -// Code generated by mockery v0.0.0-dev. DO NOT EDIT. +// Code generated by mockery v2.8.0. DO NOT EDIT. package mocks @@ -13,13 +13,13 @@ import ( types "github.com/ChainSafe/gossamer/dot/types" ) -// BlockAPI is an autogenerated mock type for the BlockAPI type -type BlockAPI struct { +// MockBlockAPI is an autogenerated mock type for the BlockAPI type +type MockBlockAPI struct { mock.Mock } // BestBlockHash provides a mock function with given fields: -func (_m *BlockAPI) BestBlockHash() common.Hash { +func (_m *MockBlockAPI) BestBlockHash() common.Hash { ret := _m.Called() var r0 common.Hash @@ -34,8 +34,13 @@ func (_m *BlockAPI) BestBlockHash() common.Hash { return r0 } +// FreeNotifierChannel provides a mock function with given fields: ch +func (_m *MockBlockAPI) FreeNotifierChannel(ch chan *types.Block) { + _m.Called(ch) +} + // GetBlockByHash provides a mock function with given fields: hash -func (_m *BlockAPI) GetBlockByHash(hash common.Hash) (*types.Block, error) { +func (_m *MockBlockAPI) GetBlockByHash(hash common.Hash) (*types.Block, error) { ret := _m.Called(hash) var r0 *types.Block @@ -58,7 +63,7 @@ func (_m *BlockAPI) GetBlockByHash(hash common.Hash) (*types.Block, error) { } // GetBlockHash provides a mock function with given fields: blockNumber -func (_m *BlockAPI) GetBlockHash(blockNumber *big.Int) (common.Hash, error) { +func (_m *MockBlockAPI) GetBlockHash(blockNumber *big.Int) (common.Hash, error) { ret := _m.Called(blockNumber) var r0 common.Hash @@ -81,7 +86,7 @@ func (_m *BlockAPI) GetBlockHash(blockNumber *big.Int) (common.Hash, error) { } // GetFinalisedHash provides a mock function with given fields: _a0, _a1 -func (_m *BlockAPI) GetFinalisedHash(_a0 uint64, _a1 uint64) (common.Hash, error) { +func (_m *MockBlockAPI) GetFinalisedHash(_a0 uint64, _a1 uint64) (common.Hash, error) { ret := _m.Called(_a0, _a1) var r0 common.Hash @@ -104,7 +109,7 @@ func (_m *BlockAPI) GetFinalisedHash(_a0 uint64, _a1 uint64) (common.Hash, error } // GetHeader provides a mock function with given fields: hash -func (_m *BlockAPI) GetHeader(hash common.Hash) (*types.Header, error) { +func (_m *MockBlockAPI) GetHeader(hash common.Hash) (*types.Header, error) { ret := _m.Called(hash) var r0 *types.Header @@ -127,7 +132,7 @@ func (_m *BlockAPI) GetHeader(hash common.Hash) (*types.Header, error) { } // GetHighestFinalisedHash provides a mock function with given fields: -func (_m *BlockAPI) GetHighestFinalisedHash() (common.Hash, error) { +func (_m *MockBlockAPI) GetHighestFinalisedHash() (common.Hash, error) { ret := _m.Called() var r0 common.Hash @@ -150,7 +155,7 @@ func (_m *BlockAPI) GetHighestFinalisedHash() (common.Hash, error) { } // GetJustification provides a mock function with given fields: hash -func (_m *BlockAPI) GetJustification(hash common.Hash) ([]byte, error) { +func (_m *MockBlockAPI) GetJustification(hash common.Hash) ([]byte, error) { ret := _m.Called(hash) var r0 []byte @@ -172,20 +177,22 @@ func (_m *BlockAPI) GetJustification(hash common.Hash) ([]byte, error) { return r0, r1 } -// HasJustification provides a mock function with given fields: hash -func (_m *BlockAPI) HasJustification(hash common.Hash) (bool, error) { - ret := _m.Called(hash) +// GetNotifierChannel provides a mock function with given fields: +func (_m *MockBlockAPI) GetNotifierChannel() (chan *types.Block, error) { + ret := _m.Called() - var r0 bool - if rf, ok := ret.Get(0).(func(common.Hash) bool); ok { - r0 = rf(hash) + var r0 chan *types.Block + if rf, ok := ret.Get(0).(func() chan *types.Block); ok { + r0 = rf() } else { - r0 = ret.Get(0).(bool) + if ret.Get(0) != nil { + r0 = ret.Get(0).(chan *types.Block) + } } var r1 error - if rf, ok := ret.Get(1).(func(common.Hash) error); ok { - r1 = rf(hash) + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() } else { r1 = ret.Error(1) } @@ -193,20 +200,20 @@ func (_m *BlockAPI) HasJustification(hash common.Hash) (bool, error) { return r0, r1 } -// RegisterFinalizedChannel provides a mock function with given fields: ch -func (_m *BlockAPI) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) { - ret := _m.Called(ch) +// HasJustification provides a mock function with given fields: hash +func (_m *MockBlockAPI) HasJustification(hash common.Hash) (bool, error) { + ret := _m.Called(hash) - var r0 byte - if rf, ok := ret.Get(0).(func(chan<- *types.FinalisationInfo) byte); ok { - r0 = rf(ch) + var r0 bool + if rf, ok := ret.Get(0).(func(common.Hash) bool); ok { + r0 = rf(hash) } else { - r0 = ret.Get(0).(byte) + r0 = ret.Get(0).(bool) } var r1 error - if rf, ok := ret.Get(1).(func(chan<- *types.FinalisationInfo) error); ok { - r1 = rf(ch) + if rf, ok := ret.Get(1).(func(common.Hash) error); ok { + r1 = rf(hash) } else { r1 = ret.Error(1) } @@ -214,19 +221,19 @@ func (_m *BlockAPI) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) return r0, r1 } -// RegisterImportedChannel provides a mock function with given fields: ch -func (_m *BlockAPI) RegisterImportedChannel(ch chan<- *types.Block) (byte, error) { +// RegisterFinalizedChannel provides a mock function with given fields: ch +func (_m *MockBlockAPI) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) { ret := _m.Called(ch) var r0 byte - if rf, ok := ret.Get(0).(func(chan<- *types.Block) byte); ok { + if rf, ok := ret.Get(0).(func(chan<- *types.FinalisationInfo) byte); ok { r0 = rf(ch) } else { r0 = ret.Get(0).(byte) } var r1 error - if rf, ok := ret.Get(1).(func(chan<- *types.Block) error); ok { + if rf, ok := ret.Get(1).(func(chan<- *types.FinalisationInfo) error); ok { r1 = rf(ch) } else { r1 = ret.Error(1) @@ -236,7 +243,7 @@ func (_m *BlockAPI) RegisterImportedChannel(ch chan<- *types.Block) (byte, error } // RegisterRuntimeUpdatedChannel provides a mock function with given fields: ch -func (_m *BlockAPI) RegisterRuntimeUpdatedChannel(ch chan<- runtime.Version) (uint32, error) { +func (_m *MockBlockAPI) RegisterRuntimeUpdatedChannel(ch chan<- runtime.Version) (uint32, error) { ret := _m.Called(ch) var r0 uint32 @@ -257,7 +264,7 @@ func (_m *BlockAPI) RegisterRuntimeUpdatedChannel(ch chan<- runtime.Version) (ui } // SubChain provides a mock function with given fields: start, end -func (_m *BlockAPI) SubChain(start common.Hash, end common.Hash) ([]common.Hash, error) { +func (_m *MockBlockAPI) SubChain(start common.Hash, end common.Hash) ([]common.Hash, error) { ret := _m.Called(start, end) var r0 []common.Hash @@ -280,17 +287,12 @@ func (_m *BlockAPI) SubChain(start common.Hash, end common.Hash) ([]common.Hash, } // UnregisterFinalisedChannel provides a mock function with given fields: id -func (_m *BlockAPI) UnregisterFinalisedChannel(id byte) { - _m.Called(id) -} - -// UnregisterImportedChannel provides a mock function with given fields: id -func (_m *BlockAPI) UnregisterImportedChannel(id byte) { +func (_m *MockBlockAPI) UnregisterFinalisedChannel(id byte) { _m.Called(id) } // UnregisterRuntimeUpdatedChannel provides a mock function with given fields: id -func (_m *BlockAPI) UnregisterRuntimeUpdatedChannel(id uint32) bool { +func (_m *MockBlockAPI) UnregisterRuntimeUpdatedChannel(id uint32) bool { ret := _m.Called(id) var r0 bool diff --git a/dot/rpc/modules/system_test.go b/dot/rpc/modules/system_test.go index afd04255f3..b70c493bfe 100644 --- a/dot/rpc/modules/system_test.go +++ b/dot/rpc/modules/system_test.go @@ -363,7 +363,7 @@ func TestSyncState(t *testing.T) { Number: big.NewInt(int64(49)), } - blockapiMock := new(mocks.BlockAPI) + blockapiMock := new(mocks.MockBlockAPI) blockapiMock.On("BestBlockHash").Return(fakeCommonHash) blockapiMock.On("GetHeader", fakeCommonHash).Return(fakeHeader, nil).Once() diff --git a/dot/rpc/subscription/listeners.go b/dot/rpc/subscription/listeners.go index 0eeba9df07..44ec837841 100644 --- a/dot/rpc/subscription/listeners.go +++ b/dot/rpc/subscription/listeners.go @@ -117,7 +117,8 @@ func (s *StorageObserver) Stop() error { type BlockListener struct { Channel chan *types.Block wsconn *WSConn - ChanID byte +// todo ed remove + //ChanID byte subID uint32 done chan struct{} cancel chan struct{} @@ -127,7 +128,6 @@ type BlockListener struct { // NewBlockListener constructor for creating BlockListener func NewBlockListener(conn *WSConn) *BlockListener { bl := &BlockListener{ - Channel: make(chan *types.Block, DEFAULT_BUFFER_SIZE), wsconn: conn, cancel: make(chan struct{}, 1), cancelTimeout: defaultCancelTimeout, @@ -140,7 +140,9 @@ func NewBlockListener(conn *WSConn) *BlockListener { func (l *BlockListener) Listen() { go func() { defer func() { - l.wsconn.BlockAPI.UnregisterImportedChannel(l.ChanID) + // todo ed remove, and move close to freeNotifier + //l.wsconn.BlockAPI.UnregisterImportedChannel(l.ChanID) + l.wsconn.BlockAPI.FreeNotifierChannel(l.Channel) close(l.done) close(l.Channel) }() @@ -235,7 +237,8 @@ type AllBlocksListener struct { wsconn *WSConn finalizedChanID byte - importedChanID byte + // todo ed remove + //importedChanID byte subID uint32 done chan struct{} cancel chan struct{} @@ -249,7 +252,6 @@ func newAllBlockListener(conn *WSConn) *AllBlocksListener { cancelTimeout: defaultCancelTimeout, wsconn: conn, finalizedChan: make(chan *types.FinalisationInfo, DEFAULT_BUFFER_SIZE), - importedChan: make(chan *types.Block, DEFAULT_BUFFER_SIZE), } } @@ -257,7 +259,9 @@ func newAllBlockListener(conn *WSConn) *AllBlocksListener { func (l *AllBlocksListener) Listen() { go func() { defer func() { - l.wsconn.BlockAPI.UnregisterImportedChannel(l.importedChanID) + // todo ed remove, mave close + //l.wsconn.BlockAPI.UnregisterImportedChannel(l.importedChanID) + l.wsconn.BlockAPI.FreeNotifierChannel(l.importedChan) l.wsconn.BlockAPI.UnregisterFinalisedChannel(l.finalizedChanID) close(l.importedChan) @@ -318,7 +322,7 @@ type ExtrinsicSubmitListener struct { subID uint32 extrinsic types.Extrinsic importedChan chan *types.Block - importedChanID byte + //importedChanID byte importedHash common.Hash finalisedChan chan *types.FinalisationInfo finalisedChanID byte @@ -330,7 +334,6 @@ type ExtrinsicSubmitListener struct { // NewExtrinsicSubmitListener constructor to build new ExtrinsicSubmitListener func NewExtrinsicSubmitListener(conn *WSConn, extBytes []byte) *ExtrinsicSubmitListener { esl := &ExtrinsicSubmitListener{ - importedChan: make(chan *types.Block, DEFAULT_BUFFER_SIZE), wsconn: conn, extrinsic: types.Extrinsic(extBytes), finalisedChan: make(chan *types.FinalisationInfo), @@ -346,7 +349,9 @@ func (l *ExtrinsicSubmitListener) Listen() { // listen for imported blocks with extrinsic go func() { defer func() { - l.wsconn.BlockAPI.UnregisterImportedChannel(l.importedChanID) + // todo ed remove, and move close + //l.wsconn.BlockAPI.UnregisterImportedChannel(l.importedChanID) + l.wsconn.BlockAPI.FreeNotifierChannel(l.importedChan) l.wsconn.BlockAPI.UnregisterFinalisedChannel(l.finalisedChanID) close(l.done) close(l.importedChan) diff --git a/dot/rpc/subscription/listeners_test.go b/dot/rpc/subscription/listeners_test.go index 6a4c612ad2..eb00e8de9a 100644 --- a/dot/rpc/subscription/listeners_test.go +++ b/dot/rpc/subscription/listeners_test.go @@ -96,8 +96,8 @@ func TestBlockListener_Listen(t *testing.T) { wsconn, ws, cancel := setupWSConn(t) defer cancel() - BlockAPI := new(mocks.BlockAPI) - BlockAPI.On("UnregisterImportedChannel", mock.AnythingOfType("uint8")) + BlockAPI := new(mocks.MockBlockAPI) + BlockAPI.On("FreeNotifierChannel", mock.AnythingOfType("chan *types.Block")) wsconn.BlockAPI = BlockAPI @@ -117,7 +117,7 @@ func TestBlockListener_Listen(t *testing.T) { defer func() { require.NoError(t, bl.Stop()) time.Sleep(time.Millisecond * 10) - BlockAPI.AssertCalled(t, "UnregisterImportedChannel", mock.AnythingOfType("uint8")) + BlockAPI.AssertCalled(t, "FreeNotifierChannel", mock.AnythingOfType("chan *types.Block")) }() notifyChan <- block @@ -143,7 +143,7 @@ func TestBlockFinalizedListener_Listen(t *testing.T) { wsconn, ws, cancel := setupWSConn(t) defer cancel() - BlockAPI := new(mocks.BlockAPI) + BlockAPI := new(mocks.MockBlockAPI) BlockAPI.On("UnregisterFinalisedChannel", mock.AnythingOfType("uint8")) wsconn.BlockAPI = BlockAPI @@ -194,7 +194,7 @@ func TestExtrinsicSubmitListener_Listen(t *testing.T) { notifyImportedChan := make(chan *types.Block, 100) notifyFinalizedChan := make(chan *types.FinalisationInfo, 100) - BlockAPI := new(mocks.BlockAPI) + BlockAPI := new(mocks.MockBlockAPI) BlockAPI.On("UnregisterImportedChannel", mock.AnythingOfType("uint8")) BlockAPI.On("UnregisterFinalisedChannel", mock.AnythingOfType("uint8")) @@ -269,7 +269,7 @@ func TestGrandpaJustification_Listen(t *testing.T) { mockedJustBytes, err := mockedJust.Encode() require.NoError(t, err) - blockStateMock := new(mocks.BlockAPI) + blockStateMock := new(mocks.MockBlockAPI) blockStateMock.On("GetJustification", mock.AnythingOfType("common.Hash")).Return(mockedJustBytes, nil) blockStateMock.On("UnregisterFinalisedChannel", mock.AnythingOfType("uint8")) wsconn.BlockAPI = blockStateMock diff --git a/dot/rpc/subscription/websocket.go b/dot/rpc/subscription/websocket.go index 7c579db91e..995d202503 100644 --- a/dot/rpc/subscription/websocket.go +++ b/dot/rpc/subscription/websocket.go @@ -223,7 +223,9 @@ func (c *WSConn) initBlockListener(reqID float64, _ interface{}) (Listener, erro } var err error - bl.ChanID, err = c.BlockAPI.RegisterImportedChannel(bl.Channel) + bl.Channel, err = c.BlockAPI.GetNotifierChannel() + + //bl.ChanID, err = c.BlockAPI.RegisterImportedChannel(bl.Channel) if err != nil { return nil, err @@ -283,7 +285,8 @@ func (c *WSConn) initAllBlocksListerner(reqID float64, _ interface{}) (Listener, } var err error - listener.importedChanID, err = c.BlockAPI.RegisterImportedChannel(listener.importedChan) + //listener.importedChanID, err = c.BlockAPI.RegisterImportedChannel(listener.importedChan) + listener.importedChan, err = c.BlockAPI.GetNotifierChannel() if err != nil { c.safeSendError(reqID, nil, "could not register imported channel") return nil, fmt.Errorf("could not register imported channel") @@ -318,7 +321,10 @@ func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener if c.BlockAPI == nil { return nil, fmt.Errorf("error BlockAPI not set") } - esl.importedChanID, err = c.BlockAPI.RegisterImportedChannel(esl.importedChan) + + // todo ed remove + //esl.importedChanID, err = c.BlockAPI.RegisterImportedChannel(esl.importedChan) + esl.importedChan, err = c.BlockAPI.GetNotifierChannel() if err != nil { return nil, err } diff --git a/dot/rpc/subscription/websocket_test.go b/dot/rpc/subscription/websocket_test.go index cac0e9db0a..fb114108c3 100644 --- a/dot/rpc/subscription/websocket_test.go +++ b/dot/rpc/subscription/websocket_test.go @@ -231,7 +231,7 @@ func TestWSConn_HandleComm(t *testing.T) { mockedJustBytes, err := mockedJust.Encode() require.NoError(t, err) - BlockAPI := new(modulesmocks.BlockAPI) + BlockAPI := new(modulesmocks.MockBlockAPI) BlockAPI.On("RegisterFinalizedChannel", mock.AnythingOfType("chan<- *types.FinalisationInfo")). Run(func(args mock.Arguments) { ch := args.Get(0).(chan<- *types.FinalisationInfo) @@ -287,7 +287,7 @@ func TestSubscribeAllHeads(t *testing.T) { require.NoError(t, err) require.Equal(t, []byte(`{"jsonrpc":"2.0","error":{"code":null,"message":"error BlockAPI not set"},"id":1}`+"\n"), msg) - mockBlockAPI := new(mocks.BlockAPI) + mockBlockAPI := new(mocks.MockBlockAPI) mockBlockAPI.On("RegisterImportedChannel", mock.AnythingOfType("chan<- *types.Block")). Return(uint8(0), errors.New("some mocked error")).Once() diff --git a/dot/state/block.go b/dot/state/block.go index 26a5114d5c..150273ae0b 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -50,11 +50,13 @@ type BlockState struct { lastFinalised common.Hash // block notifiers - imported map[byte]chan<- *types.Block + // todo ed, make this a sync.map so that we can remove importedLock + imported map[chan<- *types.Block]struct{} finalised map[byte]chan<- *types.FinalisationInfo importedLock sync.RWMutex finalisedLock sync.RWMutex - importedBytePool *common.BytePool + //importedBytePool *common.BytePool + importedChanPool *sync.Pool finalisedBytePool *common.BytePool runtimeUpdateSubscriptionsLock sync.RWMutex runtimeUpdateSubscriptions map[uint32]chan<- runtime.Version @@ -73,7 +75,7 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e dbPath: db.Path(), baseState: NewBaseState(db), db: chaindb.NewTable(db, blockPrefix), - imported: make(map[byte]chan<- *types.Block), + imported: make(map[chan<- *types.Block]struct{}), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), @@ -90,7 +92,9 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e return nil, fmt.Errorf("failed to get last finalised hash: %w", err) } - bs.importedBytePool = common.NewBytePool256() + // todo ed remove comments + bs.importedChanPool = NewBlockImportChannelPool() + //bs.importedBytePool = common.NewBytePool256() bs.finalisedBytePool = common.NewBytePool256() return bs, nil } @@ -101,7 +105,7 @@ func NewBlockStateFromGenesis(db chaindb.Database, header *types.Header) (*Block bt: blocktree.NewBlockTreeFromRoot(header, db), baseState: NewBaseState(db), db: chaindb.NewTable(db, blockPrefix), - imported: make(map[byte]chan<- *types.Block), + imported: make(map[chan<- *types.Block]struct{}), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), @@ -134,7 +138,9 @@ func NewBlockStateFromGenesis(db chaindb.Database, header *types.Header) (*Block return nil, err } - bs.importedBytePool = common.NewBytePool256() + // todo ed remove + //bs.importedBytePool = common.NewBytePool256() + bs.importedChanPool = NewBlockImportChannelPool() bs.finalisedBytePool = common.NewBytePool256() return bs, nil } diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index fc9828d41b..655702656b 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -18,6 +18,7 @@ package state import ( "errors" + "fmt" "sync" "github.com/ChainSafe/gossamer/dot/types" @@ -26,22 +27,43 @@ import ( "github.com/google/uuid" ) +// DEFAULT_BUFFER_SIZE buffer size for channels +const DEFAULT_BUFFER_SIZE = 100 + // RegisterImportedChannel registers a channel for block notification upon block import. // It returns the channel ID (used for unregistering the channel) -func (bs *BlockState) RegisterImportedChannel(ch chan<- *types.Block) (byte, error) { - bs.importedLock.RLock() - - id, err := bs.importedBytePool.Get() - if err != nil { - return 0, err - } - - bs.importedLock.RUnlock() - - bs.importedLock.Lock() - bs.imported[id] = ch - bs.importedLock.Unlock() - return id, nil +//func (bs *BlockState) RegisterImportedChannel(ch chan<- *types.Block) (byte, error) { +// bs.importedLock.RLock() +// +// id, err := bs.importedBytePool.Get() +// if err != nil { +// return 0, err +// } +// +// bs.importedLock.RUnlock() +// +// bs.importedLock.Lock() +// bs.imported[id] = ch +// bs.importedLock.Unlock() +// return id, nil +//} + +func (bs *BlockState)GetNotifierChannel() (chan *types.Block, error) { + //ch := make(chan *types.Block, 3) + ch := bs.importedChanPool.Get().(chan *types.Block) + //bs.importedLock.RLock() + // + // id, err := bs.importedBytePool.Get() + // if err != nil { + // return nil, err + // } + // + // bs.importedLock.RUnlock() + + bs.importedLock.Lock() + bs.imported[ch] = struct{}{} + bs.importedLock.Unlock() + return ch, nil } // RegisterFinalizedChannel registers a channel for block notification upon block finalisation. @@ -62,19 +84,24 @@ func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo return id, nil } -// UnregisterImportedChannel removes the block import notification channel with the given ID. -// A channel must be unregistered before closing it. -func (bs *BlockState) UnregisterImportedChannel(id byte) { - bs.importedLock.Lock() - defer bs.importedLock.Unlock() - - delete(bs.imported, id) - err := bs.importedBytePool.Put(id) - if err != nil { - logger.Error("failed to unregister imported channel", "error", err) - } +//// UnregisterImportedChannel removes the block import notification channel with the given ID. +//// A channel must be unregistered before closing it. +//func (bs *BlockState) UnregisterImportedChannel(id byte) { +// bs.importedLock.Lock() +// defer bs.importedLock.Unlock() +// +// delete(bs.imported, id) +// err := bs.importedBytePool.Put(id) +// if err != nil { +// logger.Error("failed to unregister imported channel", "error", err) +// } +//} + +func (bs *BlockState) FreeNotifierChannel(ch chan *types.Block) { + bs.importedChanPool.Put(ch) + delete(bs.imported, ch) + fmt.Printf("Free Notifier called\n") } - // UnregisterFinalisedChannel removes the block finalisation notification channel with the given ID. // A channel must be unregistered before closing it. func (bs *BlockState) UnregisterFinalisedChannel(id byte) { @@ -97,7 +124,7 @@ func (bs *BlockState) notifyImported(block *types.Block) { } logger.Trace("notifying imported block chans...", "chans", bs.imported) - for _, ch := range bs.imported { + for ch, _ := range bs.imported { go func(ch chan<- *types.Block) { select { case ch <- block: @@ -196,3 +223,12 @@ func (bs *BlockState) generateID() uint32 { } return uid.ID() } + +func NewBlockImportChannelPool() *sync.Pool { + pool := &sync.Pool{ + New: func() interface{} { + return make(chan *types.Block, DEFAULT_BUFFER_SIZE) + }, + } + return pool +} diff --git a/dot/state/block_notify_test.go b/dot/state/block_notify_test.go index 6135fbaad4..2422155ff7 100644 --- a/dot/state/block_notify_test.go +++ b/dot/state/block_notify_test.go @@ -33,11 +33,16 @@ var testMessageTimeout = time.Second * 3 func TestImportChannel(t *testing.T) { bs := newTestBlockState(t, testGenesisHeader) - ch := make(chan *types.Block, 3) - id, err := bs.RegisterImportedChannel(ch) + ch, err := bs.GetNotifierChannel() require.NoError(t, err) - defer bs.UnregisterImportedChannel(id) + defer bs.FreeNotifierChannel(ch) + //ch := make(chan *types.Block, 3) +// todo change here +// id, err := bs.RegisterImportedChannel(ch) +// require.NoError(t, err) +// +// defer bs.UnregisterImportedChannel(id) AddBlocksToState(t, bs, 3) @@ -79,12 +84,13 @@ func TestImportChannel_Multi(t *testing.T) { num := 5 chs := make([]chan *types.Block, num) - ids := make([]byte, num) + //ids := make([]byte, num) var err error for i := 0; i < num; i++ { chs[i] = make(chan *types.Block) - ids[i], err = bs.RegisterImportedChannel(chs[i]) +// todo ed change + // ids[i], err = bs.RegisterImportedChannel(chs[i]) require.NoError(t, err) } @@ -109,9 +115,10 @@ func TestImportChannel_Multi(t *testing.T) { AddBlocksToState(t, bs, 1) wg.Wait() - for _, id := range ids { - bs.UnregisterImportedChannel(id) - } + // todo ed change + //for _, id := range ids { + // bs.UnregisterImportedChannel(id) + //} } func TestFinalizedChannel_Multi(t *testing.T) { diff --git a/lib/grandpa/grandpa.go b/lib/grandpa/grandpa.go index bc47f84ae8..43ae8c96b9 100644 --- a/lib/grandpa/grandpa.go +++ b/lib/grandpa/grandpa.go @@ -376,14 +376,17 @@ func (s *Service) initiate() error { } func (s *Service) waitForFirstBlock() error { - ch := make(chan *types.Block) + //ch := make(chan *types.Block) // TODO (ed) import channel is unregistered by defer, doesn't seem to be closed - id, err := s.blockState.RegisterImportedChannel(ch) + // todo remove + ch, err := s.blockState.GetNotifierChannel() + //id, err := s.blockState.RegisterImportedChannel(ch) if err != nil { return err } - defer s.blockState.UnregisterImportedChannel(id) + //defer s.blockState.UnregisterImportedChannel(id) + defer s.blockState.FreeNotifierChannel(ch) // loop until block 1 for { diff --git a/lib/grandpa/message_tracker.go b/lib/grandpa/message_tracker.go index fad86326ee..ccc35999bf 100644 --- a/lib/grandpa/message_tracker.go +++ b/lib/grandpa/message_tracker.go @@ -30,15 +30,16 @@ type tracker struct { messages map[common.Hash][]*networkVoteMessage // map of vote block hash -> array of VoteMessages for that hash mapLock sync.Mutex in chan *types.Block // receive imported block from BlockState - chanID byte // BlockState channel ID out chan<- *networkVoteMessage // send a VoteMessage back to grandpa. corresponds to grandpa's in channel stopped chan struct{} } func newTracker(bs BlockState, out chan<- *networkVoteMessage) (*tracker, error) { - in := make(chan *types.Block, 16) + //in := make(chan *types.Block, 16) + in, err := bs.GetNotifierChannel() // TODO (ed) import channel is unregistered and closed in tracker.stop() - id, err := bs.RegisterImportedChannel(in) + // todo ed, remove, and get channel makes channel 100, not 16 + //id, err := bs.RegisterImportedChannel(in) if err != nil { return nil, err } @@ -48,7 +49,6 @@ func newTracker(bs BlockState, out chan<- *networkVoteMessage) (*tracker, error) messages: make(map[common.Hash][]*networkVoteMessage), mapLock: sync.Mutex{}, in: in, - chanID: id, out: out, stopped: make(chan struct{}), }, nil @@ -60,7 +60,9 @@ func (t *tracker) start() { func (t *tracker) stop() { close(t.stopped) - t.blockState.UnregisterImportedChannel(t.chanID) + // todo ed remave and move close + //t.blockState.UnregisterImportedChannel(t.chanID) + t.blockState.FreeNotifierChannel(t.in) close(t.in) } diff --git a/lib/grandpa/state.go b/lib/grandpa/state.go index 14bcbbab44..5f77c715e5 100644 --- a/lib/grandpa/state.go +++ b/lib/grandpa/state.go @@ -41,8 +41,11 @@ type BlockState interface { BestBlockHash() common.Hash Leaves() []common.Hash BlocktreeAsString() string - RegisterImportedChannel(ch chan<- *types.Block) (byte, error) - UnregisterImportedChannel(id byte) + // todo ed remove + //RegisterImportedChannel(ch chan<- *types.Block) (byte, error) + //UnregisterImportedChannel(id byte) + GetNotifierChannel() (chan *types.Block, error) + FreeNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) SetJustification(hash common.Hash, data []byte) error From 2b512b27ae08e25deabfb6105ef38791581c0a46 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Mon, 30 Aug 2021 15:57:58 -0400 Subject: [PATCH 06/27] remove comments, lint --- dot/core/interface.go | 7 +-- dot/core/mocks/block_state.go | 8 ++-- dot/digest/digest.go | 19 ++------- dot/digest/interface.go | 7 +-- dot/rpc/http.go | 5 +-- dot/rpc/modules/api.go | 7 +-- dot/rpc/modules/api_mocks.go | 7 +-- dot/rpc/modules/mocks/block_api.go | 4 +- dot/rpc/subscription/listeners.go | 20 ++------- dot/rpc/subscription/listeners_test.go | 4 +- dot/rpc/subscription/websocket.go | 15 ++----- dot/state/block.go | 7 +-- dot/state/block_notify.go | 59 +++++--------------------- dot/state/block_notify_test.go | 19 ++------- lib/grandpa/grandpa.go | 9 +--- lib/grandpa/message_tracker.go | 17 +------- lib/grandpa/state.go | 7 +-- 17 files changed, 46 insertions(+), 175 deletions(-) diff --git a/dot/core/interface.go b/dot/core/interface.go index 1013ebbbc6..b120248990 100644 --- a/dot/core/interface.go +++ b/dot/core/interface.go @@ -41,11 +41,8 @@ type BlockState interface { GetSlotForBlock(common.Hash) (uint64, error) GetFinalisedHeader(uint64, uint64) (*types.Header, error) GetFinalisedHash(uint64, uint64) (common.Hash, error) - // todo ed remove - //RegisterImportedChannel(ch chan<- *types.Block) (byte, error) - //UnregisterImportedChannel(id byte) - GetNotifierChannel() (chan *types.Block, error) - FreeNotifierChannel(ch chan *types.Block) + GetImportedBlockNotifierChannel() (chan *types.Block, error) + FreeImportedBlockNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) HighestCommonAncestor(a, b common.Hash) (common.Hash, error) diff --git a/dot/core/mocks/block_state.go b/dot/core/mocks/block_state.go index 1227bcd645..0f12b9c70d 100644 --- a/dot/core/mocks/block_state.go +++ b/dot/core/mocks/block_state.go @@ -143,8 +143,8 @@ func (_m *MockBlockState) BestBlockStateRoot() (common.Hash, error) { return r0, r1 } -// FreeNotifierChannel provides a mock function with given fields: ch -func (_m *MockBlockState) FreeNotifierChannel(ch chan *types.Block) { +// FreeImportedBlockNotifierChannel provides a mock function with given fields: ch +func (_m *MockBlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { _m.Called(ch) } @@ -272,8 +272,8 @@ func (_m *MockBlockState) GetFinalisedHeader(_a0 uint64, _a1 uint64) (*types.Hea return r0, r1 } -// GetNotifierChannel provides a mock function with given fields: -func (_m *MockBlockState) GetNotifierChannel() (chan *types.Block, error) { +// GetImportedBlockNotifierChannel provides a mock function with given fields: +func (_m *MockBlockState) GetImportedBlockNotifierChannel() (chan *types.Block, error) { ret := _m.Called() var r0 chan *types.Block diff --git a/dot/digest/digest.go b/dot/digest/digest.go index 16a4f22abe..552a454f59 100644 --- a/dot/digest/digest.go +++ b/dot/digest/digest.go @@ -48,8 +48,6 @@ type Handler struct { // block notification channels imported chan *types.Block - // todo remove - //importedID byte finalised chan *types.FinalisationInfo finalisedID byte @@ -75,19 +73,12 @@ type resume struct { // NewHandler returns a new Handler func NewHandler(blockState BlockState, epochState EpochState, grandpaState GrandpaState) (*Handler, error) { - // TODO (ed) imported channel is unregistered and closed in handler.Stop - // - // todo ed remove - //imported := make(chan *types.Block, 16) - imported, err := blockState.GetNotifierChannel() + imported, err := blockState.GetImportedBlockNotifierChannel() if err != nil { return nil, err } + finalised := make(chan *types.FinalisationInfo, 16) - //iid, err := blockState.RegisterImportedChannel(imported) - //if err != nil { - // return nil, err - //} fid, err := blockState.RegisterFinalizedChannel(finalised) if err != nil { @@ -103,7 +94,6 @@ func NewHandler(blockState BlockState, epochState EpochState, grandpaState Grand epochState: epochState, grandpaState: grandpaState, imported: imported, - //importedID: iid, finalised: finalised, finalisedID: fid, }, nil @@ -119,11 +109,8 @@ func (h *Handler) Start() error { // Stop stops the Handler func (h *Handler) Stop() error { h.cancel() - // todo ed remove - //h.blockState.UnregisterImportedChannel(h.importedID) - h.blockState.FreeNotifierChannel(h.imported) + h.blockState.FreeImportedBlockNotifierChannel(h.imported) h.blockState.UnregisterFinalisedChannel(h.finalisedID) - close(h.imported) close(h.finalised) return nil } diff --git a/dot/digest/interface.go b/dot/digest/interface.go index 01d0b2699d..6bee49acb5 100644 --- a/dot/digest/interface.go +++ b/dot/digest/interface.go @@ -26,11 +26,8 @@ import ( // BlockState interface for block state methods type BlockState interface { BestBlockHeader() (*types.Header, error) - // todo ed remove - //RegisterImportedChannel(ch chan<- *types.Block) (byte, error) - //UnregisterImportedChannel(id byte) - GetNotifierChannel() (chan *types.Block, error) - FreeNotifierChannel(ch chan *types.Block) + GetImportedBlockNotifierChannel() (chan *types.Block, error) + FreeImportedBlockNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) } diff --git a/dot/rpc/http.go b/dot/rpc/http.go index 286aa73f39..d541a09382 100644 --- a/dot/rpc/http.go +++ b/dot/rpc/http.go @@ -191,10 +191,7 @@ func (h *HTTPServer) Stop() error { case *subscription.StorageObserver: h.serverConfig.StorageAPI.UnregisterStorageObserver(v) case *subscription.BlockListener: - // todo ed remove, remove close channel? - //h.serverConfig.BlockAPI.UnregisterImportedChannel(v.ChanID) - h.serverConfig.BlockAPI.FreeNotifierChannel(v.Channel) - close(v.Channel) + h.serverConfig.BlockAPI.FreeImportedBlockNotifierChannel(v.Channel) } } diff --git a/dot/rpc/modules/api.go b/dot/rpc/modules/api.go index dbecfa6759..220ed45776 100644 --- a/dot/rpc/modules/api.go +++ b/dot/rpc/modules/api.go @@ -35,11 +35,8 @@ type BlockAPI interface { GetHighestFinalisedHash() (common.Hash, error) HasJustification(hash common.Hash) (bool, error) GetJustification(hash common.Hash) ([]byte, error) - // todo ed remove - //RegisterImportedChannel(ch chan<- *types.Block) (byte, error) - //UnregisterImportedChannel(id byte) - GetNotifierChannel() (chan *types.Block, error) - FreeNotifierChannel(ch chan *types.Block) + GetImportedBlockNotifierChannel() (chan *types.Block, error) + FreeImportedBlockNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) SubChain(start, end common.Hash) ([]common.Hash, error) diff --git a/dot/rpc/modules/api_mocks.go b/dot/rpc/modules/api_mocks.go index bb4daec242..4af6fbfd97 100644 --- a/dot/rpc/modules/api_mocks.go +++ b/dot/rpc/modules/api_mocks.go @@ -30,11 +30,8 @@ func NewMockBlockAPI() *modulesmocks.MockBlockAPI { m.On("GetBlockHash", mock.AnythingOfType("*big.Int")).Return(nil, nil) m.On("GetFinalisedHash", mock.AnythingOfType("uint64"), mock.AnythingOfType("uint64")).Return(common.Hash{}, nil) m.On("GetHighestFinalisedHash").Return(common.Hash{}, nil) - // todo ed remave - //m.On("RegisterImportedChannel", mock.AnythingOfType("chan<- *types.Block")).Return(byte(0), nil) - //m.On("UnregisterImportedChannel", mock.AnythingOfType("uint8")) - m.On("GetNotifierChannel").Return(make(chan *types.Block, 5), nil) - m.On("FreeNotifierChannel", mock.AnythingOfType("chan *types.Block")) + m.On("GetImportedBlockNotifierChannel").Return(make(chan *types.Block, 5), nil) + m.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) m.On("UnregisterImportedChannel", mock.AnythingOfType("uint8")) m.On("RegisterFinalizedChannel", mock.AnythingOfType("chan<- *types.FinalisationInfo")).Return(byte(0), nil) m.On("UnregisterFinalizedChannel", mock.AnythingOfType("uint8")) diff --git a/dot/rpc/modules/mocks/block_api.go b/dot/rpc/modules/mocks/block_api.go index 922e42e3a9..5ce7651249 100644 --- a/dot/rpc/modules/mocks/block_api.go +++ b/dot/rpc/modules/mocks/block_api.go @@ -35,7 +35,7 @@ func (_m *MockBlockAPI) BestBlockHash() common.Hash { } // FreeNotifierChannel provides a mock function with given fields: ch -func (_m *MockBlockAPI) FreeNotifierChannel(ch chan *types.Block) { +func (_m *MockBlockAPI) FreeImportedBlockNotifierChannel(ch chan *types.Block) { _m.Called(ch) } @@ -178,7 +178,7 @@ func (_m *MockBlockAPI) GetJustification(hash common.Hash) ([]byte, error) { } // GetNotifierChannel provides a mock function with given fields: -func (_m *MockBlockAPI) GetNotifierChannel() (chan *types.Block, error) { +func (_m *MockBlockAPI) GetImportedBlockNotifierChannel() (chan *types.Block, error) { ret := _m.Called() var r0 chan *types.Block diff --git a/dot/rpc/subscription/listeners.go b/dot/rpc/subscription/listeners.go index 44ec837841..f02216719c 100644 --- a/dot/rpc/subscription/listeners.go +++ b/dot/rpc/subscription/listeners.go @@ -117,8 +117,6 @@ func (s *StorageObserver) Stop() error { type BlockListener struct { Channel chan *types.Block wsconn *WSConn -// todo ed remove - //ChanID byte subID uint32 done chan struct{} cancel chan struct{} @@ -140,11 +138,8 @@ func NewBlockListener(conn *WSConn) *BlockListener { func (l *BlockListener) Listen() { go func() { defer func() { - // todo ed remove, and move close to freeNotifier - //l.wsconn.BlockAPI.UnregisterImportedChannel(l.ChanID) - l.wsconn.BlockAPI.FreeNotifierChannel(l.Channel) + l.wsconn.BlockAPI.FreeImportedBlockNotifierChannel(l.Channel) close(l.done) - close(l.Channel) }() for { @@ -237,8 +232,6 @@ type AllBlocksListener struct { wsconn *WSConn finalizedChanID byte - // todo ed remove - //importedChanID byte subID uint32 done chan struct{} cancel chan struct{} @@ -259,12 +252,9 @@ func newAllBlockListener(conn *WSConn) *AllBlocksListener { func (l *AllBlocksListener) Listen() { go func() { defer func() { - // todo ed remove, mave close - //l.wsconn.BlockAPI.UnregisterImportedChannel(l.importedChanID) - l.wsconn.BlockAPI.FreeNotifierChannel(l.importedChan) + l.wsconn.BlockAPI.FreeImportedBlockNotifierChannel(l.importedChan) l.wsconn.BlockAPI.UnregisterFinalisedChannel(l.finalizedChanID) - close(l.importedChan) close(l.finalizedChan) close(l.done) }() @@ -322,7 +312,6 @@ type ExtrinsicSubmitListener struct { subID uint32 extrinsic types.Extrinsic importedChan chan *types.Block - //importedChanID byte importedHash common.Hash finalisedChan chan *types.FinalisationInfo finalisedChanID byte @@ -349,12 +338,9 @@ func (l *ExtrinsicSubmitListener) Listen() { // listen for imported blocks with extrinsic go func() { defer func() { - // todo ed remove, and move close - //l.wsconn.BlockAPI.UnregisterImportedChannel(l.importedChanID) - l.wsconn.BlockAPI.FreeNotifierChannel(l.importedChan) + l.wsconn.BlockAPI.FreeImportedBlockNotifierChannel(l.importedChan) l.wsconn.BlockAPI.UnregisterFinalisedChannel(l.finalisedChanID) close(l.done) - close(l.importedChan) close(l.finalisedChan) }() diff --git a/dot/rpc/subscription/listeners_test.go b/dot/rpc/subscription/listeners_test.go index eb00e8de9a..9ccc8c5418 100644 --- a/dot/rpc/subscription/listeners_test.go +++ b/dot/rpc/subscription/listeners_test.go @@ -97,7 +97,7 @@ func TestBlockListener_Listen(t *testing.T) { defer cancel() BlockAPI := new(mocks.MockBlockAPI) - BlockAPI.On("FreeNotifierChannel", mock.AnythingOfType("chan *types.Block")) + BlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) wsconn.BlockAPI = BlockAPI @@ -117,7 +117,7 @@ func TestBlockListener_Listen(t *testing.T) { defer func() { require.NoError(t, bl.Stop()) time.Sleep(time.Millisecond * 10) - BlockAPI.AssertCalled(t, "FreeNotifierChannel", mock.AnythingOfType("chan *types.Block")) + BlockAPI.AssertCalled(t, "FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) }() notifyChan <- block diff --git a/dot/rpc/subscription/websocket.go b/dot/rpc/subscription/websocket.go index 995d202503..4adc1a21e2 100644 --- a/dot/rpc/subscription/websocket.go +++ b/dot/rpc/subscription/websocket.go @@ -213,8 +213,6 @@ func (c *WSConn) initStorageChangeListener(reqID float64, params interface{}) (L } func (c *WSConn) initBlockListener(reqID float64, _ interface{}) (Listener, error) { - // TODO (ed) here the imported Channel get unregistered and closed in http.Stop - // there doesn't seem to be an un-subscribe for this bl := NewBlockListener(c) if c.BlockAPI == nil { @@ -223,10 +221,7 @@ func (c *WSConn) initBlockListener(reqID float64, _ interface{}) (Listener, erro } var err error - bl.Channel, err = c.BlockAPI.GetNotifierChannel() - - //bl.ChanID, err = c.BlockAPI.RegisterImportedChannel(bl.Channel) - + bl.Channel, err = c.BlockAPI.GetImportedBlockNotifierChannel() if err != nil { return nil, err } @@ -285,8 +280,7 @@ func (c *WSConn) initAllBlocksListerner(reqID float64, _ interface{}) (Listener, } var err error - //listener.importedChanID, err = c.BlockAPI.RegisterImportedChannel(listener.importedChan) - listener.importedChan, err = c.BlockAPI.GetNotifierChannel() + listener.importedChan, err = c.BlockAPI.GetImportedBlockNotifierChannel() if err != nil { c.safeSendError(reqID, nil, "could not register imported channel") return nil, fmt.Errorf("could not register imported channel") @@ -314,7 +308,6 @@ func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener return nil, err } - // TODO (ed) the importedChan get deleted by unregister imported channel in defer in Listen (listeners.go line 236) // listen for built blocks esl := NewExtrinsicSubmitListener(c, extBytes) @@ -322,9 +315,7 @@ func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener return nil, fmt.Errorf("error BlockAPI not set") } - // todo ed remove - //esl.importedChanID, err = c.BlockAPI.RegisterImportedChannel(esl.importedChan) - esl.importedChan, err = c.BlockAPI.GetNotifierChannel() + esl.importedChan, err = c.BlockAPI.GetImportedBlockNotifierChannel() if err != nil { return nil, err } diff --git a/dot/state/block.go b/dot/state/block.go index 150273ae0b..4575ad0bbb 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -55,8 +55,7 @@ type BlockState struct { finalised map[byte]chan<- *types.FinalisationInfo importedLock sync.RWMutex finalisedLock sync.RWMutex - //importedBytePool *common.BytePool - importedChanPool *sync.Pool + importedChanPool *sync.Pool finalisedBytePool *common.BytePool runtimeUpdateSubscriptionsLock sync.RWMutex runtimeUpdateSubscriptions map[uint32]chan<- runtime.Version @@ -92,9 +91,7 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e return nil, fmt.Errorf("failed to get last finalised hash: %w", err) } - // todo ed remove comments bs.importedChanPool = NewBlockImportChannelPool() - //bs.importedBytePool = common.NewBytePool256() bs.finalisedBytePool = common.NewBytePool256() return bs, nil } @@ -138,8 +135,6 @@ func NewBlockStateFromGenesis(db chaindb.Database, header *types.Header) (*Block return nil, err } - // todo ed remove - //bs.importedBytePool = common.NewBytePool256() bs.importedChanPool = NewBlockImportChannelPool() bs.finalisedBytePool = common.NewBytePool256() return bs, nil diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index 655702656b..68625b3103 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -18,7 +18,6 @@ package state import ( "errors" - "fmt" "sync" "github.com/ChainSafe/gossamer/dot/types" @@ -30,39 +29,13 @@ import ( // DEFAULT_BUFFER_SIZE buffer size for channels const DEFAULT_BUFFER_SIZE = 100 -// RegisterImportedChannel registers a channel for block notification upon block import. -// It returns the channel ID (used for unregistering the channel) -//func (bs *BlockState) RegisterImportedChannel(ch chan<- *types.Block) (byte, error) { -// bs.importedLock.RLock() -// -// id, err := bs.importedBytePool.Get() -// if err != nil { -// return 0, err -// } -// -// bs.importedLock.RUnlock() -// -// bs.importedLock.Lock() -// bs.imported[id] = ch -// bs.importedLock.Unlock() -// return id, nil -//} - -func (bs *BlockState)GetNotifierChannel() (chan *types.Block, error) { - //ch := make(chan *types.Block, 3) +func (bs *BlockState) GetImportedBlockNotifierChannel() (chan *types.Block, error) { ch := bs.importedChanPool.Get().(chan *types.Block) - //bs.importedLock.RLock() - // - // id, err := bs.importedBytePool.Get() - // if err != nil { - // return nil, err - // } - // - // bs.importedLock.RUnlock() - - bs.importedLock.Lock() - bs.imported[ch] = struct{}{} - bs.importedLock.Unlock() + + // todo ed make this a sync make + bs.importedLock.Lock() + bs.imported[ch] = struct{}{} + bs.importedLock.Unlock() return ch, nil } @@ -84,24 +57,12 @@ func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo return id, nil } -//// UnregisterImportedChannel removes the block import notification channel with the given ID. -//// A channel must be unregistered before closing it. -//func (bs *BlockState) UnregisterImportedChannel(id byte) { -// bs.importedLock.Lock() -// defer bs.importedLock.Unlock() -// -// delete(bs.imported, id) -// err := bs.importedBytePool.Put(id) -// if err != nil { -// logger.Error("failed to unregister imported channel", "error", err) -// } -//} - -func (bs *BlockState) FreeNotifierChannel(ch chan *types.Block) { +func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { bs.importedChanPool.Put(ch) delete(bs.imported, ch) - fmt.Printf("Free Notifier called\n") + close(ch) } + // UnregisterFinalisedChannel removes the block finalisation notification channel with the given ID. // A channel must be unregistered before closing it. func (bs *BlockState) UnregisterFinalisedChannel(id byte) { @@ -124,7 +85,7 @@ func (bs *BlockState) notifyImported(block *types.Block) { } logger.Trace("notifying imported block chans...", "chans", bs.imported) - for ch, _ := range bs.imported { + for ch := range bs.imported { go func(ch chan<- *types.Block) { select { case ch <- block: diff --git a/dot/state/block_notify_test.go b/dot/state/block_notify_test.go index 2422155ff7..9c9cdd55a2 100644 --- a/dot/state/block_notify_test.go +++ b/dot/state/block_notify_test.go @@ -33,16 +33,10 @@ var testMessageTimeout = time.Second * 3 func TestImportChannel(t *testing.T) { bs := newTestBlockState(t, testGenesisHeader) - ch, err := bs.GetNotifierChannel() + ch, err := bs.GetImportedBlockNotifierChannel() require.NoError(t, err) - defer bs.FreeNotifierChannel(ch) - //ch := make(chan *types.Block, 3) -// todo change here -// id, err := bs.RegisterImportedChannel(ch) -// require.NoError(t, err) -// -// defer bs.UnregisterImportedChannel(id) + defer bs.FreeImportedBlockNotifierChannel(ch) AddBlocksToState(t, bs, 3) @@ -84,13 +78,10 @@ func TestImportChannel_Multi(t *testing.T) { num := 5 chs := make([]chan *types.Block, num) - //ids := make([]byte, num) var err error for i := 0; i < num; i++ { - chs[i] = make(chan *types.Block) -// todo ed change - // ids[i], err = bs.RegisterImportedChannel(chs[i]) + chs[i], err = bs.GetImportedBlockNotifierChannel() require.NoError(t, err) } @@ -115,10 +106,6 @@ func TestImportChannel_Multi(t *testing.T) { AddBlocksToState(t, bs, 1) wg.Wait() - // todo ed change - //for _, id := range ids { - // bs.UnregisterImportedChannel(id) - //} } func TestFinalizedChannel_Multi(t *testing.T) { diff --git a/lib/grandpa/grandpa.go b/lib/grandpa/grandpa.go index e89b20da9c..2455cfaede 100644 --- a/lib/grandpa/grandpa.go +++ b/lib/grandpa/grandpa.go @@ -376,17 +376,12 @@ func (s *Service) initiate() error { } func (s *Service) waitForFirstBlock() error { - //ch := make(chan *types.Block) - // TODO (ed) import channel is unregistered by defer, doesn't seem to be closed - // todo remove - ch, err := s.blockState.GetNotifierChannel() - //id, err := s.blockState.RegisterImportedChannel(ch) + ch, err := s.blockState.GetImportedBlockNotifierChannel() if err != nil { return err } - //defer s.blockState.UnregisterImportedChannel(id) - defer s.blockState.FreeNotifierChannel(ch) + defer s.blockState.FreeImportedBlockNotifierChannel(ch) // loop until block 1 for { diff --git a/lib/grandpa/message_tracker.go b/lib/grandpa/message_tracker.go index cc1ae28bb3..4144acc1fb 100644 --- a/lib/grandpa/message_tracker.go +++ b/lib/grandpa/message_tracker.go @@ -27,26 +27,17 @@ import ( // tracker keeps track of messages that have been received that have failed to validate with ErrBlockDoesNotExist // these messages may be needed again in the case that we are slightly out of sync with the rest of the network type tracker struct { - -//func newTracker(bs BlockState, out chan<- *networkVoteMessage) (*tracker, error) { -// //in := make(chan *types.Block, 16) -// in, err := bs.GetNotifierChannel() - // TODO (ed) import channel is unregistered and closed in tracker.stop() - // todo ed, remove, and get channel makes channel 100, not 16 - //id, err := bs.RegisterImportedChannel(in) blockState BlockState handler *MessageHandler voteMessages map[common.Hash]map[ed25519.PublicKeyBytes]*networkVoteMessage // map of vote block hash -> array of VoteMessages for that hash commitMessages map[common.Hash]*CommitMessage // map of commit block hash to commit message mapLock sync.Mutex in chan *types.Block // receive imported block from BlockState - chanID byte // BlockState channel ID stopped chan struct{} } func newTracker(bs BlockState, handler *MessageHandler) (*tracker, error) { - in := make(chan *types.Block, 16) - id, err := bs.RegisterImportedChannel(in) + in, err := bs.GetImportedBlockNotifierChannel() if err != nil { return nil, err } @@ -58,7 +49,6 @@ func newTracker(bs BlockState, handler *MessageHandler) (*tracker, error) { commitMessages: make(map[common.Hash]*CommitMessage), mapLock: sync.Mutex{}, in: in, - chanID: id, stopped: make(chan struct{}), }, nil } @@ -69,10 +59,7 @@ func (t *tracker) start() { func (t *tracker) stop() { close(t.stopped) - // todo ed remave and move close - //t.blockState.UnregisterImportedChannel(t.chanID) - t.blockState.FreeNotifierChannel(t.in) - close(t.in) + t.blockState.FreeImportedBlockNotifierChannel(t.in) } func (t *tracker) addVote(v *networkVoteMessage) { diff --git a/lib/grandpa/state.go b/lib/grandpa/state.go index 5f77c715e5..688f291c49 100644 --- a/lib/grandpa/state.go +++ b/lib/grandpa/state.go @@ -41,11 +41,8 @@ type BlockState interface { BestBlockHash() common.Hash Leaves() []common.Hash BlocktreeAsString() string - // todo ed remove - //RegisterImportedChannel(ch chan<- *types.Block) (byte, error) - //UnregisterImportedChannel(id byte) - GetNotifierChannel() (chan *types.Block, error) - FreeNotifierChannel(ch chan *types.Block) + GetImportedBlockNotifierChannel() (chan *types.Block, error) + FreeImportedBlockNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) SetJustification(hash common.Hash, data []byte) error From e3bd2201f34e997479b9ea14dc867488d43e5412 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Mon, 30 Aug 2021 16:06:49 -0400 Subject: [PATCH 07/27] handle lint issues --- dot/state/block_notify.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index 68625b3103..fd7133671d 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -29,6 +29,7 @@ import ( // DEFAULT_BUFFER_SIZE buffer size for channels const DEFAULT_BUFFER_SIZE = 100 +// GetImportedBlockNotifierChannel function to retrieve a imported block notifier channel func (bs *BlockState) GetImportedBlockNotifierChannel() (chan *types.Block, error) { ch := bs.importedChanPool.Get().(chan *types.Block) @@ -57,6 +58,7 @@ func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo return id, nil } +// FreeImportedBlockNotifierChannel to free and close imported block notifier channel func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { bs.importedChanPool.Put(ch) delete(bs.imported, ch) @@ -185,6 +187,7 @@ func (bs *BlockState) generateID() uint32 { return uid.ID() } +// NewBlockImportChannelPool to create a sync pool of imported block notifier channels func NewBlockImportChannelPool() *sync.Pool { pool := &sync.Pool{ New: func() interface{} { From fd6d7cc2e1f11e2ac5e7739caf135a9d4bb55a48 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Mon, 30 Aug 2021 16:38:49 -0400 Subject: [PATCH 08/27] replace imported channel map with sync.Map --- dot/state/block.go | 24 ++++++++++++------------ dot/state/block_notify.go | 37 ++++++++++++++++++++++--------------- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 4575ad0bbb..25441f05e7 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -50,10 +50,8 @@ type BlockState struct { lastFinalised common.Hash // block notifiers - // todo ed, make this a sync.map so that we can remove importedLock - imported map[chan<- *types.Block]struct{} + imported *sync.Map finalised map[byte]chan<- *types.FinalisationInfo - importedLock sync.RWMutex finalisedLock sync.RWMutex importedChanPool *sync.Pool finalisedBytePool *common.BytePool @@ -70,11 +68,12 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e } bs := &BlockState{ - bt: bt, - dbPath: db.Path(), - baseState: NewBaseState(db), - db: chaindb.NewTable(db, blockPrefix), - imported: make(map[chan<- *types.Block]struct{}), + bt: bt, + dbPath: db.Path(), + baseState: NewBaseState(db), + db: chaindb.NewTable(db, blockPrefix), + //imported: make(map[chan<- *types.Block]struct{}), + imported: new(sync.Map), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), @@ -99,10 +98,11 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e // NewBlockStateFromGenesis initialises a BlockState from a genesis header, saving it to the database located at basePath func NewBlockStateFromGenesis(db chaindb.Database, header *types.Header) (*BlockState, error) { bs := &BlockState{ - bt: blocktree.NewBlockTreeFromRoot(header, db), - baseState: NewBaseState(db), - db: chaindb.NewTable(db, blockPrefix), - imported: make(map[chan<- *types.Block]struct{}), + bt: blocktree.NewBlockTreeFromRoot(header, db), + baseState: NewBaseState(db), + db: chaindb.NewTable(db, blockPrefix), + //imported: make(map[chan<- *types.Block]struct{}), + imported: new(sync.Map), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index fd7133671d..eefe3281c5 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -32,11 +32,7 @@ const DEFAULT_BUFFER_SIZE = 100 // GetImportedBlockNotifierChannel function to retrieve a imported block notifier channel func (bs *BlockState) GetImportedBlockNotifierChannel() (chan *types.Block, error) { ch := bs.importedChanPool.Get().(chan *types.Block) - - // todo ed make this a sync make - bs.importedLock.Lock() - bs.imported[ch] = struct{}{} - bs.importedLock.Unlock() + bs.imported.Store(ch, struct{}{}) return ch, nil } @@ -61,7 +57,8 @@ func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo // FreeImportedBlockNotifierChannel to free and close imported block notifier channel func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { bs.importedChanPool.Put(ch) - delete(bs.imported, ch) + //delete(bs.imported, ch) + bs.imported.Delete(ch) close(ch) } @@ -79,22 +76,32 @@ func (bs *BlockState) UnregisterFinalisedChannel(id byte) { } func (bs *BlockState) notifyImported(block *types.Block) { - bs.importedLock.RLock() - defer bs.importedLock.RUnlock() + //bs.importedLock.RLock() + //defer bs.importedLock.RUnlock() - if len(bs.imported) == 0 { - return - } + //if len(bs.imported) == 0 { + // return + //} logger.Trace("notifying imported block chans...", "chans", bs.imported) - for ch := range bs.imported { - go func(ch chan<- *types.Block) { + bs.imported.Range(func(k, v interface{}) bool { + go func(ch chan *types.Block) { select { case ch <- block: default: } - }(ch) - } + }(k.(chan *types.Block)) + return true + }) + + //for ch := range bs.imported { + // go func(ch chan<- *types.Block) { + // select { + // case ch <- block: + // default: + // } + // }(ch) + //} } func (bs *BlockState) notifyFinalized(hash common.Hash, round, setID uint64) { From 125b68a401da3d65a3d5ddf4739653ed0d818c05 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Mon, 30 Aug 2021 17:15:45 -0400 Subject: [PATCH 09/27] fix mocks in listeners test --- dot/rpc/subscription/listeners_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dot/rpc/subscription/listeners_test.go b/dot/rpc/subscription/listeners_test.go index 9ccc8c5418..3188670f07 100644 --- a/dot/rpc/subscription/listeners_test.go +++ b/dot/rpc/subscription/listeners_test.go @@ -195,7 +195,7 @@ func TestExtrinsicSubmitListener_Listen(t *testing.T) { notifyFinalizedChan := make(chan *types.FinalisationInfo, 100) BlockAPI := new(mocks.MockBlockAPI) - BlockAPI.On("UnregisterImportedChannel", mock.AnythingOfType("uint8")) + BlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) BlockAPI.On("UnregisterFinalisedChannel", mock.AnythingOfType("uint8")) wsconn.BlockAPI = BlockAPI @@ -225,7 +225,7 @@ func TestExtrinsicSubmitListener_Listen(t *testing.T) { require.NoError(t, esl.Stop()) time.Sleep(time.Millisecond * 10) - BlockAPI.AssertCalled(t, "UnregisterImportedChannel", mock.AnythingOfType("uint8")) + BlockAPI.AssertCalled(t, "FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) BlockAPI.AssertCalled(t, "UnregisterFinalisedChannel", mock.AnythingOfType("uint8")) }() From 6ca1a8b918e8d714efef0e7cbc2eb94a52cf5526 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Tue, 31 Aug 2021 15:17:23 -0400 Subject: [PATCH 10/27] fix mock functions for new imported notification channel --- dot/rpc/subscription/websocket_test.go | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/dot/rpc/subscription/websocket_test.go b/dot/rpc/subscription/websocket_test.go index fb114108c3..97296f805b 100644 --- a/dot/rpc/subscription/websocket_test.go +++ b/dot/rpc/subscription/websocket_test.go @@ -288,8 +288,7 @@ func TestSubscribeAllHeads(t *testing.T) { require.Equal(t, []byte(`{"jsonrpc":"2.0","error":{"code":null,"message":"error BlockAPI not set"},"id":1}`+"\n"), msg) mockBlockAPI := new(mocks.MockBlockAPI) - mockBlockAPI.On("RegisterImportedChannel", mock.AnythingOfType("chan<- *types.Block")). - Return(uint8(0), errors.New("some mocked error")).Once() + mockBlockAPI.On("GetImportedBlockNotifierChannel").Return(nil, errors.New("some mocked error")).Once() wsconn.BlockAPI = mockBlockAPI _, err = wsconn.initAllBlocksListerner(1, nil) @@ -299,8 +298,7 @@ func TestSubscribeAllHeads(t *testing.T) { require.NoError(t, err) require.Equal(t, []byte(`{"jsonrpc":"2.0","error":{"code":null,"message":"could not register imported channel"},"id":1}`+"\n"), msg) - mockBlockAPI.On("RegisterImportedChannel", mock.AnythingOfType("chan<- *types.Block")). - Return(uint8(10), nil).Once() + mockBlockAPI.On("GetImportedBlockNotifierChannel").Return(make(chan *types.Block), nil).Once() mockBlockAPI.On("RegisterFinalizedChannel", mock.AnythingOfType("chan<- *types.FinalisationInfo")). Return(uint8(0), errors.New("failed")).Once() @@ -308,17 +306,11 @@ func TestSubscribeAllHeads(t *testing.T) { require.Error(t, err, "could not register finalised channel") c.ReadMessage() - importedChanID := uint8(10) finalizedChanID := uint8(11) var fCh chan<- *types.FinalisationInfo - var iCh chan<- *types.Block - - mockBlockAPI.On("RegisterImportedChannel", mock.AnythingOfType("chan<- *types.Block")). - Run(func(args mock.Arguments) { - ch := args.Get(0).(chan<- *types.Block) - iCh = ch - }).Return(importedChanID, nil).Once() + iCh := make(chan *types.Block) + mockBlockAPI.On("GetImportedBlockNotifierChannel").Return(iCh, nil).Once() mockBlockAPI.On("RegisterFinalizedChannel", mock.AnythingOfType("chan<- *types.FinalisationInfo")). Run(func(args mock.Arguments) { @@ -361,7 +353,6 @@ func TestSubscribeAllHeads(t *testing.T) { _, msg, err = c.ReadMessage() require.NoError(t, err) require.Equal(t, expected+"\n", string(msg)) - iCh <- &types.Block{ Header: &types.Header{ ParentHash: common.EmptyHash, @@ -376,10 +367,10 @@ func TestSubscribeAllHeads(t *testing.T) { require.NoError(t, err) require.Equal(t, []byte(expected+"\n"), msg) - mockBlockAPI.On("UnregisterImportedChannel", importedChanID) + mockBlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) mockBlockAPI.On("UnregisterFinalisedChannel", finalizedChanID) require.NoError(t, l.Stop()) - mockBlockAPI.AssertCalled(t, "UnregisterImportedChannel", importedChanID) + mockBlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) mockBlockAPI.AssertCalled(t, "UnregisterFinalisedChannel", finalizedChanID) } From d3eaa0621f31793b3186748051cbd1a29bb359f6 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Tue, 31 Aug 2021 15:32:44 -0400 Subject: [PATCH 11/27] fix deep source issues --- dot/digest/digest.go | 2 +- dot/rpc/subscription/listeners.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dot/digest/digest.go b/dot/digest/digest.go index 552a454f59..e2258e0fd5 100644 --- a/dot/digest/digest.go +++ b/dot/digest/digest.go @@ -422,7 +422,7 @@ func newGrandpaChange(raw []*types.GrandpaAuthoritiesRaw, delay uint32, currBloc }, nil } -func (h *Handler) handleBABEOnDisabled(d *types.ConsensusDigest, _ *types.Header) error { +func (*Handler) handleBABEOnDisabled(_ *types.ConsensusDigest, _ *types.Header) error { od := &types.BABEOnDisabled{} logger.Debug("handling BABEOnDisabled", "data", od) return nil diff --git a/dot/rpc/subscription/listeners.go b/dot/rpc/subscription/listeners.go index f02216719c..460f542fe9 100644 --- a/dot/rpc/subscription/listeners.go +++ b/dot/rpc/subscription/listeners.go @@ -105,7 +105,7 @@ func (s *StorageObserver) GetFilter() map[string][]byte { } // Listen to satisfy Listener interface (but is no longer used by StorageObserver) -func (s *StorageObserver) Listen() {} +func (*StorageObserver) Listen() {} // Stop to satisfy Listener interface (but is no longer used by StorageObserver) func (s *StorageObserver) Stop() error { @@ -451,7 +451,7 @@ func (l *RuntimeVersionListener) GetChannelID() uint32 { // Stop to runtimeVersionListener not implemented yet because the listener // does not need to be stoped -func (l *RuntimeVersionListener) Stop() error { return nil } +func (*RuntimeVersionListener) Stop() error { return nil } // GrandpaJustificationListener struct has the finalisedCh and the context to stop the goroutines type GrandpaJustificationListener struct { From 41bb429b7e8ae0cc249635f01076ff9d4c5c43db Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Wed, 1 Sep 2021 15:07:00 -0400 Subject: [PATCH 12/27] add debugging printf --- dot/state/block_notify.go | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index eefe3281c5..f5111ed513 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -18,6 +18,7 @@ package state import ( "errors" + "fmt" "sync" "github.com/ChainSafe/gossamer/dot/types" @@ -57,9 +58,10 @@ func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo // FreeImportedBlockNotifierChannel to free and close imported block notifier channel func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { bs.importedChanPool.Put(ch) - //delete(bs.imported, ch) bs.imported.Delete(ch) close(ch) + + fmt.Printf("Freed channel %v\n", ch) } // UnregisterFinalisedChannel removes the block finalisation notification channel with the given ID. @@ -76,15 +78,9 @@ func (bs *BlockState) UnregisterFinalisedChannel(id byte) { } func (bs *BlockState) notifyImported(block *types.Block) { - //bs.importedLock.RLock() - //defer bs.importedLock.RUnlock() - - //if len(bs.imported) == 0 { - // return - //} - logger.Trace("notifying imported block chans...", "chans", bs.imported) bs.imported.Range(func(k, v interface{}) bool { + fmt.Printf("CHANNEL %v\n", k) go func(ch chan *types.Block) { select { case ch <- block: @@ -94,14 +90,6 @@ func (bs *BlockState) notifyImported(block *types.Block) { return true }) - //for ch := range bs.imported { - // go func(ch chan<- *types.Block) { - // select { - // case ch <- block: - // default: - // } - // }(ch) - //} } func (bs *BlockState) notifyFinalized(hash common.Hash, round, setID uint64) { From 01643053a2fea07fb61cb5798e3c58ecd8b4c629 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 2 Sep 2021 10:40:20 -0400 Subject: [PATCH 13/27] remove sync.Pool, and sync.Map --- dot/state/block.go | 26 +++++++++++-------------- dot/state/block_notify.go | 41 +++++++++++++++++++-------------------- 2 files changed, 31 insertions(+), 36 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 25441f05e7..a972ecf245 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -50,10 +50,10 @@ type BlockState struct { lastFinalised common.Hash // block notifiers - imported *sync.Map + imported map[chan<- *types.Block]struct{} finalised map[byte]chan<- *types.FinalisationInfo finalisedLock sync.RWMutex - importedChanPool *sync.Pool + importedLock sync.RWMutex finalisedBytePool *common.BytePool runtimeUpdateSubscriptionsLock sync.RWMutex runtimeUpdateSubscriptions map[uint32]chan<- runtime.Version @@ -68,12 +68,11 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e } bs := &BlockState{ - bt: bt, - dbPath: db.Path(), - baseState: NewBaseState(db), - db: chaindb.NewTable(db, blockPrefix), - //imported: make(map[chan<- *types.Block]struct{}), - imported: new(sync.Map), + bt: bt, + dbPath: db.Path(), + baseState: NewBaseState(db), + db: chaindb.NewTable(db, blockPrefix), + imported: make(map[chan<- *types.Block]struct{}), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), @@ -90,7 +89,6 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e return nil, fmt.Errorf("failed to get last finalised hash: %w", err) } - bs.importedChanPool = NewBlockImportChannelPool() bs.finalisedBytePool = common.NewBytePool256() return bs, nil } @@ -98,11 +96,10 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e // NewBlockStateFromGenesis initialises a BlockState from a genesis header, saving it to the database located at basePath func NewBlockStateFromGenesis(db chaindb.Database, header *types.Header) (*BlockState, error) { bs := &BlockState{ - bt: blocktree.NewBlockTreeFromRoot(header, db), - baseState: NewBaseState(db), - db: chaindb.NewTable(db, blockPrefix), - //imported: make(map[chan<- *types.Block]struct{}), - imported: new(sync.Map), + bt: blocktree.NewBlockTreeFromRoot(header, db), + baseState: NewBaseState(db), + db: chaindb.NewTable(db, blockPrefix), + imported: make(map[chan<- *types.Block]struct{}), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), @@ -135,7 +132,6 @@ func NewBlockStateFromGenesis(db chaindb.Database, header *types.Header) (*Block return nil, err } - bs.importedChanPool = NewBlockImportChannelPool() bs.finalisedBytePool = common.NewBytePool256() return bs, nil } diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index f5111ed513..dcf1d42383 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -32,8 +32,10 @@ const DEFAULT_BUFFER_SIZE = 100 // GetImportedBlockNotifierChannel function to retrieve a imported block notifier channel func (bs *BlockState) GetImportedBlockNotifierChannel() (chan *types.Block, error) { - ch := bs.importedChanPool.Get().(chan *types.Block) - bs.imported.Store(ch, struct{}{}) + ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) + bs.importedLock.Lock() + bs.imported[ch] = struct{}{} + bs.importedLock.Unlock() return ch, nil } @@ -57,8 +59,10 @@ func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo // FreeImportedBlockNotifierChannel to free and close imported block notifier channel func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { - bs.importedChanPool.Put(ch) - bs.imported.Delete(ch) + bs.importedLock.Lock() + defer bs.importedLock.Unlock() + + delete(bs.imported, ch) close(ch) fmt.Printf("Freed channel %v\n", ch) @@ -78,18 +82,23 @@ func (bs *BlockState) UnregisterFinalisedChannel(id byte) { } func (bs *BlockState) notifyImported(block *types.Block) { + bs.importedLock.RLock() + defer bs.importedLock.RUnlock() + + if len(bs.imported) == 0 { + return + } + logger.Trace("notifying imported block chans...", "chans", bs.imported) - bs.imported.Range(func(k, v interface{}) bool { - fmt.Printf("CHANNEL %v\n", k) - go func(ch chan *types.Block) { + for ch := range bs.imported { + fmt.Printf("CHANNEL %v\n", ch) + go func(ch chan<- *types.Block) { select { case ch <- block: default: } - }(k.(chan *types.Block)) - return true - }) - + }(ch) + } } func (bs *BlockState) notifyFinalized(hash common.Hash, round, setID uint64) { @@ -181,13 +190,3 @@ func (bs *BlockState) generateID() uint32 { } return uid.ID() } - -// NewBlockImportChannelPool to create a sync pool of imported block notifier channels -func NewBlockImportChannelPool() *sync.Pool { - pool := &sync.Pool{ - New: func() interface{} { - return make(chan *types.Block, DEFAULT_BUFFER_SIZE) - }, - } - return pool -} From b7f658f6ff28f3fb6613ed9b26f6eee038e0cb55 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Fri, 3 Sep 2021 14:06:25 -0400 Subject: [PATCH 14/27] handle channel closing --- dot/state/block_notify.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index dcf1d42383..93908eeb47 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -18,7 +18,6 @@ package state import ( "errors" - "fmt" "sync" "github.com/ChainSafe/gossamer/dot/types" @@ -63,9 +62,6 @@ func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { defer bs.importedLock.Unlock() delete(bs.imported, ch) - close(ch) - - fmt.Printf("Freed channel %v\n", ch) } // UnregisterFinalisedChannel removes the block finalisation notification channel with the given ID. @@ -91,7 +87,6 @@ func (bs *BlockState) notifyImported(block *types.Block) { logger.Trace("notifying imported block chans...", "chans", bs.imported) for ch := range bs.imported { - fmt.Printf("CHANNEL %v\n", ch) go func(ch chan<- *types.Block) { select { case ch <- block: From 74f094d42cd32e37e1f7884dbb43a9c835eb0b0f Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Fri, 3 Sep 2021 14:19:12 -0400 Subject: [PATCH 15/27] add sleep before close --- dot/state/block_notify.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index 93908eeb47..dec4205e46 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -19,6 +19,7 @@ package state import ( "errors" "sync" + "time" "github.com/ChainSafe/gossamer/dot/types" "github.com/ChainSafe/gossamer/lib/common" @@ -62,6 +63,8 @@ func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { defer bs.importedLock.Unlock() delete(bs.imported, ch) + time.Sleep(time.Millisecond * 50) + close(ch) } // UnregisterFinalisedChannel removes the block finalisation notification channel with the given ID. From 8a49ae79bc370bca4f9d8e7d460f73dbffc7b67b Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Fri, 3 Sep 2021 14:27:40 -0400 Subject: [PATCH 16/27] remove channel close --- dot/state/block_notify.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index dec4205e46..2fcc88e54c 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -18,13 +18,11 @@ package state import ( "errors" - "sync" - "time" - "github.com/ChainSafe/gossamer/dot/types" "github.com/ChainSafe/gossamer/lib/common" "github.com/ChainSafe/gossamer/lib/runtime" "github.com/google/uuid" + "sync" ) // DEFAULT_BUFFER_SIZE buffer size for channels @@ -63,8 +61,6 @@ func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { defer bs.importedLock.Unlock() delete(bs.imported, ch) - time.Sleep(time.Millisecond * 50) - close(ch) } // UnregisterFinalisedChannel removes the block finalisation notification channel with the given ID. From 35180df839e29cac7753b3af0ede159ea4d32bee Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Fri, 3 Sep 2021 14:33:36 -0400 Subject: [PATCH 17/27] run go imported --- dot/state/block_notify.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index 2fcc88e54c..93908eeb47 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -18,11 +18,12 @@ package state import ( "errors" + "sync" + "github.com/ChainSafe/gossamer/dot/types" "github.com/ChainSafe/gossamer/lib/common" "github.com/ChainSafe/gossamer/lib/runtime" "github.com/google/uuid" - "sync" ) // DEFAULT_BUFFER_SIZE buffer size for channels From faf26649579992d8e1e93dd044a6422960c06b96 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Tue, 7 Sep 2021 10:23:50 -0400 Subject: [PATCH 18/27] defer importedLock unlock --- dot/state/block_notify.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index 93908eeb47..182a6c6bff 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -31,10 +31,12 @@ const DEFAULT_BUFFER_SIZE = 100 // GetImportedBlockNotifierChannel function to retrieve a imported block notifier channel func (bs *BlockState) GetImportedBlockNotifierChannel() (chan *types.Block, error) { - ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) bs.importedLock.Lock() + defer bs.importedLock.Unlock() + + ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) bs.imported[ch] = struct{}{} - bs.importedLock.Unlock() + return ch, nil } From db397cda9571306628e33cd60279d1afb9a1b546 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Wed, 8 Sep 2021 18:34:26 -0400 Subject: [PATCH 19/27] wrap notifier channel in struct --- dot/core/interface.go | 7 +++++-- dot/state/block.go | 7 ++++--- dot/state/block_notify.go | 37 +++++++++++++++++++++++++++------- dot/state/block_notify_test.go | 8 ++++---- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/dot/core/interface.go b/dot/core/interface.go index b120248990..094c935833 100644 --- a/dot/core/interface.go +++ b/dot/core/interface.go @@ -17,6 +17,7 @@ package core import ( + "github.com/ChainSafe/gossamer/dot/state" "math/big" "github.com/ChainSafe/gossamer/dot/network" @@ -41,8 +42,10 @@ type BlockState interface { GetSlotForBlock(common.Hash) (uint64, error) GetFinalisedHeader(uint64, uint64) (*types.Header, error) GetFinalisedHash(uint64, uint64) (common.Hash, error) - GetImportedBlockNotifierChannel() (chan *types.Block, error) - FreeImportedBlockNotifierChannel(ch chan *types.Block) + //GetImportedBlockNotifierChannel() (<-chan *types.Block, error) + //FreeImportedBlockNotifierChannel(ch <-chan *types.Block) + GetImportedBlockNotifierChannel() (*state.ImportNotifier, error) + FreeImportedBlockNotifierChannel(notifier *state.ImportNotifier) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) HighestCommonAncestor(a, b common.Hash) (common.Hash, error) diff --git a/dot/state/block.go b/dot/state/block.go index a972ecf245..725e326c74 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -50,7 +50,8 @@ type BlockState struct { lastFinalised common.Hash // block notifiers - imported map[chan<- *types.Block]struct{} + //imported map[chan *types.Block]struct{} + imported map[*ImportNotifier]struct{} finalised map[byte]chan<- *types.FinalisationInfo finalisedLock sync.RWMutex importedLock sync.RWMutex @@ -72,7 +73,7 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e dbPath: db.Path(), baseState: NewBaseState(db), db: chaindb.NewTable(db, blockPrefix), - imported: make(map[chan<- *types.Block]struct{}), + imported: make(map[*ImportNotifier]struct{}), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), @@ -99,7 +100,7 @@ func NewBlockStateFromGenesis(db chaindb.Database, header *types.Header) (*Block bt: blocktree.NewBlockTreeFromRoot(header, db), baseState: NewBaseState(db), db: chaindb.NewTable(db, blockPrefix), - imported: make(map[chan<- *types.Block]struct{}), + imported: make(map[*ImportNotifier]struct{}), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index 182a6c6bff..e77a6d7f0f 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -29,15 +29,32 @@ import ( // DEFAULT_BUFFER_SIZE buffer size for channels const DEFAULT_BUFFER_SIZE = 100 +type ImportNotifier struct { + ch chan *types.Block +} + // GetImportedBlockNotifierChannel function to retrieve a imported block notifier channel -func (bs *BlockState) GetImportedBlockNotifierChannel() (chan *types.Block, error) { +//func (bs *BlockState) GetImportedBlockNotifierChannel() (<-chan *types.Block, error) { +// bs.importedLock.Lock() +// defer bs.importedLock.Unlock() +// +// ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) +// bs.imported[ch] = struct{}{} +// +// return ch, nil +//} + +func (bs *BlockState) GetImportedBlockNotifierChannel() (*ImportNotifier, error) { bs.importedLock.Lock() defer bs.importedLock.Unlock() - ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) - bs.imported[ch] = struct{}{} + in := &ImportNotifier{ + ch: make(chan *types.Block, DEFAULT_BUFFER_SIZE), + } + //ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) + bs.imported[in] = struct{}{} - return ch, nil + return in, nil } // RegisterFinalizedChannel registers a channel for block notification upon block finalisation. @@ -59,10 +76,16 @@ func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo } // FreeImportedBlockNotifierChannel to free and close imported block notifier channel -func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { +//func (bs *BlockState) FreeImportedBlockNotifierChannel(ch <-chan *types.Block) { +// bs.importedLock.Lock() +// defer bs.importedLock.Unlock() +//fmt.Printf("delete chan %v\n", ch) +// delete(bs.imported, ch) +//} +func (bs *BlockState) FreeImportedBlockNotifierChannel(ch *ImportNotifier) { bs.importedLock.Lock() defer bs.importedLock.Unlock() - + // todo (ed) add test to confirm this is deleting delete(bs.imported, ch) } @@ -94,7 +117,7 @@ func (bs *BlockState) notifyImported(block *types.Block) { case ch <- block: default: } - }(ch) + }(ch.ch) } } diff --git a/dot/state/block_notify_test.go b/dot/state/block_notify_test.go index 9c9cdd55a2..06df76063b 100644 --- a/dot/state/block_notify_test.go +++ b/dot/state/block_notify_test.go @@ -42,7 +42,7 @@ func TestImportChannel(t *testing.T) { for i := 0; i < 3; i++ { select { - case <-ch: + case <-ch.ch: case <-time.After(testMessageTimeout): t.Fatal("did not receive imported block") } @@ -77,7 +77,7 @@ func TestImportChannel_Multi(t *testing.T) { bs := newTestBlockState(t, testGenesisHeader) num := 5 - chs := make([]chan *types.Block, num) + chs := make([]*ImportNotifier, num) var err error for i := 0; i < num; i++ { @@ -90,7 +90,7 @@ func TestImportChannel_Multi(t *testing.T) { for i, ch := range chs { - go func(i int, ch chan *types.Block) { + go func(i int, ch <-chan *types.Block) { select { case b := <-ch: require.Equal(t, big.NewInt(1), b.Header.Number) @@ -98,7 +98,7 @@ func TestImportChannel_Multi(t *testing.T) { t.Error("did not receive imported block: ch=", i) } wg.Done() - }(i, ch) + }(i, ch.ch) } From d5d0d57cc93685e6c9fe4ebb649a1dfd48955c2d Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 9 Sep 2021 17:10:32 -0400 Subject: [PATCH 20/27] store channel by interface{} key --- dot/core/interface.go | 5 ++- dot/state/block.go | 7 ++-- dot/state/block_notify.go | 62 ++++++++++++++++++---------------- dot/state/block_notify_test.go | 13 ++++--- 4 files changed, 43 insertions(+), 44 deletions(-) diff --git a/dot/core/interface.go b/dot/core/interface.go index 094c935833..249a8b8389 100644 --- a/dot/core/interface.go +++ b/dot/core/interface.go @@ -17,7 +17,6 @@ package core import ( - "github.com/ChainSafe/gossamer/dot/state" "math/big" "github.com/ChainSafe/gossamer/dot/network" @@ -44,8 +43,8 @@ type BlockState interface { GetFinalisedHash(uint64, uint64) (common.Hash, error) //GetImportedBlockNotifierChannel() (<-chan *types.Block, error) //FreeImportedBlockNotifierChannel(ch <-chan *types.Block) - GetImportedBlockNotifierChannel() (*state.ImportNotifier, error) - FreeImportedBlockNotifierChannel(notifier *state.ImportNotifier) + GetImportedBlockNotifierChannel() (interface{}, error) + FreeImportedBlockNotifierChannel(notifier interface{}) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) HighestCommonAncestor(a, b common.Hash) (common.Hash, error) diff --git a/dot/state/block.go b/dot/state/block.go index 725e326c74..8737ac078a 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -50,8 +50,7 @@ type BlockState struct { lastFinalised common.Hash // block notifiers - //imported map[chan *types.Block]struct{} - imported map[*ImportNotifier]struct{} + imported map[interface{}]chan<- *types.Block finalised map[byte]chan<- *types.FinalisationInfo finalisedLock sync.RWMutex importedLock sync.RWMutex @@ -73,7 +72,7 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e dbPath: db.Path(), baseState: NewBaseState(db), db: chaindb.NewTable(db, blockPrefix), - imported: make(map[*ImportNotifier]struct{}), + imported: make(map[interface{}]chan<- *types.Block), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), @@ -100,7 +99,7 @@ func NewBlockStateFromGenesis(db chaindb.Database, header *types.Header) (*Block bt: blocktree.NewBlockTreeFromRoot(header, db), baseState: NewBaseState(db), db: chaindb.NewTable(db, blockPrefix), - imported: make(map[*ImportNotifier]struct{}), + imported: make(map[interface{}]chan<- *types.Block), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index e77a6d7f0f..5a13387779 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -18,6 +18,7 @@ package state import ( "errors" + "fmt" "sync" "github.com/ChainSafe/gossamer/dot/types" @@ -29,34 +30,34 @@ import ( // DEFAULT_BUFFER_SIZE buffer size for channels const DEFAULT_BUFFER_SIZE = 100 -type ImportNotifier struct { - ch chan *types.Block -} +//type ImportNotifier struct { +// ch chan *types.Block +//} // GetImportedBlockNotifierChannel function to retrieve a imported block notifier channel -//func (bs *BlockState) GetImportedBlockNotifierChannel() (<-chan *types.Block, error) { +func (bs *BlockState) GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) { + bs.importedLock.Lock() + defer bs.importedLock.Unlock() + + ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) + bs.imported[conn] = ch + fmt.Printf("Added conn %v\n", conn) + return ch, nil +} + +//func (bs *BlockState) GetImportedBlockNotifierChannel() (*ImportNotifier, error) { // bs.importedLock.Lock() // defer bs.importedLock.Unlock() // -// ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) -// bs.imported[ch] = struct{}{} +// in := &ImportNotifier{ +// ch: make(chan *types.Block, DEFAULT_BUFFER_SIZE), +// } +// //ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) +// bs.imported[in] = struct{}{} // -// return ch, nil +// return in, nil //} -func (bs *BlockState) GetImportedBlockNotifierChannel() (*ImportNotifier, error) { - bs.importedLock.Lock() - defer bs.importedLock.Unlock() - - in := &ImportNotifier{ - ch: make(chan *types.Block, DEFAULT_BUFFER_SIZE), - } - //ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) - bs.imported[in] = struct{}{} - - return in, nil -} - // RegisterFinalizedChannel registers a channel for block notification upon block finalisation. // It returns the channel ID (used for unregistering the channel) func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) { @@ -76,18 +77,19 @@ func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo } // FreeImportedBlockNotifierChannel to free and close imported block notifier channel -//func (bs *BlockState) FreeImportedBlockNotifierChannel(ch <-chan *types.Block) { +func (bs *BlockState) FreeImportedBlockNotifierChannel(conn interface{}) { + bs.importedLock.Lock() + defer bs.importedLock.Unlock() + fmt.Printf("delete chan %v\n", conn) + delete(bs.imported, conn) +} + +//func (bs *BlockState) FreeImportedBlockNotifierChannel(ch *ImportNotifier) { // bs.importedLock.Lock() // defer bs.importedLock.Unlock() -//fmt.Printf("delete chan %v\n", ch) +// // todo (ed) add test to confirm this is deleting // delete(bs.imported, ch) //} -func (bs *BlockState) FreeImportedBlockNotifierChannel(ch *ImportNotifier) { - bs.importedLock.Lock() - defer bs.importedLock.Unlock() - // todo (ed) add test to confirm this is deleting - delete(bs.imported, ch) -} // UnregisterFinalisedChannel removes the block finalisation notification channel with the given ID. // A channel must be unregistered before closing it. @@ -111,13 +113,13 @@ func (bs *BlockState) notifyImported(block *types.Block) { } logger.Trace("notifying imported block chans...", "chans", bs.imported) - for ch := range bs.imported { + for _, ch := range bs.imported { go func(ch chan<- *types.Block) { select { case ch <- block: default: } - }(ch.ch) + }(ch) } } diff --git a/dot/state/block_notify_test.go b/dot/state/block_notify_test.go index 06df76063b..8c9dce1383 100644 --- a/dot/state/block_notify_test.go +++ b/dot/state/block_notify_test.go @@ -32,17 +32,16 @@ var testMessageTimeout = time.Second * 3 func TestImportChannel(t *testing.T) { bs := newTestBlockState(t, testGenesisHeader) - - ch, err := bs.GetImportedBlockNotifierChannel() + ch, err := bs.GetImportedBlockNotifierChannel(bs) require.NoError(t, err) - defer bs.FreeImportedBlockNotifierChannel(ch) + defer bs.FreeImportedBlockNotifierChannel(bs) AddBlocksToState(t, bs, 3) for i := 0; i < 3; i++ { select { - case <-ch.ch: + case <-ch: case <-time.After(testMessageTimeout): t.Fatal("did not receive imported block") } @@ -77,11 +76,11 @@ func TestImportChannel_Multi(t *testing.T) { bs := newTestBlockState(t, testGenesisHeader) num := 5 - chs := make([]*ImportNotifier, num) + chs := make([]<-chan *types.Block, num) var err error for i := 0; i < num; i++ { - chs[i], err = bs.GetImportedBlockNotifierChannel() + chs[i], err = bs.GetImportedBlockNotifierChannel(i) require.NoError(t, err) } @@ -98,7 +97,7 @@ func TestImportChannel_Multi(t *testing.T) { t.Error("did not receive imported block: ch=", i) } wg.Done() - }(i, ch.ch) + }(i, ch) } From a478d5c20a8671aabe608ef923e32ecdc261049c Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 9 Sep 2021 18:02:01 -0400 Subject: [PATCH 21/27] update storage key for imported block listeners --- dot/core/interface.go | 6 ++-- dot/core/mocks/block_state.go | 24 +++++++-------- dot/digest/digest.go | 6 ++-- dot/digest/interface.go | 4 +-- dot/rpc/modules/api.go | 4 +-- dot/rpc/modules/mocks/block_api.go | 42 +++++++++++++------------- dot/rpc/subscription/listeners.go | 6 ++-- dot/rpc/subscription/listeners_test.go | 8 ++--- dot/rpc/subscription/websocket.go | 6 ++-- lib/grandpa/grandpa.go | 4 +-- lib/grandpa/message_tracker.go | 6 ++-- lib/grandpa/state.go | 4 +-- 12 files changed, 59 insertions(+), 61 deletions(-) diff --git a/dot/core/interface.go b/dot/core/interface.go index 249a8b8389..3d1634add5 100644 --- a/dot/core/interface.go +++ b/dot/core/interface.go @@ -41,10 +41,8 @@ type BlockState interface { GetSlotForBlock(common.Hash) (uint64, error) GetFinalisedHeader(uint64, uint64) (*types.Header, error) GetFinalisedHash(uint64, uint64) (common.Hash, error) - //GetImportedBlockNotifierChannel() (<-chan *types.Block, error) - //FreeImportedBlockNotifierChannel(ch <-chan *types.Block) - GetImportedBlockNotifierChannel() (interface{}, error) - FreeImportedBlockNotifierChannel(notifier interface{}) + GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) + FreeImportedBlockNotifierChannel(conn interface{}) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) HighestCommonAncestor(a, b common.Hash) (common.Hash, error) diff --git a/dot/core/mocks/block_state.go b/dot/core/mocks/block_state.go index 0f12b9c70d..2bfa6dff8d 100644 --- a/dot/core/mocks/block_state.go +++ b/dot/core/mocks/block_state.go @@ -143,9 +143,9 @@ func (_m *MockBlockState) BestBlockStateRoot() (common.Hash, error) { return r0, r1 } -// FreeImportedBlockNotifierChannel provides a mock function with given fields: ch -func (_m *MockBlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { - _m.Called(ch) +// FreeImportedBlockNotifierChannel provides a mock function with given fields: notifier +func (_m *MockBlockState) FreeImportedBlockNotifierChannel(notifier interface{}) { + _m.Called(notifier) } // GenesisHash provides a mock function with given fields: @@ -272,22 +272,22 @@ func (_m *MockBlockState) GetFinalisedHeader(_a0 uint64, _a1 uint64) (*types.Hea return r0, r1 } -// GetImportedBlockNotifierChannel provides a mock function with given fields: -func (_m *MockBlockState) GetImportedBlockNotifierChannel() (chan *types.Block, error) { - ret := _m.Called() +// GetImportedBlockNotifierChannel provides a mock function with given fields: conn +func (_m *MockBlockState) GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) { + ret := _m.Called(conn) - var r0 chan *types.Block - if rf, ok := ret.Get(0).(func() chan *types.Block); ok { - r0 = rf() + var r0 <-chan *types.Block + if rf, ok := ret.Get(0).(func(interface{}) <-chan *types.Block); ok { + r0 = rf(conn) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(chan *types.Block) + r0 = ret.Get(0).(<-chan *types.Block) } } var r1 error - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() + if rf, ok := ret.Get(1).(func(interface{}) error); ok { + r1 = rf(conn) } else { r1 = ret.Error(1) } diff --git a/dot/digest/digest.go b/dot/digest/digest.go index e2258e0fd5..0856687c1c 100644 --- a/dot/digest/digest.go +++ b/dot/digest/digest.go @@ -47,7 +47,7 @@ type Handler struct { grandpaState GrandpaState // block notification channels - imported chan *types.Block + imported <-chan *types.Block finalised chan *types.FinalisationInfo finalisedID byte @@ -73,7 +73,7 @@ type resume struct { // NewHandler returns a new Handler func NewHandler(blockState BlockState, epochState EpochState, grandpaState GrandpaState) (*Handler, error) { - imported, err := blockState.GetImportedBlockNotifierChannel() + imported, err := blockState.GetImportedBlockNotifierChannel(blockState) if err != nil { return nil, err } @@ -109,7 +109,7 @@ func (h *Handler) Start() error { // Stop stops the Handler func (h *Handler) Stop() error { h.cancel() - h.blockState.FreeImportedBlockNotifierChannel(h.imported) + h.blockState.FreeImportedBlockNotifierChannel(h.blockState) h.blockState.UnregisterFinalisedChannel(h.finalisedID) close(h.finalised) return nil diff --git a/dot/digest/interface.go b/dot/digest/interface.go index 6bee49acb5..1947874cd8 100644 --- a/dot/digest/interface.go +++ b/dot/digest/interface.go @@ -26,8 +26,8 @@ import ( // BlockState interface for block state methods type BlockState interface { BestBlockHeader() (*types.Header, error) - GetImportedBlockNotifierChannel() (chan *types.Block, error) - FreeImportedBlockNotifierChannel(ch chan *types.Block) + GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) + FreeImportedBlockNotifierChannel(conn interface{}) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) } diff --git a/dot/rpc/modules/api.go b/dot/rpc/modules/api.go index 220ed45776..4d8cffcf09 100644 --- a/dot/rpc/modules/api.go +++ b/dot/rpc/modules/api.go @@ -35,8 +35,8 @@ type BlockAPI interface { GetHighestFinalisedHash() (common.Hash, error) HasJustification(hash common.Hash) (bool, error) GetJustification(hash common.Hash) ([]byte, error) - GetImportedBlockNotifierChannel() (chan *types.Block, error) - FreeImportedBlockNotifierChannel(ch chan *types.Block) + GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) + FreeImportedBlockNotifierChannel(conn interface{}) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) SubChain(start, end common.Hash) ([]common.Hash, error) diff --git a/dot/rpc/modules/mocks/block_api.go b/dot/rpc/modules/mocks/block_api.go index 5ce7651249..f6c40d032d 100644 --- a/dot/rpc/modules/mocks/block_api.go +++ b/dot/rpc/modules/mocks/block_api.go @@ -34,9 +34,9 @@ func (_m *MockBlockAPI) BestBlockHash() common.Hash { return r0 } -// FreeNotifierChannel provides a mock function with given fields: ch -func (_m *MockBlockAPI) FreeImportedBlockNotifierChannel(ch chan *types.Block) { - _m.Called(ch) +// FreeImportedBlockNotifierChannel provides a mock function with given fields: conn +func (_m *MockBlockAPI) FreeImportedBlockNotifierChannel(conn interface{}) { + _m.Called(conn) } // GetBlockByHash provides a mock function with given fields: hash @@ -154,22 +154,22 @@ func (_m *MockBlockAPI) GetHighestFinalisedHash() (common.Hash, error) { return r0, r1 } -// GetJustification provides a mock function with given fields: hash -func (_m *MockBlockAPI) GetJustification(hash common.Hash) ([]byte, error) { - ret := _m.Called(hash) +// GetImportedBlockNotifierChannel provides a mock function with given fields: conn +func (_m *MockBlockAPI) GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) { + ret := _m.Called(conn) - var r0 []byte - if rf, ok := ret.Get(0).(func(common.Hash) []byte); ok { - r0 = rf(hash) + var r0 <-chan *types.Block + if rf, ok := ret.Get(0).(func(interface{}) <-chan *types.Block); ok { + r0 = rf(conn) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]byte) + r0 = ret.Get(0).(<-chan *types.Block) } } var r1 error - if rf, ok := ret.Get(1).(func(common.Hash) error); ok { - r1 = rf(hash) + if rf, ok := ret.Get(1).(func(interface{}) error); ok { + r1 = rf(conn) } else { r1 = ret.Error(1) } @@ -177,22 +177,22 @@ func (_m *MockBlockAPI) GetJustification(hash common.Hash) ([]byte, error) { return r0, r1 } -// GetNotifierChannel provides a mock function with given fields: -func (_m *MockBlockAPI) GetImportedBlockNotifierChannel() (chan *types.Block, error) { - ret := _m.Called() +// GetJustification provides a mock function with given fields: hash +func (_m *MockBlockAPI) GetJustification(hash common.Hash) ([]byte, error) { + ret := _m.Called(hash) - var r0 chan *types.Block - if rf, ok := ret.Get(0).(func() chan *types.Block); ok { - r0 = rf() + var r0 []byte + if rf, ok := ret.Get(0).(func(common.Hash) []byte); ok { + r0 = rf(hash) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(chan *types.Block) + r0 = ret.Get(0).([]byte) } } var r1 error - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() + if rf, ok := ret.Get(1).(func(common.Hash) error); ok { + r1 = rf(hash) } else { r1 = ret.Error(1) } diff --git a/dot/rpc/subscription/listeners.go b/dot/rpc/subscription/listeners.go index 460f542fe9..9f31f4fcde 100644 --- a/dot/rpc/subscription/listeners.go +++ b/dot/rpc/subscription/listeners.go @@ -115,7 +115,7 @@ func (s *StorageObserver) Stop() error { // BlockListener to handle listening for blocks importedChan type BlockListener struct { - Channel chan *types.Block + Channel <-chan *types.Block wsconn *WSConn subID uint32 done chan struct{} @@ -228,7 +228,7 @@ func (l *BlockFinalizedListener) Stop() error { // AllBlocksListener is a listener that is aware of new and newly finalised blocks``` type AllBlocksListener struct { finalizedChan chan *types.FinalisationInfo - importedChan chan *types.Block + importedChan <-chan *types.Block wsconn *WSConn finalizedChanID byte @@ -311,7 +311,7 @@ type ExtrinsicSubmitListener struct { wsconn *WSConn subID uint32 extrinsic types.Extrinsic - importedChan chan *types.Block + importedChan <-chan *types.Block importedHash common.Hash finalisedChan chan *types.FinalisationInfo finalisedChanID byte diff --git a/dot/rpc/subscription/listeners_test.go b/dot/rpc/subscription/listeners_test.go index 3188670f07..ab189e749c 100644 --- a/dot/rpc/subscription/listeners_test.go +++ b/dot/rpc/subscription/listeners_test.go @@ -97,7 +97,7 @@ func TestBlockListener_Listen(t *testing.T) { defer cancel() BlockAPI := new(mocks.MockBlockAPI) - BlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) + BlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("<-chan *types.Block")) wsconn.BlockAPI = BlockAPI @@ -117,7 +117,7 @@ func TestBlockListener_Listen(t *testing.T) { defer func() { require.NoError(t, bl.Stop()) time.Sleep(time.Millisecond * 10) - BlockAPI.AssertCalled(t, "FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) + BlockAPI.AssertCalled(t, "FreeImportedBlockNotifierChannel", mock.AnythingOfType("<-chan *types.Block")) }() notifyChan <- block @@ -195,7 +195,7 @@ func TestExtrinsicSubmitListener_Listen(t *testing.T) { notifyFinalizedChan := make(chan *types.FinalisationInfo, 100) BlockAPI := new(mocks.MockBlockAPI) - BlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) + BlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("<-chan *types.Block")) BlockAPI.On("UnregisterFinalisedChannel", mock.AnythingOfType("uint8")) wsconn.BlockAPI = BlockAPI @@ -225,7 +225,7 @@ func TestExtrinsicSubmitListener_Listen(t *testing.T) { require.NoError(t, esl.Stop()) time.Sleep(time.Millisecond * 10) - BlockAPI.AssertCalled(t, "FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) + BlockAPI.AssertCalled(t, "FreeImportedBlockNotifierChannel", mock.AnythingOfType("<-chan *types.Block")) BlockAPI.AssertCalled(t, "UnregisterFinalisedChannel", mock.AnythingOfType("uint8")) }() diff --git a/dot/rpc/subscription/websocket.go b/dot/rpc/subscription/websocket.go index 4adc1a21e2..0e063632ed 100644 --- a/dot/rpc/subscription/websocket.go +++ b/dot/rpc/subscription/websocket.go @@ -221,7 +221,7 @@ func (c *WSConn) initBlockListener(reqID float64, _ interface{}) (Listener, erro } var err error - bl.Channel, err = c.BlockAPI.GetImportedBlockNotifierChannel() + bl.Channel, err = c.BlockAPI.GetImportedBlockNotifierChannel(c.Wsconn) if err != nil { return nil, err } @@ -280,7 +280,7 @@ func (c *WSConn) initAllBlocksListerner(reqID float64, _ interface{}) (Listener, } var err error - listener.importedChan, err = c.BlockAPI.GetImportedBlockNotifierChannel() + listener.importedChan, err = c.BlockAPI.GetImportedBlockNotifierChannel(c.Wsconn) if err != nil { c.safeSendError(reqID, nil, "could not register imported channel") return nil, fmt.Errorf("could not register imported channel") @@ -315,7 +315,7 @@ func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener return nil, fmt.Errorf("error BlockAPI not set") } - esl.importedChan, err = c.BlockAPI.GetImportedBlockNotifierChannel() + esl.importedChan, err = c.BlockAPI.GetImportedBlockNotifierChannel(c.Wsconn) if err != nil { return nil, err } diff --git a/lib/grandpa/grandpa.go b/lib/grandpa/grandpa.go index 2455cfaede..80dd3911a9 100644 --- a/lib/grandpa/grandpa.go +++ b/lib/grandpa/grandpa.go @@ -376,12 +376,12 @@ func (s *Service) initiate() error { } func (s *Service) waitForFirstBlock() error { - ch, err := s.blockState.GetImportedBlockNotifierChannel() + ch, err := s.blockState.GetImportedBlockNotifierChannel(s.blockState) if err != nil { return err } - defer s.blockState.FreeImportedBlockNotifierChannel(ch) + defer s.blockState.FreeImportedBlockNotifierChannel(s.blockState) // loop until block 1 for { diff --git a/lib/grandpa/message_tracker.go b/lib/grandpa/message_tracker.go index 4144acc1fb..a3f6fb0a8c 100644 --- a/lib/grandpa/message_tracker.go +++ b/lib/grandpa/message_tracker.go @@ -32,12 +32,12 @@ type tracker struct { voteMessages map[common.Hash]map[ed25519.PublicKeyBytes]*networkVoteMessage // map of vote block hash -> array of VoteMessages for that hash commitMessages map[common.Hash]*CommitMessage // map of commit block hash to commit message mapLock sync.Mutex - in chan *types.Block // receive imported block from BlockState + in <-chan *types.Block // receive imported block from BlockState stopped chan struct{} } func newTracker(bs BlockState, handler *MessageHandler) (*tracker, error) { - in, err := bs.GetImportedBlockNotifierChannel() + in, err := bs.GetImportedBlockNotifierChannel(bs) if err != nil { return nil, err } @@ -59,7 +59,7 @@ func (t *tracker) start() { func (t *tracker) stop() { close(t.stopped) - t.blockState.FreeImportedBlockNotifierChannel(t.in) + t.blockState.FreeImportedBlockNotifierChannel(t.blockState) } func (t *tracker) addVote(v *networkVoteMessage) { diff --git a/lib/grandpa/state.go b/lib/grandpa/state.go index 688f291c49..c8579a70d4 100644 --- a/lib/grandpa/state.go +++ b/lib/grandpa/state.go @@ -41,8 +41,8 @@ type BlockState interface { BestBlockHash() common.Hash Leaves() []common.Hash BlocktreeAsString() string - GetImportedBlockNotifierChannel() (chan *types.Block, error) - FreeImportedBlockNotifierChannel(ch chan *types.Block) + GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) + FreeImportedBlockNotifierChannel(conn interface{}) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) SetJustification(hash common.Hash, data []byte) error From 8b8066c9cc050d8dd1a2f6fad7cf8b52a4356de4 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Fri, 10 Sep 2021 15:32:03 -0400 Subject: [PATCH 22/27] refacter GetImportedBlockNotifierChannel arugments --- dot/core/interface.go | 4 ++-- dot/core/mocks/block_state.go | 29 ++++++++++---------------- dot/digest/digest.go | 9 +++----- dot/digest/interface.go | 4 ++-- dot/rpc/modules/api.go | 4 ++-- dot/rpc/modules/mocks/block_api.go | 29 ++++++++++---------------- dot/rpc/subscription/listeners.go | 6 +++--- dot/rpc/subscription/listeners_test.go | 8 +++---- dot/rpc/subscription/websocket.go | 19 ++++------------- dot/rpc/subscription/websocket_test.go | 1 + dot/state/block.go | 6 +++--- dot/state/block_notify.go | 21 +++++++------------ dot/state/block_notify_test.go | 11 ++++------ lib/grandpa/grandpa.go | 12 +++-------- lib/grandpa/message_tracker.go | 13 +++++------- lib/grandpa/message_tracker_test.go | 9 +++----- lib/grandpa/state.go | 4 ++-- lib/grandpa/vote_message_test.go | 6 ++---- 18 files changed, 73 insertions(+), 122 deletions(-) diff --git a/dot/core/interface.go b/dot/core/interface.go index 3d1634add5..505b04c264 100644 --- a/dot/core/interface.go +++ b/dot/core/interface.go @@ -41,8 +41,8 @@ type BlockState interface { GetSlotForBlock(common.Hash) (uint64, error) GetFinalisedHeader(uint64, uint64) (*types.Header, error) GetFinalisedHash(uint64, uint64) (common.Hash, error) - GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) - FreeImportedBlockNotifierChannel(conn interface{}) + GetImportedBlockNotifierChannel() chan *types.Block + FreeImportedBlockNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) HighestCommonAncestor(a, b common.Hash) (common.Hash, error) diff --git a/dot/core/mocks/block_state.go b/dot/core/mocks/block_state.go index 2bfa6dff8d..65cc19ed21 100644 --- a/dot/core/mocks/block_state.go +++ b/dot/core/mocks/block_state.go @@ -143,9 +143,9 @@ func (_m *MockBlockState) BestBlockStateRoot() (common.Hash, error) { return r0, r1 } -// FreeImportedBlockNotifierChannel provides a mock function with given fields: notifier -func (_m *MockBlockState) FreeImportedBlockNotifierChannel(notifier interface{}) { - _m.Called(notifier) +// FreeImportedBlockNotifierChannel provides a mock function with given fields: ch +func (_m *MockBlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { + _m.Called(ch) } // GenesisHash provides a mock function with given fields: @@ -272,27 +272,20 @@ func (_m *MockBlockState) GetFinalisedHeader(_a0 uint64, _a1 uint64) (*types.Hea return r0, r1 } -// GetImportedBlockNotifierChannel provides a mock function with given fields: conn -func (_m *MockBlockState) GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) { - ret := _m.Called(conn) +// GetImportedBlockNotifierChannel provides a mock function with given fields: +func (_m *MockBlockState) GetImportedBlockNotifierChannel() chan *types.Block { + ret := _m.Called() - var r0 <-chan *types.Block - if rf, ok := ret.Get(0).(func(interface{}) <-chan *types.Block); ok { - r0 = rf(conn) + var r0 chan *types.Block + if rf, ok := ret.Get(0).(func() chan *types.Block); ok { + r0 = rf() } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(<-chan *types.Block) + r0 = ret.Get(0).(chan *types.Block) } } - var r1 error - if rf, ok := ret.Get(1).(func(interface{}) error); ok { - r1 = rf(conn) - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return r0 } // GetRuntime provides a mock function with given fields: _a0 diff --git a/dot/digest/digest.go b/dot/digest/digest.go index 0856687c1c..808553008e 100644 --- a/dot/digest/digest.go +++ b/dot/digest/digest.go @@ -47,7 +47,7 @@ type Handler struct { grandpaState GrandpaState // block notification channels - imported <-chan *types.Block + imported chan *types.Block finalised chan *types.FinalisationInfo finalisedID byte @@ -73,10 +73,7 @@ type resume struct { // NewHandler returns a new Handler func NewHandler(blockState BlockState, epochState EpochState, grandpaState GrandpaState) (*Handler, error) { - imported, err := blockState.GetImportedBlockNotifierChannel(blockState) - if err != nil { - return nil, err - } + imported := blockState.GetImportedBlockNotifierChannel() finalised := make(chan *types.FinalisationInfo, 16) @@ -109,7 +106,7 @@ func (h *Handler) Start() error { // Stop stops the Handler func (h *Handler) Stop() error { h.cancel() - h.blockState.FreeImportedBlockNotifierChannel(h.blockState) + h.blockState.FreeImportedBlockNotifierChannel(h.imported) h.blockState.UnregisterFinalisedChannel(h.finalisedID) close(h.finalised) return nil diff --git a/dot/digest/interface.go b/dot/digest/interface.go index 1947874cd8..a19769c075 100644 --- a/dot/digest/interface.go +++ b/dot/digest/interface.go @@ -26,8 +26,8 @@ import ( // BlockState interface for block state methods type BlockState interface { BestBlockHeader() (*types.Header, error) - GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) - FreeImportedBlockNotifierChannel(conn interface{}) + GetImportedBlockNotifierChannel() chan *types.Block + FreeImportedBlockNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) } diff --git a/dot/rpc/modules/api.go b/dot/rpc/modules/api.go index 8cf09c4b73..febd9dac80 100644 --- a/dot/rpc/modules/api.go +++ b/dot/rpc/modules/api.go @@ -35,8 +35,8 @@ type BlockAPI interface { GetHighestFinalisedHash() (common.Hash, error) HasJustification(hash common.Hash) (bool, error) GetJustification(hash common.Hash) ([]byte, error) - GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) - FreeImportedBlockNotifierChannel(conn interface{}) + GetImportedBlockNotifierChannel() chan *types.Block + FreeImportedBlockNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) SubChain(start, end common.Hash) ([]common.Hash, error) diff --git a/dot/rpc/modules/mocks/block_api.go b/dot/rpc/modules/mocks/block_api.go index f6c40d032d..d343fe1627 100644 --- a/dot/rpc/modules/mocks/block_api.go +++ b/dot/rpc/modules/mocks/block_api.go @@ -34,9 +34,9 @@ func (_m *MockBlockAPI) BestBlockHash() common.Hash { return r0 } -// FreeImportedBlockNotifierChannel provides a mock function with given fields: conn -func (_m *MockBlockAPI) FreeImportedBlockNotifierChannel(conn interface{}) { - _m.Called(conn) +// FreeImportedBlockNotifierChannel provides a mock function with given fields: ch +func (_m *MockBlockAPI) FreeImportedBlockNotifierChannel(ch chan *types.Block) { + _m.Called(ch) } // GetBlockByHash provides a mock function with given fields: hash @@ -154,27 +154,20 @@ func (_m *MockBlockAPI) GetHighestFinalisedHash() (common.Hash, error) { return r0, r1 } -// GetImportedBlockNotifierChannel provides a mock function with given fields: conn -func (_m *MockBlockAPI) GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) { - ret := _m.Called(conn) +// GetImportedBlockNotifierChannel provides a mock function with given fields: +func (_m *MockBlockAPI) GetImportedBlockNotifierChannel() chan *types.Block { + ret := _m.Called() - var r0 <-chan *types.Block - if rf, ok := ret.Get(0).(func(interface{}) <-chan *types.Block); ok { - r0 = rf(conn) + var r0 chan *types.Block + if rf, ok := ret.Get(0).(func() chan *types.Block); ok { + r0 = rf() } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(<-chan *types.Block) + r0 = ret.Get(0).(chan *types.Block) } } - var r1 error - if rf, ok := ret.Get(1).(func(interface{}) error); ok { - r1 = rf(conn) - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return r0 } // GetJustification provides a mock function with given fields: hash diff --git a/dot/rpc/subscription/listeners.go b/dot/rpc/subscription/listeners.go index 9f31f4fcde..460f542fe9 100644 --- a/dot/rpc/subscription/listeners.go +++ b/dot/rpc/subscription/listeners.go @@ -115,7 +115,7 @@ func (s *StorageObserver) Stop() error { // BlockListener to handle listening for blocks importedChan type BlockListener struct { - Channel <-chan *types.Block + Channel chan *types.Block wsconn *WSConn subID uint32 done chan struct{} @@ -228,7 +228,7 @@ func (l *BlockFinalizedListener) Stop() error { // AllBlocksListener is a listener that is aware of new and newly finalised blocks``` type AllBlocksListener struct { finalizedChan chan *types.FinalisationInfo - importedChan <-chan *types.Block + importedChan chan *types.Block wsconn *WSConn finalizedChanID byte @@ -311,7 +311,7 @@ type ExtrinsicSubmitListener struct { wsconn *WSConn subID uint32 extrinsic types.Extrinsic - importedChan <-chan *types.Block + importedChan chan *types.Block importedHash common.Hash finalisedChan chan *types.FinalisationInfo finalisedChanID byte diff --git a/dot/rpc/subscription/listeners_test.go b/dot/rpc/subscription/listeners_test.go index ab189e749c..3188670f07 100644 --- a/dot/rpc/subscription/listeners_test.go +++ b/dot/rpc/subscription/listeners_test.go @@ -97,7 +97,7 @@ func TestBlockListener_Listen(t *testing.T) { defer cancel() BlockAPI := new(mocks.MockBlockAPI) - BlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("<-chan *types.Block")) + BlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) wsconn.BlockAPI = BlockAPI @@ -117,7 +117,7 @@ func TestBlockListener_Listen(t *testing.T) { defer func() { require.NoError(t, bl.Stop()) time.Sleep(time.Millisecond * 10) - BlockAPI.AssertCalled(t, "FreeImportedBlockNotifierChannel", mock.AnythingOfType("<-chan *types.Block")) + BlockAPI.AssertCalled(t, "FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) }() notifyChan <- block @@ -195,7 +195,7 @@ func TestExtrinsicSubmitListener_Listen(t *testing.T) { notifyFinalizedChan := make(chan *types.FinalisationInfo, 100) BlockAPI := new(mocks.MockBlockAPI) - BlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("<-chan *types.Block")) + BlockAPI.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) BlockAPI.On("UnregisterFinalisedChannel", mock.AnythingOfType("uint8")) wsconn.BlockAPI = BlockAPI @@ -225,7 +225,7 @@ func TestExtrinsicSubmitListener_Listen(t *testing.T) { require.NoError(t, esl.Stop()) time.Sleep(time.Millisecond * 10) - BlockAPI.AssertCalled(t, "FreeImportedBlockNotifierChannel", mock.AnythingOfType("<-chan *types.Block")) + BlockAPI.AssertCalled(t, "FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) BlockAPI.AssertCalled(t, "UnregisterFinalisedChannel", mock.AnythingOfType("uint8")) }() diff --git a/dot/rpc/subscription/websocket.go b/dot/rpc/subscription/websocket.go index 0e063632ed..ff8742407f 100644 --- a/dot/rpc/subscription/websocket.go +++ b/dot/rpc/subscription/websocket.go @@ -220,11 +220,7 @@ func (c *WSConn) initBlockListener(reqID float64, _ interface{}) (Listener, erro return nil, fmt.Errorf("error BlockAPI not set") } - var err error - bl.Channel, err = c.BlockAPI.GetImportedBlockNotifierChannel(c.Wsconn) - if err != nil { - return nil, err - } + bl.Channel = c.BlockAPI.GetImportedBlockNotifierChannel() c.mu.Lock() @@ -279,13 +275,9 @@ func (c *WSConn) initAllBlocksListerner(reqID float64, _ interface{}) (Listener, return nil, fmt.Errorf("error BlockAPI not set") } - var err error - listener.importedChan, err = c.BlockAPI.GetImportedBlockNotifierChannel(c.Wsconn) - if err != nil { - c.safeSendError(reqID, nil, "could not register imported channel") - return nil, fmt.Errorf("could not register imported channel") - } + listener.importedChan = c.BlockAPI.GetImportedBlockNotifierChannel() + var err error listener.finalizedChanID, err = c.BlockAPI.RegisterFinalizedChannel(listener.finalizedChan) if err != nil { c.safeSendError(reqID, nil, "could not register finalised channel") @@ -315,10 +307,7 @@ func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener return nil, fmt.Errorf("error BlockAPI not set") } - esl.importedChan, err = c.BlockAPI.GetImportedBlockNotifierChannel(c.Wsconn) - if err != nil { - return nil, err - } + esl.importedChan = c.BlockAPI.GetImportedBlockNotifierChannel() esl.finalisedChanID, err = c.BlockAPI.RegisterFinalizedChannel(esl.finalisedChan) if err != nil { diff --git a/dot/rpc/subscription/websocket_test.go b/dot/rpc/subscription/websocket_test.go index 97296f805b..d27e175964 100644 --- a/dot/rpc/subscription/websocket_test.go +++ b/dot/rpc/subscription/websocket_test.go @@ -288,6 +288,7 @@ func TestSubscribeAllHeads(t *testing.T) { require.Equal(t, []byte(`{"jsonrpc":"2.0","error":{"code":null,"message":"error BlockAPI not set"},"id":1}`+"\n"), msg) mockBlockAPI := new(mocks.MockBlockAPI) + mockBlockAPI.On("GetImportedBlockNotifierChannel").Return(nil, errors.New("some mocked error")).Once() wsconn.BlockAPI = mockBlockAPI diff --git a/dot/state/block.go b/dot/state/block.go index 8737ac078a..b2aeda9092 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -50,7 +50,7 @@ type BlockState struct { lastFinalised common.Hash // block notifiers - imported map[interface{}]chan<- *types.Block + imported map[chan *types.Block]struct{} finalised map[byte]chan<- *types.FinalisationInfo finalisedLock sync.RWMutex importedLock sync.RWMutex @@ -72,7 +72,7 @@ func NewBlockState(db chaindb.Database, bt *blocktree.BlockTree) (*BlockState, e dbPath: db.Path(), baseState: NewBaseState(db), db: chaindb.NewTable(db, blockPrefix), - imported: make(map[interface{}]chan<- *types.Block), + imported: make(map[chan *types.Block]struct{}), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), @@ -99,7 +99,7 @@ func NewBlockStateFromGenesis(db chaindb.Database, header *types.Header) (*Block bt: blocktree.NewBlockTreeFromRoot(header, db), baseState: NewBaseState(db), db: chaindb.NewTable(db, blockPrefix), - imported: make(map[interface{}]chan<- *types.Block), + imported: make(map[chan *types.Block]struct{}), finalised: make(map[byte]chan<- *types.FinalisationInfo), pruneKeyCh: make(chan *types.Header, pruneKeyBufferSize), runtimeUpdateSubscriptions: make(map[uint32]chan<- runtime.Version), diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index 5a13387779..09f4cd025f 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -30,19 +30,14 @@ import ( // DEFAULT_BUFFER_SIZE buffer size for channels const DEFAULT_BUFFER_SIZE = 100 -//type ImportNotifier struct { -// ch chan *types.Block -//} - // GetImportedBlockNotifierChannel function to retrieve a imported block notifier channel -func (bs *BlockState) GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) { +func (bs *BlockState) GetImportedBlockNotifierChannel() chan *types.Block { bs.importedLock.Lock() defer bs.importedLock.Unlock() ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) - bs.imported[conn] = ch - fmt.Printf("Added conn %v\n", conn) - return ch, nil + bs.imported[ch] = struct{}{} + return ch } //func (bs *BlockState) GetImportedBlockNotifierChannel() (*ImportNotifier, error) { @@ -77,11 +72,11 @@ func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo } // FreeImportedBlockNotifierChannel to free and close imported block notifier channel -func (bs *BlockState) FreeImportedBlockNotifierChannel(conn interface{}) { +func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { bs.importedLock.Lock() defer bs.importedLock.Unlock() - fmt.Printf("delete chan %v\n", conn) - delete(bs.imported, conn) + fmt.Printf("delete chan %v\n", ch) + delete(bs.imported, ch) } //func (bs *BlockState) FreeImportedBlockNotifierChannel(ch *ImportNotifier) { @@ -113,8 +108,8 @@ func (bs *BlockState) notifyImported(block *types.Block) { } logger.Trace("notifying imported block chans...", "chans", bs.imported) - for _, ch := range bs.imported { - go func(ch chan<- *types.Block) { + for ch := range bs.imported { + go func(ch chan *types.Block) { select { case ch <- block: default: diff --git a/dot/state/block_notify_test.go b/dot/state/block_notify_test.go index 8c9dce1383..b5c0f96c92 100644 --- a/dot/state/block_notify_test.go +++ b/dot/state/block_notify_test.go @@ -32,10 +32,9 @@ var testMessageTimeout = time.Second * 3 func TestImportChannel(t *testing.T) { bs := newTestBlockState(t, testGenesisHeader) - ch, err := bs.GetImportedBlockNotifierChannel(bs) - require.NoError(t, err) + ch := bs.GetImportedBlockNotifierChannel() - defer bs.FreeImportedBlockNotifierChannel(bs) + defer bs.FreeImportedBlockNotifierChannel(ch) AddBlocksToState(t, bs, 3) @@ -76,12 +75,10 @@ func TestImportChannel_Multi(t *testing.T) { bs := newTestBlockState(t, testGenesisHeader) num := 5 - chs := make([]<-chan *types.Block, num) + chs := make([]chan *types.Block, num) - var err error for i := 0; i < num; i++ { - chs[i], err = bs.GetImportedBlockNotifierChannel(i) - require.NoError(t, err) + chs[i] = bs.GetImportedBlockNotifierChannel() } var wg sync.WaitGroup diff --git a/lib/grandpa/grandpa.go b/lib/grandpa/grandpa.go index 80dd3911a9..f18dfba8c7 100644 --- a/lib/grandpa/grandpa.go +++ b/lib/grandpa/grandpa.go @@ -326,10 +326,7 @@ func (s *Service) initiateRound() error { s.precommits = new(sync.Map) s.pvEquivocations = make(map[ed25519.PublicKeyBytes][]*SignedVote) s.pcEquivocations = make(map[ed25519.PublicKeyBytes][]*SignedVote) - s.tracker, err = newTracker(s.blockState, s.messageHandler) - if err != nil { - return err - } + s.tracker = newTracker(s.blockState, s.messageHandler) s.tracker.start() logger.Trace("started message tracker") s.roundLock.Unlock() @@ -376,12 +373,9 @@ func (s *Service) initiate() error { } func (s *Service) waitForFirstBlock() error { - ch, err := s.blockState.GetImportedBlockNotifierChannel(s.blockState) - if err != nil { - return err - } + ch := s.blockState.GetImportedBlockNotifierChannel() - defer s.blockState.FreeImportedBlockNotifierChannel(s.blockState) + defer s.blockState.FreeImportedBlockNotifierChannel(ch) // loop until block 1 for { diff --git a/lib/grandpa/message_tracker.go b/lib/grandpa/message_tracker.go index a3f6fb0a8c..e66cac24db 100644 --- a/lib/grandpa/message_tracker.go +++ b/lib/grandpa/message_tracker.go @@ -32,15 +32,12 @@ type tracker struct { voteMessages map[common.Hash]map[ed25519.PublicKeyBytes]*networkVoteMessage // map of vote block hash -> array of VoteMessages for that hash commitMessages map[common.Hash]*CommitMessage // map of commit block hash to commit message mapLock sync.Mutex - in <-chan *types.Block // receive imported block from BlockState + in chan *types.Block // receive imported block from BlockState stopped chan struct{} } -func newTracker(bs BlockState, handler *MessageHandler) (*tracker, error) { - in, err := bs.GetImportedBlockNotifierChannel(bs) - if err != nil { - return nil, err - } +func newTracker(bs BlockState, handler *MessageHandler) *tracker { + in := bs.GetImportedBlockNotifierChannel() return &tracker{ blockState: bs, @@ -50,7 +47,7 @@ func newTracker(bs BlockState, handler *MessageHandler) (*tracker, error) { mapLock: sync.Mutex{}, in: in, stopped: make(chan struct{}), - }, nil + } } func (t *tracker) start() { @@ -59,7 +56,7 @@ func (t *tracker) start() { func (t *tracker) stop() { close(t.stopped) - t.blockState.FreeImportedBlockNotifierChannel(t.blockState) + t.blockState.FreeImportedBlockNotifierChannel(t.in) } func (t *tracker) addVote(v *networkVoteMessage) { diff --git a/lib/grandpa/message_tracker_test.go b/lib/grandpa/message_tracker_test.go index d1215db4c5..f183b82a97 100644 --- a/lib/grandpa/message_tracker_test.go +++ b/lib/grandpa/message_tracker_test.go @@ -35,8 +35,7 @@ func TestMessageTracker_ValidateMessage(t *testing.T) { gs, _, _, _ := setupGrandpa(t, kr.Bob().(*ed25519.Keypair)) state.AddBlocksToState(t, gs.blockState.(*state.BlockState), 3) - gs.tracker, err = newTracker(gs.blockState, gs.messageHandler) - require.NoError(t, err) + gs.tracker = newTracker(gs.blockState, gs.messageHandler) fake := &types.Header{ Number: big.NewInt(77), @@ -62,8 +61,7 @@ func TestMessageTracker_SendMessage(t *testing.T) { gs, in, _, _ := setupGrandpa(t, kr.Bob().(*ed25519.Keypair)) state.AddBlocksToState(t, gs.blockState.(*state.BlockState), 3) - gs.tracker, err = newTracker(gs.blockState, gs.messageHandler) - require.NoError(t, err) + gs.tracker = newTracker(gs.blockState, gs.messageHandler) gs.tracker.start() defer gs.tracker.stop() @@ -156,8 +154,7 @@ func TestMessageTracker_MapInsideMap(t *testing.T) { gs, _, _, _ := setupGrandpa(t, kr.Bob().(*ed25519.Keypair)) state.AddBlocksToState(t, gs.blockState.(*state.BlockState), 3) - gs.tracker, err = newTracker(gs.blockState, gs.messageHandler) - require.NoError(t, err) + gs.tracker = newTracker(gs.blockState, gs.messageHandler) header := &types.Header{ Number: big.NewInt(77), diff --git a/lib/grandpa/state.go b/lib/grandpa/state.go index c8579a70d4..ca8ecbf238 100644 --- a/lib/grandpa/state.go +++ b/lib/grandpa/state.go @@ -41,8 +41,8 @@ type BlockState interface { BestBlockHash() common.Hash Leaves() []common.Hash BlocktreeAsString() string - GetImportedBlockNotifierChannel(conn interface{}) (<-chan *types.Block, error) - FreeImportedBlockNotifierChannel(conn interface{}) + GetImportedBlockNotifierChannel() chan *types.Block + FreeImportedBlockNotifierChannel(ch chan *types.Block) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) UnregisterFinalisedChannel(id byte) SetJustification(hash common.Hash, data []byte) error diff --git a/lib/grandpa/vote_message_test.go b/lib/grandpa/vote_message_test.go index f9376b3e3b..b621438a29 100644 --- a/lib/grandpa/vote_message_test.go +++ b/lib/grandpa/vote_message_test.go @@ -337,8 +337,7 @@ func TestValidateMessage_BlockDoesNotExist(t *testing.T) { gs, err := NewService(cfg) require.NoError(t, err) state.AddBlocksToState(t, st.Block, 3) - gs.tracker, err = newTracker(st.Block, gs.messageHandler) - require.NoError(t, err) + gs.tracker = newTracker(st.Block, gs.messageHandler) fake := &types.Header{ Number: big.NewInt(77), @@ -371,8 +370,7 @@ func TestValidateMessage_IsNotDescendant(t *testing.T) { gs, err := NewService(cfg) require.NoError(t, err) - gs.tracker, err = newTracker(gs.blockState, gs.messageHandler) - require.NoError(t, err) + gs.tracker = newTracker(gs.blockState, gs.messageHandler) var branches []*types.Header for { From 1acfba744775499c90fc81d277e3553d814bd969 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Fri, 10 Sep 2021 16:25:42 -0400 Subject: [PATCH 23/27] GetImportedBlockNotifierChannel doesn't return error, fixed related test --- dot/rpc/modules/api_mocks.go | 2 +- dot/rpc/subscription/websocket_test.go | 12 ++---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/dot/rpc/modules/api_mocks.go b/dot/rpc/modules/api_mocks.go index 4af6fbfd97..b9f9802eaa 100644 --- a/dot/rpc/modules/api_mocks.go +++ b/dot/rpc/modules/api_mocks.go @@ -30,7 +30,7 @@ func NewMockBlockAPI() *modulesmocks.MockBlockAPI { m.On("GetBlockHash", mock.AnythingOfType("*big.Int")).Return(nil, nil) m.On("GetFinalisedHash", mock.AnythingOfType("uint64"), mock.AnythingOfType("uint64")).Return(common.Hash{}, nil) m.On("GetHighestFinalisedHash").Return(common.Hash{}, nil) - m.On("GetImportedBlockNotifierChannel").Return(make(chan *types.Block, 5), nil) + m.On("GetImportedBlockNotifierChannel").Return(make(chan *types.Block, 5)) m.On("FreeImportedBlockNotifierChannel", mock.AnythingOfType("chan *types.Block")) m.On("UnregisterImportedChannel", mock.AnythingOfType("uint8")) m.On("RegisterFinalizedChannel", mock.AnythingOfType("chan<- *types.FinalisationInfo")).Return(byte(0), nil) diff --git a/dot/rpc/subscription/websocket_test.go b/dot/rpc/subscription/websocket_test.go index d27e175964..a9f4bf207b 100644 --- a/dot/rpc/subscription/websocket_test.go +++ b/dot/rpc/subscription/websocket_test.go @@ -289,17 +289,9 @@ func TestSubscribeAllHeads(t *testing.T) { mockBlockAPI := new(mocks.MockBlockAPI) - mockBlockAPI.On("GetImportedBlockNotifierChannel").Return(nil, errors.New("some mocked error")).Once() - wsconn.BlockAPI = mockBlockAPI - _, err = wsconn.initAllBlocksListerner(1, nil) - require.Error(t, err, "could not register imported channel") - - _, msg, err = c.ReadMessage() - require.NoError(t, err) - require.Equal(t, []byte(`{"jsonrpc":"2.0","error":{"code":null,"message":"could not register imported channel"},"id":1}`+"\n"), msg) - mockBlockAPI.On("GetImportedBlockNotifierChannel").Return(make(chan *types.Block), nil).Once() + mockBlockAPI.On("GetImportedBlockNotifierChannel").Return(make(chan *types.Block)).Once() mockBlockAPI.On("RegisterFinalizedChannel", mock.AnythingOfType("chan<- *types.FinalisationInfo")). Return(uint8(0), errors.New("failed")).Once() @@ -311,7 +303,7 @@ func TestSubscribeAllHeads(t *testing.T) { var fCh chan<- *types.FinalisationInfo iCh := make(chan *types.Block) - mockBlockAPI.On("GetImportedBlockNotifierChannel").Return(iCh, nil).Once() + mockBlockAPI.On("GetImportedBlockNotifierChannel").Return(iCh).Once() mockBlockAPI.On("RegisterFinalizedChannel", mock.AnythingOfType("chan<- *types.FinalisationInfo")). Run(func(args mock.Arguments) { From 8ad895c7f8ac5ef08ba8e9749e91c2a258e1fe79 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Tue, 21 Sep 2021 14:29:07 -0400 Subject: [PATCH 24/27] remove un-needed comments --- dot/state/block_notify.go | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/dot/state/block_notify.go b/dot/state/block_notify.go index 4cb2cbc131..ce58e0304e 100644 --- a/dot/state/block_notify.go +++ b/dot/state/block_notify.go @@ -18,7 +18,6 @@ package state import ( "errors" - "fmt" "sync" "github.com/ChainSafe/gossamer/dot/types" @@ -40,19 +39,6 @@ func (bs *BlockState) GetImportedBlockNotifierChannel() chan *types.Block { return ch } -//func (bs *BlockState) GetImportedBlockNotifierChannel() (*ImportNotifier, error) { -// bs.importedLock.Lock() -// defer bs.importedLock.Unlock() -// -// in := &ImportNotifier{ -// ch: make(chan *types.Block, DEFAULT_BUFFER_SIZE), -// } -// //ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE) -// bs.imported[in] = struct{}{} -// -// return in, nil -//} - // RegisterFinalizedChannel registers a channel for block notification upon block finalisation. // It returns the channel ID (used for unregistering the channel) func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error) { @@ -71,21 +57,13 @@ func (bs *BlockState) RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo return id, nil } -// FreeImportedBlockNotifierChannel to free and close imported block notifier channel +// FreeImportedBlockNotifierChannel to free imported block notifier channel func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) { bs.importedLock.Lock() defer bs.importedLock.Unlock() - fmt.Printf("delete chan %v\n", ch) delete(bs.imported, ch) } -//func (bs *BlockState) FreeImportedBlockNotifierChannel(ch *ImportNotifier) { -// bs.importedLock.Lock() -// defer bs.importedLock.Unlock() -// // todo (ed) add test to confirm this is deleting -// delete(bs.imported, ch) -//} - // UnregisterFinalisedChannel removes the block finalisation notification channel with the given ID. // A channel must be unregistered before closing it. func (bs *BlockState) UnregisterFinalisedChannel(id byte) { From 6bfb77dc2c9d4eaa2e35b809cd78b932ace16aa8 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 23 Sep 2021 16:32:40 -0400 Subject: [PATCH 25/27] remove close for FinalisedChannel listener --- dot/rpc/subscription/listeners.go | 1 - 1 file changed, 1 deletion(-) diff --git a/dot/rpc/subscription/listeners.go b/dot/rpc/subscription/listeners.go index 2076006b8c..6299d7599c 100644 --- a/dot/rpc/subscription/listeners.go +++ b/dot/rpc/subscription/listeners.go @@ -191,7 +191,6 @@ func (l *BlockFinalizedListener) Listen() { defer func() { l.wsconn.BlockAPI.UnregisterFinalisedChannel(l.chanID) close(l.done) - close(l.channel) }() for { From 48f6db0aaf0aa6c7ac35a31c290f6f86ff4082d2 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 23 Sep 2021 16:52:03 -0400 Subject: [PATCH 26/27] added test for free imported channel --- dot/state/block_notify_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dot/state/block_notify_test.go b/dot/state/block_notify_test.go index b5c0f96c92..cd9bcafe6c 100644 --- a/dot/state/block_notify_test.go +++ b/dot/state/block_notify_test.go @@ -47,6 +47,15 @@ func TestImportChannel(t *testing.T) { } } +func TestFreeImportedBlockNotifierChannel(t *testing.T) { + bs := newTestBlockState(t, testGenesisHeader) + ch := bs.GetImportedBlockNotifierChannel() + require.Equal(t, 1, len(bs.imported)) + + bs.FreeImportedBlockNotifierChannel(ch) + require.Equal(t, 0, len(bs.imported)) +} + func TestFinalizedChannel(t *testing.T) { bs := newTestBlockState(t, testGenesisHeader) From 8b30110b48a8f2a0793ee167160c1cc621482865 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Fri, 24 Sep 2021 10:58:09 -0400 Subject: [PATCH 27/27] add mocks paths to .deepsource exclude_patterns --- .deepsource.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.deepsource.toml b/.deepsource.toml index 1c41320658..9053d32f39 100644 --- a/.deepsource.toml +++ b/.deepsource.toml @@ -11,7 +11,9 @@ exclude_patterns = [ "dot/config/**/*", "dot/rpc/modules/test_data", "lib/runtime/test_data", - "**/*_test.go" + "**/*_test.go", + "**/mocks/*", + "**/mock_*" ] [[analyzers]]