From 3fd63dbc8aeb9db41b0103580372aa6c84e63499 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Wed, 24 Aug 2022 17:17:27 -0400 Subject: [PATCH] feat(wasmer): get and cache state version in instance context (#2747) - Codec support for Core_version and state_version - Cache core version in runtime instance context - Set when creating instance - Update when updating runtime code - Handle `Core_version` call not present in host API test runtimes --- dot/core/messages_integration_test.go | 3 +- dot/core/mocks_runtime_test.go | 5 ++-- dot/core/service.go | 2 +- dot/core/service_integration_test.go | 15 ++++------ dot/state/block.go | 7 ++--- dot/sync/mock_instance_test.go | 5 ++-- lib/babe/build_integration_test.go | 3 +- lib/blocktree/mock_instance_test.go | 5 ++-- lib/runtime/interface.go | 4 ++- lib/runtime/mocks/instance.go | 11 ++------ lib/runtime/test_helpers.go | 2 +- lib/runtime/types.go | 1 + lib/runtime/version.go | 2 ++ lib/runtime/version_test.go | 40 ++++++++++++++++++++++++++- lib/runtime/wasmer/config.go | 1 + lib/runtime/wasmer/exports.go | 12 ++++++-- lib/runtime/wasmer/exports_test.go | 5 ++-- lib/runtime/wasmer/instance.go | 30 ++++++++++++++++++-- lib/runtime/wasmer/test_helpers.go | 17 +++++++++--- 19 files changed, 117 insertions(+), 53 deletions(-) diff --git a/dot/core/messages_integration_test.go b/dot/core/messages_integration_test.go index a0f2775c8e..bc7644357c 100644 --- a/dot/core/messages_integration_test.go +++ b/dot/core/messages_integration_test.go @@ -40,8 +40,7 @@ func createExtrinsic(t *testing.T, rt RuntimeInstance, genHash common.Hash, nonc err = ctypes.Decode(decoded, meta) require.NoError(t, err) - rv, err := rt.Version() - require.NoError(t, err) + rv := rt.Version() c, err := ctypes.NewCall(meta, "System.remark", []byte{0xab, 0xcd}) require.NoError(t, err) diff --git a/dot/core/mocks_runtime_test.go b/dot/core/mocks_runtime_test.go index bbbf50a449..755735f99b 100644 --- a/dot/core/mocks_runtime_test.go +++ b/dot/core/mocks_runtime_test.go @@ -374,12 +374,11 @@ func (mr *MockInstanceMockRecorder) Validator() *gomock.Call { } // Version mocks base method. -func (m *MockInstance) Version() (runtime.Version, error) { +func (m *MockInstance) Version() runtime.Version { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Version") ret0, _ := ret[0].(runtime.Version) - ret1, _ := ret[1].(error) - return ret0, ret1 + return ret0 } // Version indicates an expected call of Version. diff --git a/dot/core/service.go b/dot/core/service.go index 641b5f8dc6..23a5b12a93 100644 --- a/dot/core/service.go +++ b/dot/core/service.go @@ -478,7 +478,7 @@ func (s *Service) GetRuntimeVersion(bhash *common.Hash) ( } rt.SetContextStorage(ts) - return rt.Version() + return rt.Version(), nil } // HandleSubmittedExtrinsic is used to send a Transaction message containing a Extrinsic @ext diff --git a/dot/core/service_integration_test.go b/dot/core/service_integration_test.go index 206af16751..e7f30a171a 100644 --- a/dot/core/service_integration_test.go +++ b/dot/core/service_integration_test.go @@ -522,8 +522,7 @@ func TestService_GetRuntimeVersion(t *testing.T) { rt, err := s.blockState.GetRuntime(nil) require.NoError(t, err) - rtExpected, err := rt.Version() - require.NoError(t, err) + rtExpected := rt.Version() rtv, err := s.GetRuntimeVersion(nil) require.NoError(t, err) @@ -577,8 +576,7 @@ func TestService_HandleRuntimeChanges(t *testing.T) { rt, err := s.blockState.GetRuntime(nil) require.NoError(t, err) - v, err := rt.Version() - require.NoError(t, err) + v := rt.Version() currSpecVersion := v.SpecVersion // genesis runtime version. hash := s.blockState.BestBlockHash() // genesisHash @@ -613,8 +611,7 @@ func TestService_HandleRuntimeChanges(t *testing.T) { parentRt, err := s.blockState.GetRuntime(&hash) require.NoError(t, err) - v, err = parentRt.Version() - require.NoError(t, err) + v = parentRt.Version() require.Equal(t, v.SpecVersion, currSpecVersion) bhash1 := newBlock1.Header.Hash() @@ -635,15 +632,13 @@ func TestService_HandleRuntimeChanges(t *testing.T) { rt, err = s.blockState.GetRuntime(&bhash1) require.NoError(t, err) - v, err = rt.Version() - require.NoError(t, err) + v = rt.Version() require.Equal(t, v.SpecVersion, currSpecVersion) rt, err = s.blockState.GetRuntime(&rtUpdateBhash) require.NoError(t, err) - v, err = rt.Version() - require.NoError(t, err) + v = rt.Version() require.Equal(t, v.SpecVersion, updatedSpecVersion) } diff --git a/dot/state/block.go b/dot/state/block.go index a76dc015fb..0135e6bdd7 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -597,7 +597,7 @@ func (bs *BlockState) HandleRuntimeChanges(newState *rtstorage.TrieState, } // only update runtime during code substitution if runtime SpecVersion is updated - previousVersion, _ := rt.Version() + previousVersion := rt.Version() if previousVersion.SpecVersion == newVersion.SpecVersion { logger.Info("not upgrading runtime code during code substitution") bs.StoreRuntime(bHash, rt) @@ -633,10 +633,7 @@ func (bs *BlockState) HandleRuntimeChanges(newState *rtstorage.TrieState, return fmt.Errorf("failed to update code substituted block hash: %w", err) } - newVersion, err := rt.Version() - if err != nil { - return fmt.Errorf("failed to retrieve runtime version: %w", err) - } + newVersion := rt.Version() go bs.notifyRuntimeUpdated(newVersion) return nil } diff --git a/dot/sync/mock_instance_test.go b/dot/sync/mock_instance_test.go index a526402635..893be0faae 100644 --- a/dot/sync/mock_instance_test.go +++ b/dot/sync/mock_instance_test.go @@ -374,12 +374,11 @@ func (mr *MockInstanceMockRecorder) Validator() *gomock.Call { } // Version mocks base method. -func (m *MockInstance) Version() (runtime.Version, error) { +func (m *MockInstance) Version() runtime.Version { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Version") ret0, _ := ret[0].(runtime.Version) - ret1, _ := ret[1].(error) - return ret0, ret1 + return ret0 } // Version indicates an expected call of Version. diff --git a/lib/babe/build_integration_test.go b/lib/babe/build_integration_test.go index 03985ef041..527c08765f 100644 --- a/lib/babe/build_integration_test.go +++ b/lib/babe/build_integration_test.go @@ -273,8 +273,7 @@ func TestBuildAndApplyExtrinsic(t *testing.T) { err = ctypes.Decode(decoded, meta) require.NoError(t, err) - rv, err := rt.Version() - require.NoError(t, err) + rv := rt.Version() bob, err := ctypes.NewMultiAddressFromHexAccountID( "0x90b5ab205c6974c9ea841be688864633dc9ca8a357843eeacf2314649965fe22") diff --git a/lib/blocktree/mock_instance_test.go b/lib/blocktree/mock_instance_test.go index 64c90b571e..f187bf1583 100644 --- a/lib/blocktree/mock_instance_test.go +++ b/lib/blocktree/mock_instance_test.go @@ -374,12 +374,11 @@ func (mr *MockInstanceMockRecorder) Validator() *gomock.Call { } // Version mocks base method. -func (m *MockInstance) Version() (runtime.Version, error) { +func (m *MockInstance) Version() runtime.Version { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Version") ret0, _ := ret[0].(runtime.Version) - ret1, _ := ret[1].(error) - return ret0, ret1 + return ret0 } // Version indicates an expected call of Version. diff --git a/lib/runtime/interface.go b/lib/runtime/interface.go index 30fc3a6e43..6fc9f3a01a 100644 --- a/lib/runtime/interface.go +++ b/lib/runtime/interface.go @@ -25,7 +25,9 @@ type Instance interface { SetContextStorage(s Storage) // used to set the TrieState before a runtime call GetCodeHash() common.Hash - Version() (Version, error) + // Version returns the version from the runtime. + // This should return the cached version and be cheap to execute. + Version() (version Version) Metadata() ([]byte, error) BabeConfiguration() (*types.BabeConfiguration, error) GrandpaAuthorities() ([]types.Authority, error) diff --git a/lib/runtime/mocks/instance.go b/lib/runtime/mocks/instance.go index 995407b07e..e6bfb07ac6 100644 --- a/lib/runtime/mocks/instance.go +++ b/lib/runtime/mocks/instance.go @@ -408,7 +408,7 @@ func (_m *Instance) Validator() bool { } // Version provides a mock function with given fields: -func (_m *Instance) Version() (runtime.Version, error) { +func (_m *Instance) Version() runtime.Version { ret := _m.Called() var r0 runtime.Version @@ -418,14 +418,7 @@ func (_m *Instance) Version() (runtime.Version, error) { r0 = ret.Get(0).(runtime.Version) } - var r1 error - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return r0 } type mockConstructorTestingTNewInstance interface { diff --git a/lib/runtime/test_helpers.go b/lib/runtime/test_helpers.go index f57efb6c03..e3487f8920 100644 --- a/lib/runtime/test_helpers.go +++ b/lib/runtime/test_helpers.go @@ -211,7 +211,7 @@ func NewTestExtrinsic(t *testing.T, rt Instance, genHash, blockHash common.Hash, err = ctypes.Decode(decoded, meta) require.NoError(t, err) - rv, err := rt.Version() + rv := rt.Version() require.NoError(t, err) c, err := ctypes.NewCall(meta, call, args...) diff --git a/lib/runtime/types.go b/lib/runtime/types.go index 8701814086..95ad9f3309 100644 --- a/lib/runtime/types.go +++ b/lib/runtime/types.go @@ -56,6 +56,7 @@ type Context struct { Transaction TransactionState SigVerifier *crypto.SignatureVerifier OffchainHTTPSet *offchain.HTTPSet + Version Version } // NewValidateTransactionError returns an error based on a return value from TaggedTransactionQueueValidateTransaction diff --git a/lib/runtime/version.go b/lib/runtime/version.go index 23f377cde0..ede7de847f 100644 --- a/lib/runtime/version.go +++ b/lib/runtime/version.go @@ -26,6 +26,7 @@ type Version struct { ImplVersion uint32 APIItems []APIItem TransactionVersion uint32 + StateVersion uint32 } var ( @@ -63,6 +64,7 @@ func DecodeVersion(encoded []byte) (version Version, err error) { optionalFields := [...]namedValue{ {name: "transaction version", value: &version.TransactionVersion}, + {name: "state version", value: &version.StateVersion}, } for _, optionalField := range optionalFields { err = decoder.Decode(optionalField.value) diff --git a/lib/runtime/version_test.go b/lib/runtime/version_test.go index 412941995e..138046f3b4 100644 --- a/lib/runtime/version_test.go +++ b/lib/runtime/version_test.go @@ -57,6 +57,23 @@ func Test_DecodeVersion(t *testing.T) { // errWrapped: ErrDecoding, // errMessage: "decoding transaction version: could not decode invalid integer", // }, + // TODO add state version decode error once + // https://github.com/ChainSafe/gossamer/pull/2683 + // is merged. + // "state version decode error": { + // encoded: concatBytes([][]byte{ + // scaleEncode(t, []byte("a")), // spec name + // scaleEncode(t, []byte("b")), // impl name + // scaleEncode(t, uint32(1)), // authoring version + // scaleEncode(t, uint32(2)), // spec version + // scaleEncode(t, uint32(3)), // impl version + // scaleEncode(t, []APIItem{{}}), // api items + // scaleEncode(t, uint32(4)), // transaction version + // {1, 2, 3}, // state version + // }), + // errWrapped: ErrDecoding, + // errMessage: "decoding state version: could not decode invalid integer", + // }, "no optional field set": { encoded: []byte{ 0x4, 0x1, 0x4, 0x2, 0x3, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0, @@ -92,6 +109,25 @@ func Test_DecodeVersion(t *testing.T) { TransactionVersion: 7, }, }, + "transaction and state versions set": { + encoded: []byte{ + 0x4, 0x1, 0x4, 0x2, 0x3, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0, + 0x5, 0x0, 0x0, 0x0, 0x4, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, + 0x8, 0x6, 0x0, 0x0, 0x0, 0x7, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0}, + version: Version{ + SpecName: []byte{1}, + ImplName: []byte{2}, + AuthoringVersion: 3, + SpecVersion: 4, + ImplVersion: 5, + APIItems: []APIItem{{ + Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8}, + Ver: 6, + }}, + TransactionVersion: 7, + StateVersion: 4, + }, + }, } for name, testCase := range testCases { @@ -130,11 +166,12 @@ func Test_Version_Scale(t *testing.T) { Ver: 6, }}, TransactionVersion: 7, + StateVersion: 4, }, encoding: []byte{ 0x4, 0x1, 0x4, 0x2, 0x3, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0, 0x5, 0x0, 0x0, 0x0, 0x4, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, - 0x8, 0x6, 0x0, 0x0, 0x0, 0x7, 0x0, 0x0, 0x0}, + 0x8, 0x6, 0x0, 0x0, 0x0, 0x7, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0}, decoded: Version{ SpecName: []byte{1}, ImplName: []byte{2}, @@ -146,6 +183,7 @@ func Test_Version_Scale(t *testing.T) { Ver: 6, }}, TransactionVersion: 7, + StateVersion: 4, }, }, } diff --git a/lib/runtime/wasmer/config.go b/lib/runtime/wasmer/config.go index 0730bbdd1c..20dc703e1e 100644 --- a/lib/runtime/wasmer/config.go +++ b/lib/runtime/wasmer/config.go @@ -21,4 +21,5 @@ type Config struct { Network runtime.BasicNetwork Transaction runtime.TransactionState CodeHash common.Hash + testVersion *runtime.Version } diff --git a/lib/runtime/wasmer/exports.go b/lib/runtime/wasmer/exports.go index ebb9bc3270..c9947fecab 100644 --- a/lib/runtime/wasmer/exports.go +++ b/lib/runtime/wasmer/exports.go @@ -30,8 +30,16 @@ func (in *Instance) ValidateTransaction(e types.Extrinsic) (*transaction.Validit return v, err } -// Version calls runtime function Core_Version -func (in *Instance) Version() (version runtime.Version, err error) { +// Version returns the instance version. +// This is cheap to call since the instance version is cached. +// Note the instance version is set at creation and on code update. +func (in *Instance) Version() (version runtime.Version) { + return in.ctx.Version +} + +// version calls runtime function Core_Version and returns the +// decoded version structure. +func (in *Instance) version() (version runtime.Version, err error) { res, err := in.Exec(runtime.CoreVersion, []byte{}) if err != nil { return version, err diff --git a/lib/runtime/wasmer/exports_test.go b/lib/runtime/wasmer/exports_test.go index 2df47b7be0..8785b8d012 100644 --- a/lib/runtime/wasmer/exports_test.go +++ b/lib/runtime/wasmer/exports_test.go @@ -30,7 +30,7 @@ func Test_Instance_Version(t *testing.T) { t.Parallel() type InstanceVersion interface { - Version() (runtime.Version, error) + Version() (version runtime.Version) } testCases := map[string]struct { @@ -280,8 +280,7 @@ func Test_Instance_Version(t *testing.T) { t.Parallel() instance := testCase.instanceBuilder(t) - version, err := instance.Version() - require.NoError(t, err) + version := instance.Version() assert.Equal(t, testCase.expectedVersion, version) }) } diff --git a/lib/runtime/wasmer/instance.go b/lib/runtime/wasmer/instance.go index 60b779be2e..0bcf13c3d0 100644 --- a/lib/runtime/wasmer/instance.go +++ b/lib/runtime/wasmer/instance.go @@ -103,11 +103,25 @@ func NewInstance(code []byte, cfg Config) (instance *Instance, err error) { } wasmInstance.SetContextData(runtimeCtx) - return &Instance{ + instance = &Instance{ vm: wasmInstance, ctx: runtimeCtx, codeHash: cfg.CodeHash, - }, nil + } + + if cfg.testVersion != nil { + instance.ctx.Version = *cfg.testVersion + } else { + instance.ctx.Version, err = instance.version() + if err != nil { + instance.close() + return nil, fmt.Errorf("getting instance version: %w", err) + } + } + + wasmInstance.SetContextData(instance.ctx) + + return instance, nil } // decompressWasm decompresses a Wasm blob that may or may not be compressed with zstd @@ -153,6 +167,16 @@ func (in *Instance) UpdateRuntimeCode(code []byte) (err error) { in.vm = wasmInstance + // Find runtime instance version and cache it in its + // instance context. + version, err := in.version() + if err != nil { + in.close() + return fmt.Errorf("getting instance version: %w", err) + } + in.ctx.Version = version + wasmInstance.SetContextData(in.ctx) + return nil } @@ -168,7 +192,7 @@ func GetRuntimeVersion(code []byte) (version runtime.Version, err error) { } defer instance.Stop() - version, err = instance.Version() + version, err = instance.version() if err != nil { return version, fmt.Errorf("running runtime: %w", err) } diff --git a/lib/runtime/wasmer/test_helpers.go b/lib/runtime/wasmer/test_helpers.go index 5a894bcc4c..a3c8edb62e 100644 --- a/lib/runtime/wasmer/test_helpers.go +++ b/lib/runtime/wasmer/test_helpers.go @@ -31,17 +31,17 @@ func NewTestInstance(t *testing.T, targetRuntime string) *Instance { func NewTestInstanceWithTrie(t *testing.T, targetRuntime string, tt *trie.Trie) *Instance { t.Helper() - cfg := setupConfig(t, tt, DefaultTestLogLvl, common.NoNetworkRole) + cfg := setupConfig(t, tt, DefaultTestLogLvl, common.NoNetworkRole, targetRuntime) runtimeFilepath, err := runtime.GetRuntime(context.Background(), targetRuntime) require.NoError(t, err) r, err := NewInstanceFromFile(runtimeFilepath, cfg) - require.NoError(t, err, "Got error when trying to create new VM", "targetRuntime", targetRuntime) - require.NotNil(t, r, "Could not create new VM instance", "targetRuntime", targetRuntime) + require.NoError(t, err) return r } -func setupConfig(t *testing.T, tt *trie.Trie, lvl log.Level, role common.Roles) Config { +func setupConfig(t *testing.T, tt *trie.Trie, lvl log.Level, + role common.Roles, targetRuntime string) Config { t.Helper() s := storage.NewTrieState(tt) @@ -52,6 +52,14 @@ func setupConfig(t *testing.T, tt *trie.Trie, lvl log.Level, role common.Roles) BaseDB: runtime.NewInMemoryDB(t), // we're using a local storage here since this is a test runtime } + version := (*runtime.Version)(nil) + if targetRuntime == runtime.HOST_API_TEST_RUNTIME { + // Force state version to 0 since the host api test runtime + // does not implement the Core_version call so we cannot get the + // state version from it. + version = &runtime.Version{} + } + return Config{ Storage: s, Keystore: keystore.NewGlobalKeystore(), @@ -60,6 +68,7 @@ func setupConfig(t *testing.T, tt *trie.Trie, lvl log.Level, role common.Roles) Network: new(runtime.TestRuntimeNetwork), Transaction: newTransactionStateMock(), Role: role, + testVersion: version, } }