fix(rpc/subscription): subscribe runtime version notify when version changes (#1686)

* add code for runtime version changed fix

* fix return type

* update mock in Makefile

* fix core_api mock

* implement notifications for runtime updates

* lint

* implement unregister runtime version listener

* make mocks and lint

* refactor runtime subscription name, change error handeling

* remove unneeded select case

* lint, refactor GetID to GetChannelID

* lint

* add sync.WaitGroup to notify Runtime Updated

* add test for Register UnRegister Runtime Update Channel

* add check for id

* add register panic test

* add test to produce panic

* generate id as uuid, add locks, delete from map

* update runtime updated channel ids to use uint32

* change logging to debug

* remove comment

* move uuid into check loop

* update runtime test

* add test for notify runtime updated

* update runtime tests

* move runtime notify from coreAPI to blockStateAPI

* fix mocks for tests

* refactor state_test

* add tests

* lint

* remove commented code, fix var assignment

* fix merge conflict

* add check if channel is ok

* update unit test

* send notification as go routine

Co-authored-by: Eclésio Júnior <[email protected]>

Author: edwardmack

dot/core/service_test.go

@@ -25,6 +25,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/ChainSafe/gossamer/dot/core/mocks"
+	coremocks "github.com/ChainSafe/gossamer/dot/core/mocks"
 	"github.com/ChainSafe/gossamer/dot/network"
 	"github.com/ChainSafe/gossamer/dot/state"
 	"github.com/ChainSafe/gossamer/dot/sync"
@@ -33,6 +35,7 @@ import (
 	"github.com/ChainSafe/gossamer/lib/keystore"
 	"github.com/ChainSafe/gossamer/lib/runtime"
 	"github.com/ChainSafe/gossamer/lib/runtime/extrinsic"
+	runtimemocks "github.com/ChainSafe/gossamer/lib/runtime/mocks"
 	"github.com/ChainSafe/gossamer/lib/runtime/storage"
 	"github.com/ChainSafe/gossamer/lib/runtime/wasmer"
 	"github.com/ChainSafe/gossamer/lib/transaction"
@@ -41,10 +44,6 @@ import (
 	log "github.com/ChainSafe/log15"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
-
-	"github.com/ChainSafe/gossamer/dot/core/mocks"
-	coremocks "github.com/ChainSafe/gossamer/dot/core/mocks"
-	runtimemocks "github.com/ChainSafe/gossamer/lib/runtime/mocks"
 )
 
 func addTestBlocksToState(t *testing.T, depth int, blockState BlockState) {

dot/rpc/modules/api.go

@@ -40,6 +40,8 @@ type BlockAPI interface {
 	RegisterFinalizedChannel(ch chan<- *types.FinalisationInfo) (byte, error)
 	UnregisterFinalisedChannel(id byte)
 	SubChain(start, end common.Hash) ([]common.Hash, error)
+	RegisterRuntimeUpdatedChannel(ch chan<- runtime.Version) (uint32, error)
+	UnregisterRuntimeUpdatedChannel(id uint32) bool
 }
 
 // NetworkAPI interface for network state methods

dot/rpc/modules/api_mocks.go

@@ -3,6 +3,7 @@ package modules
 import (
 	modulesmocks "github.com/ChainSafe/gossamer/dot/rpc/modules/mocks"
 	"github.com/ChainSafe/gossamer/lib/common"
+	runtimemocks "github.com/ChainSafe/gossamer/lib/runtime/mocks"
 	"github.com/stretchr/testify/mock"
 )
 
@@ -35,6 +36,8 @@ func NewMockBlockAPI() *modulesmocks.BlockAPI {
 	m.On("GetJustification", mock.AnythingOfType("common.Hash")).Return(make([]byte, 10), nil)
 	m.On("HasJustification", mock.AnythingOfType("common.Hash")).Return(true, nil)
 	m.On("SubChain", mock.AnythingOfType("common.Hash"), mock.AnythingOfType("common.Hash")).Return(make([]common.Hash, 0), nil)
+	m.On("RegisterRuntimeUpdatedChannel", mock.AnythingOfType("chan<- runtime.Version")).Return(uint32(0), nil)
+
 	return m
 }
 
@@ -43,9 +46,22 @@ func NewMockCoreAPI() *modulesmocks.MockCoreAPI {
 	m := new(modulesmocks.MockCoreAPI)
 	m.On("InsertKey", mock.AnythingOfType("crypto.Keypair"))
 	m.On("HasKey", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(false, nil)
-	m.On("GetRuntimeVersion", mock.AnythingOfType("*common.Hash")).Return(nil, nil)
+	m.On("GetRuntimeVersion", mock.AnythingOfType("*common.Hash")).Return(NewMockVersion(), nil)
 	m.On("IsBlockProducer").Return(false)
 	m.On("HandleSubmittedExtrinsic", mock.AnythingOfType("types.Extrinsic")).Return(nil)
 	m.On("GetMetadata", mock.AnythingOfType("*common.Hash")).Return(nil, nil)
 	return m
 }
+
+// NewMockVersion creates and returns an runtime Version interface mock
+func NewMockVersion() *runtimemocks.MockVersion {
+	m := new(runtimemocks.MockVersion)
+	m.On("SpecName").Return([]byte(`mock-spec`))
+	m.On("ImplName").Return(nil)
+	m.On("AuthoringVersion").Return(uint32(0))
+	m.On("SpecVersion").Return(uint32(0))
+	m.On("ImplVersion").Return(uint32(0))
+	m.On("TransactionVersion").Return(uint32(0))
+	m.On("APIItems").Return(nil)
+	return m
+}

dot/rpc/modules/api_mocks_test.go

@@ -0,0 +1,43 @@
+// Copyright 2019 ChainSafe Systems (ON) Corp.
+// This file is part of gossamer.
+//
+// The gossamer library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The gossamer library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the gossamer library. If not, see <http://www.gnu.org/licenses/>.
+
+package modules
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewMockStorageAPI(t *testing.T) {
+	m := NewMockStorageAPI()
+	require.NotNil(t, m)
+}
+
+func TestNewMockBlockAPI(t *testing.T) {
+	m := NewMockBlockAPI()
+	require.NotNil(t, m)
+}
+
+func TestNewMockCoreAPI(t *testing.T) {
+	m := NewMockCoreAPI()
+	require.NotNil(t, m)
+}
+
+func TestNewMockVersion(t *testing.T) {
+	m := NewMockVersion()
+	require.NotNil(t, m)
+}

dot/rpc/modules/mocks/BlockAPI.go

@@ -26,6 +26,7 @@ import (
 	"github.com/ChainSafe/gossamer/dot/state"
 	"github.com/ChainSafe/gossamer/dot/types"
 	"github.com/ChainSafe/gossamer/lib/common"
+	"github.com/ChainSafe/gossamer/lib/runtime"
 )
 
 const (
@@ -282,20 +283,26 @@ func (l *ExtrinsicSubmitListener) Stop() error {
 
 // RuntimeVersionListener to handle listening for Runtime Version
 type RuntimeVersionListener struct {
-	wsconn *WSConn
-	subID  uint32
+	wsconn        WSConnAPI
+	subID         uint32
+	runtimeUpdate chan runtime.Version
+	channelID     uint32
+	coreAPI       modules.CoreAPI
+}
+
+// VersionListener interface defining methods that version listener must implement
+type VersionListener interface {
+	GetChannelID() uint32
 }
 
 // Listen implementation of Listen interface to listen for runtime version changes
 func (l *RuntimeVersionListener) Listen() {
 	// This sends current runtime version once when subscription is created
-	// TODO (ed) add logic to send updates when runtime version changes
-	rtVersion, err := l.wsconn.CoreAPI.GetRuntimeVersion(nil)
+	rtVersion, err := l.coreAPI.GetRuntimeVersion(nil)
 	if err != nil {
 		return
 	}
 	ver := modules.StateRuntimeVersionResponse{}
-
 	ver.SpecName = string(rtVersion.SpecName())
 	ver.ImplName = string(rtVersion.ImplName())
 	ver.AuthoringVersion = rtVersion.AuthoringVersion()
@@ -304,7 +311,34 @@ func (l *RuntimeVersionListener) Listen() {
 	ver.TransactionVersion = rtVersion.TransactionVersion()
 	ver.Apis = modules.ConvertAPIs(rtVersion.APIItems())
 
-	l.wsconn.safeSend(newSubscriptionResponse(stateRuntimeVersionMethod, l.subID, ver))
+	go l.wsconn.safeSend(newSubscriptionResponse(stateRuntimeVersionMethod, l.subID, ver))
+
+	// listen for runtime updates
+	go func() {
+		for {
+			info, ok := <-l.runtimeUpdate
+			if !ok {
+				return
+			}
+
+			ver := modules.StateRuntimeVersionResponse{}
+
+			ver.SpecName = string(info.SpecName())
+			ver.ImplName = string(info.ImplName())
+			ver.AuthoringVersion = info.AuthoringVersion()
+			ver.SpecVersion = info.SpecVersion()
+			ver.ImplVersion = info.ImplVersion()
+			ver.TransactionVersion = info.TransactionVersion()
+			ver.Apis = modules.ConvertAPIs(info.APIItems())
+
+			l.wsconn.safeSend(newSubscriptionResponse(stateRuntimeVersionMethod, l.subID, ver))
+		}
+	}()
+}
+
+// GetChannelID function that returns listener's channel ID
+func (l *RuntimeVersionListener) GetChannelID() uint32 {
+	return l.channelID
 }
 
 // Stop to runtimeVersionListener not implemented yet because the listener

dot/rpc/subscription/listeners.go

@@ -19,10 +19,12 @@ package subscription
 import (
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"log"
 	"math/big"
 	"net/http"
 	"net/http/httptest"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -33,6 +35,8 @@ import (
 	"github.com/ChainSafe/gossamer/dot/types"
 	"github.com/ChainSafe/gossamer/lib/common"
 	"github.com/ChainSafe/gossamer/lib/grandpa"
+	"github.com/ChainSafe/gossamer/lib/runtime"
+	"github.com/ChainSafe/gossamer/lib/runtime/wasmer"
 	"github.com/gorilla/websocket"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
@@ -325,3 +329,58 @@ func setupWSConn(t *testing.T) (*WSConn, *websocket.Conn, func()) {
 
 	return wskt, ws, cancel
 }
+
+func TestRuntimeChannelListener_Listen(t *testing.T) {
+	notifyChan := make(chan runtime.Version)
+	mockConnection := &MockWSConnAPI{}
+	rvl := RuntimeVersionListener{
+		wsconn:        mockConnection,
+		subID:         0,
+		runtimeUpdate: notifyChan,
+		coreAPI:       modules.NewMockCoreAPI(),
+	}
+
+	expectedInitialVersion := modules.StateRuntimeVersionResponse{
+		SpecName: "mock-spec",
+		Apis:     modules.ConvertAPIs(nil),
+	}
+
+	expectedInitialResponse := newSubcriptionBaseResponseJSON()
+	expectedInitialResponse.Method = "state_runtimeVersion"
+	expectedInitialResponse.Params.Result = expectedInitialVersion
+
+	instance := wasmer.NewTestInstance(t, runtime.NODE_RUNTIME)
+	_, err := runtime.GetRuntimeBlob(runtime.POLKADOT_RUNTIME_FP, runtime.POLKADOT_RUNTIME_URL)
+	require.NoError(t, err)
+	fp, err := filepath.Abs(runtime.POLKADOT_RUNTIME_FP)
+	require.NoError(t, err)
+	code, err := ioutil.ReadFile(fp)
+	require.NoError(t, err)
+	version, err := instance.CheckRuntimeVersion(code)
+	require.NoError(t, err)
+
+	expectedUpdatedVersion := modules.StateRuntimeVersionResponse{
+		SpecName:           "polkadot",
+		ImplName:           "parity-polkadot",
+		AuthoringVersion:   0,
+		SpecVersion:        25,
+		ImplVersion:        0,
+		TransactionVersion: 5,
+		Apis:               modules.ConvertAPIs(version.APIItems()),
+	}
+
+	expectedUpdateResponse := newSubcriptionBaseResponseJSON()
+	expectedUpdateResponse.Method = "state_runtimeVersion"
+	expectedUpdateResponse.Params.Result = expectedUpdatedVersion
+
+	go rvl.Listen()
+
+	//check initial response
+	time.Sleep(time.Millisecond * 10)
+	require.Equal(t, expectedInitialResponse, mockConnection.lastMessage)
+
+	// check response after update
+	notifyChan <- version
+	time.Sleep(time.Millisecond * 10)
+	require.Equal(t, expectedUpdateResponse, mockConnection.lastMessage)
+}

dot/rpc/subscription/listeners_test.go

@@ -47,6 +47,8 @@ func (c *WSConn) getUnsubListener(method string, params interface{}) (unsubListe
 	switch method {
 	case "state_unsubscribeStorage":
 		unsub = c.unsubscribeStorageListener
+	case "state_unsubscribeRuntimeVersion":
+		unsub = c.unsubscribeRuntimeVersionListener
 	case "grandpa_unsubscribeJustifications":
 		unsub = c.unsubscribeGrandpaJustificationListener
 	default: