From 2444d0bc8ef82fa8c7d7fd9f0097ba71880e4651 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Mon, 15 Aug 2022 15:09:22 -0400 Subject: [PATCH] chore(lib/runtime): `GetRuntimeVersion` independent from existing runtime instance (#2687) - `GetRuntimeVersion` as function instead of instance method - No dependency on existing instance - Fix: do not modify parent instance allocator - Fix: close temporary instance on exit - Remove `CheckRuntimeVersion` from Instance interface - `ext_misc_runtime_version_version_1` uses `GetRuntimeVersion` --- dot/core/mocks_runtime_test.go | 15 -------------- dot/rpc/subscription/listeners_test.go | 3 +-- dot/state/block.go | 2 +- dot/sync/mock_instance_test.go | 15 -------------- lib/blocktree/mock_instance_test.go | 15 -------------- lib/runtime/interface.go | 1 - lib/runtime/mocks/instance.go | 23 ---------------------- lib/runtime/wasmer/imports.go | 17 ++-------------- lib/runtime/wasmer/instance.go | 27 +++++++++++++------------- lib/runtime/wasmer/instance_test.go | 17 +++++++++++++--- 10 files changed, 31 insertions(+), 104 deletions(-) diff --git a/dot/core/mocks_runtime_test.go b/dot/core/mocks_runtime_test.go index 510b87714d..bbbf50a449 100644 --- a/dot/core/mocks_runtime_test.go +++ b/dot/core/mocks_runtime_test.go @@ -80,21 +80,6 @@ func (mr *MockInstanceMockRecorder) CheckInherents() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckInherents", reflect.TypeOf((*MockInstance)(nil).CheckInherents)) } -// CheckRuntimeVersion mocks base method. -func (m *MockInstance) CheckRuntimeVersion(arg0 []byte) (runtime.Version, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CheckRuntimeVersion", arg0) - ret0, _ := ret[0].(runtime.Version) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// CheckRuntimeVersion indicates an expected call of CheckRuntimeVersion. -func (mr *MockInstanceMockRecorder) CheckRuntimeVersion(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckRuntimeVersion", reflect.TypeOf((*MockInstance)(nil).CheckRuntimeVersion), arg0) -} - // DecodeSessionKeys mocks base method. func (m *MockInstance) DecodeSessionKeys(arg0 []byte) ([]byte, error) { m.ctrl.T.Helper() diff --git a/dot/rpc/subscription/listeners_test.go b/dot/rpc/subscription/listeners_test.go index 6576216e91..d442feb1a2 100644 --- a/dot/rpc/subscription/listeners_test.go +++ b/dot/rpc/subscription/listeners_test.go @@ -351,12 +351,11 @@ func TestRuntimeChannelListener_Listen(t *testing.T) { expectedInitialResponse.Method = "state_runtimeVersion" expectedInitialResponse.Params.Result = expectedInitialVersion - instance := wasmer.NewTestInstance(t, runtime.NODE_RUNTIME) polkadotRuntimeFilepath, err := runtime.GetRuntime(context.Background(), runtime.POLKADOT_RUNTIME) require.NoError(t, err) code, err := os.ReadFile(polkadotRuntimeFilepath) require.NoError(t, err) - version, err := instance.CheckRuntimeVersion(code) + version, err := wasmer.GetRuntimeVersion(code) require.NoError(t, err) expectedUpdatedVersion := modules.StateRuntimeVersionResponse{ diff --git a/dot/state/block.go b/dot/state/block.go index e0409ae971..119368e73f 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -591,7 +591,7 @@ func (bs *BlockState) HandleRuntimeChanges(newState *rtstorage.TrieState, codeSubBlockHash := bs.baseState.LoadCodeSubstitutedBlockHash() if !codeSubBlockHash.Equal(common.Hash{}) { - newVersion, err := rt.CheckRuntimeVersion(code) + newVersion, err := wasmer.GetRuntimeVersion(code) if err != nil { return err } diff --git a/dot/sync/mock_instance_test.go b/dot/sync/mock_instance_test.go index ce3af4cb60..a526402635 100644 --- a/dot/sync/mock_instance_test.go +++ b/dot/sync/mock_instance_test.go @@ -80,21 +80,6 @@ func (mr *MockInstanceMockRecorder) CheckInherents() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckInherents", reflect.TypeOf((*MockInstance)(nil).CheckInherents)) } -// CheckRuntimeVersion mocks base method. -func (m *MockInstance) CheckRuntimeVersion(arg0 []byte) (runtime.Version, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CheckRuntimeVersion", arg0) - ret0, _ := ret[0].(runtime.Version) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// CheckRuntimeVersion indicates an expected call of CheckRuntimeVersion. -func (mr *MockInstanceMockRecorder) CheckRuntimeVersion(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckRuntimeVersion", reflect.TypeOf((*MockInstance)(nil).CheckRuntimeVersion), arg0) -} - // DecodeSessionKeys mocks base method. func (m *MockInstance) DecodeSessionKeys(arg0 []byte) ([]byte, error) { m.ctrl.T.Helper() diff --git a/lib/blocktree/mock_instance_test.go b/lib/blocktree/mock_instance_test.go index a6f99b5b1a..64c90b571e 100644 --- a/lib/blocktree/mock_instance_test.go +++ b/lib/blocktree/mock_instance_test.go @@ -80,21 +80,6 @@ func (mr *MockInstanceMockRecorder) CheckInherents() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckInherents", reflect.TypeOf((*MockInstance)(nil).CheckInherents)) } -// CheckRuntimeVersion mocks base method. -func (m *MockInstance) CheckRuntimeVersion(arg0 []byte) (runtime.Version, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CheckRuntimeVersion", arg0) - ret0, _ := ret[0].(runtime.Version) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// CheckRuntimeVersion indicates an expected call of CheckRuntimeVersion. -func (mr *MockInstanceMockRecorder) CheckRuntimeVersion(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckRuntimeVersion", reflect.TypeOf((*MockInstance)(nil).CheckRuntimeVersion), arg0) -} - // DecodeSessionKeys mocks base method. func (m *MockInstance) DecodeSessionKeys(arg0 []byte) ([]byte, error) { m.ctrl.T.Helper() diff --git a/lib/runtime/interface.go b/lib/runtime/interface.go index e34fda6954..30fc3a6e43 100644 --- a/lib/runtime/interface.go +++ b/lib/runtime/interface.go @@ -16,7 +16,6 @@ import ( // Instance is the interface a v0.8 runtime instance must implement type Instance interface { UpdateRuntimeCode([]byte) error - CheckRuntimeVersion([]byte) (Version, error) Stop() NodeStorage() NodeStorage NetworkService() BasicNetwork diff --git a/lib/runtime/mocks/instance.go b/lib/runtime/mocks/instance.go index b7e9ecc2d7..fe94c83705 100644 --- a/lib/runtime/mocks/instance.go +++ b/lib/runtime/mocks/instance.go @@ -71,29 +71,6 @@ func (_m *Instance) CheckInherents() { _m.Called() } -// CheckRuntimeVersion provides a mock function with given fields: _a0 -func (_m *Instance) CheckRuntimeVersion(_a0 []byte) (runtime.Version, error) { - ret := _m.Called(_a0) - - var r0 runtime.Version - if rf, ok := ret.Get(0).(func([]byte) runtime.Version); ok { - r0 = rf(_a0) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(runtime.Version) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func([]byte) error); ok { - r1 = rf(_a0) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // DecodeSessionKeys provides a mock function with given fields: enc func (_m *Instance) DecodeSessionKeys(enc []byte) ([]byte, error) { ret := _m.Called(enc) diff --git a/lib/runtime/wasmer/imports.go b/lib/runtime/wasmer/imports.go index 9972e488bc..4a41cf093e 100644 --- a/lib/runtime/wasmer/imports.go +++ b/lib/runtime/wasmer/imports.go @@ -110,7 +110,6 @@ import ( "time" "unsafe" - "github.com/ChainSafe/gossamer/internal/log" "github.com/ChainSafe/gossamer/lib/common" rtype "github.com/ChainSafe/gossamer/lib/common/types" "github.com/ChainSafe/gossamer/lib/crypto" @@ -118,7 +117,6 @@ import ( "github.com/ChainSafe/gossamer/lib/crypto/secp256k1" "github.com/ChainSafe/gossamer/lib/crypto/sr25519" "github.com/ChainSafe/gossamer/lib/runtime" - rtstorage "github.com/ChainSafe/gossamer/lib/runtime/storage" "github.com/ChainSafe/gossamer/lib/transaction" "github.com/ChainSafe/gossamer/lib/trie" "github.com/ChainSafe/gossamer/lib/trie/proof" @@ -944,20 +942,9 @@ func ext_misc_runtime_version_version_1(context unsafe.Pointer, dataSpan C.int64 logger.Trace("executing...") instanceContext := wasm.IntoInstanceContext(context) - data := asMemorySlice(instanceContext, dataSpan) - - cfg := runtime.InstanceConfig{ - LogLvl: log.DoNotChange, - Storage: rtstorage.NewTrieState(nil), - } - - instance, err := NewInstance(data, cfg) - if err != nil { - logger.Errorf("failed to create instance: %s", err) - return 0 - } + code := asMemorySlice(instanceContext, dataSpan) - version, err := instance.Version() + version, err := GetRuntimeVersion(code) if err != nil { logger.Errorf("failed to get runtime version: %s", err) out, _ := toWasmMemoryOptional(instanceContext, nil) diff --git a/lib/runtime/wasmer/instance.go b/lib/runtime/wasmer/instance.go index a4eb82b35e..4427e13d9d 100644 --- a/lib/runtime/wasmer/instance.go +++ b/lib/runtime/wasmer/instance.go @@ -156,25 +156,24 @@ func (in *Instance) UpdateRuntimeCode(code []byte) (err error) { return nil } -// CheckRuntimeVersion calculates runtime Version for runtime blob passed in -func (in *Instance) CheckRuntimeVersion(code []byte) (runtime.Version, error) { - in.mutex.Lock() - defer in.mutex.Unlock() - - wasmInstance, allocator, err := setupVM(code) +// GetRuntimeVersion finds the runtime version by initiating a temporary +// runtime instance using the WASM code provided, and querying it. +func GetRuntimeVersion(code []byte) (version runtime.Version, err error) { + config := runtime.InstanceConfig{ + LogLvl: log.DoNotChange, + } + instance, err := NewInstance(code, config) if err != nil { - return nil, fmt.Errorf("setting up VM: %w", err) + return version, fmt.Errorf("creating runtime instance: %w", err) } + defer instance.Stop() - in.ctx.Allocator = allocator // TODO we should no change the allocator of the parent instance - wasmInstance.SetContextData(in.ctx) - - instance := Instance{ - vm: wasmInstance, - ctx: in.ctx, + version, err = instance.Version() + if err != nil { + return nil, fmt.Errorf("running runtime: %w", err) } - return instance.Version() + return version, nil } var ( diff --git a/lib/runtime/wasmer/instance_test.go b/lib/runtime/wasmer/instance_test.go index 8f1a9b3c9b..48a53539a1 100644 --- a/lib/runtime/wasmer/instance_test.go +++ b/lib/runtime/wasmer/instance_test.go @@ -36,14 +36,13 @@ func TestPointerSize(t *testing.T) { require.Equal(t, in, res) } -func TestInstance_CheckRuntimeVersion(t *testing.T) { - instance := NewTestInstance(t, runtime.NODE_RUNTIME) +func Test_GetRuntimeVersion(t *testing.T) { polkadotRuntimeFilepath, err := runtime.GetRuntime( context.Background(), runtime.POLKADOT_RUNTIME) require.NoError(t, err) code, err := os.ReadFile(polkadotRuntimeFilepath) require.NoError(t, err) - version, err := instance.CheckRuntimeVersion(code) + version, err := GetRuntimeVersion(code) require.NoError(t, err) expected := runtime.NewVersionData( @@ -65,6 +64,18 @@ func TestInstance_CheckRuntimeVersion(t *testing.T) { require.Equal(t, expected.TransactionVersion(), version.TransactionVersion()) } +func Benchmark_GetRuntimeVersion(b *testing.B) { + polkadotRuntimeFilepath, err := runtime.GetRuntime( + context.Background(), runtime.POLKADOT_RUNTIME) + require.NoError(b, err) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + code, _ := os.ReadFile(polkadotRuntimeFilepath) + _, _ = GetRuntimeVersion(code) + } +} + func TestDecompressWasm(t *testing.T) { encoder, err := zstd.NewWriter(nil) require.NoError(t, err)