From 491af9516dad3cef582d04162558d83acc225e77 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Fri, 22 Jul 2022 16:24:05 -0400 Subject: [PATCH] chore(lib/runtime/wasmer): refactor `Exec` and `exec` methods (#2686) - Remove nil storage check (temporary instances might not need it, and we should panic if this happens) - Fix behavior for pointers and/or data lengths larger than the max int32 value - Only use `Exec` instead of both directly wrapping `exec` - Inline single/2-lines functions content in `exec`: `malloc`, `clear`, `store`, `load` - Add sentinel errors and error wrappings - Add named returns --- dot/rpc/websocket_test.go | 6 ++- lib/runtime/errors.go | 3 -- lib/runtime/wasmer/exports.go | 22 +++++------ lib/runtime/wasmer/exports_test.go | 22 +++++------ lib/runtime/wasmer/imports_test.go | 18 +++++---- lib/runtime/wasmer/instance.go | 58 +++++++++-------------------- lib/runtime/wasmer/instance_test.go | 4 +- 7 files changed, 57 insertions(+), 76 deletions(-) diff --git a/dot/rpc/websocket_test.go b/dot/rpc/websocket_test.go index 0d0aa0f648..1cd814ed8d 100644 --- a/dot/rpc/websocket_test.go +++ b/dot/rpc/websocket_test.go @@ -50,8 +50,10 @@ var testCalls = []struct { expected: []byte(`{"jsonrpc":"2.0","result":3,"id":5}` + "\n")}, { call: []byte(`{"jsonrpc":"2.0","method":"author_submitAndWatchExtrinsic","params":["0x010203"],"id":6}`), - expected: []byte(`{"jsonrpc":"2.0","error":{"code":null,"message":"Failed to call the ` + - "`" + `TaggedTransactionQueue_validate_transaction` + "`" + ` exported function."},"id":6}` + "\n")}, + expected: []byte(`{"jsonrpc":"2.0","error":{"code":null,` + + `"message":"running runtime function: Failed to call the ` + + "`" + `TaggedTransactionQueue_validate_transaction` + "`" + + ` exported function."},"id":6}` + "\n")}, { call: []byte(`{"jsonrpc":"2.0","method":"state_subscribeRuntimeVersion","params":[],"id":7}`), expected: []byte(`{"jsonrpc":"2.0","result":6,"id":7}` + "\n")}, diff --git a/lib/runtime/errors.go b/lib/runtime/errors.go index 20f0b03ba8..7e0048c7ef 100644 --- a/lib/runtime/errors.go +++ b/lib/runtime/errors.go @@ -19,6 +19,3 @@ var ErrInvalidTransaction = &json2.Error{Code: 1010, Message: "Invalid Transacti // ErrUnknownTransaction is returned if the call to runtime function // TaggedTransactionQueueValidateTransaction fails with value of [1, 1, x] var ErrUnknownTransaction = &json2.Error{Code: 1011, Message: "Unknown Transaction Validity"} - -// ErrNilStorage is returned when the runtime context storage isn't set -var ErrNilStorage = errors.New("runtime context storage is nil") diff --git a/lib/runtime/wasmer/exports.go b/lib/runtime/wasmer/exports.go index 7e22fcfc6e..fbeacabedc 100644 --- a/lib/runtime/wasmer/exports.go +++ b/lib/runtime/wasmer/exports.go @@ -16,7 +16,7 @@ import ( // ValidateTransaction runs the extrinsic through the runtime function // TaggedTransactionQueue_validate_transaction and returns *Validity func (in *Instance) ValidateTransaction(e types.Extrinsic) (*transaction.Validity, error) { - ret, err := in.exec(runtime.TaggedTransactionQueueValidateTransaction, e) + ret, err := in.Exec(runtime.TaggedTransactionQueueValidateTransaction, e) if err != nil { return nil, err } @@ -33,7 +33,7 @@ func (in *Instance) ValidateTransaction(e types.Extrinsic) (*transaction.Validit // Version calls runtime function Core_Version func (in *Instance) Version() (runtime.Version, error) { - res, err := in.exec(runtime.CoreVersion, []byte{}) + res, err := in.Exec(runtime.CoreVersion, []byte{}) if err != nil { return nil, err } @@ -56,12 +56,12 @@ func (in *Instance) Version() (runtime.Version, error) { // Metadata calls runtime function Metadata_metadata func (in *Instance) Metadata() ([]byte, error) { - return in.exec(runtime.Metadata, []byte{}) + return in.Exec(runtime.Metadata, []byte{}) } // BabeConfiguration gets the configuration data for BABE from the runtime func (in *Instance) BabeConfiguration() (*types.BabeConfiguration, error) { - data, err := in.exec(runtime.BabeAPIConfiguration, []byte{}) + data, err := in.Exec(runtime.BabeAPIConfiguration, []byte{}) if err != nil { return nil, err } @@ -77,7 +77,7 @@ func (in *Instance) BabeConfiguration() (*types.BabeConfiguration, error) { // GrandpaAuthorities returns the genesis authorities from the runtime func (in *Instance) GrandpaAuthorities() ([]types.Authority, error) { - ret, err := in.exec(runtime.GrandpaAuthorities, []byte{}) + ret, err := in.Exec(runtime.GrandpaAuthorities, []byte{}) if err != nil { return nil, err } @@ -98,23 +98,23 @@ func (in *Instance) InitializeBlock(header *types.Header) error { return fmt.Errorf("cannot encode header: %w", err) } - _, err = in.exec(runtime.CoreInitializeBlock, encodedHeader) + _, err = in.Exec(runtime.CoreInitializeBlock, encodedHeader) return err } // InherentExtrinsics calls runtime API function BlockBuilder_inherent_extrinsics func (in *Instance) InherentExtrinsics(data []byte) ([]byte, error) { - return in.exec(runtime.BlockBuilderInherentExtrinsics, data) + return in.Exec(runtime.BlockBuilderInherentExtrinsics, data) } // ApplyExtrinsic calls runtime API function BlockBuilder_apply_extrinsic func (in *Instance) ApplyExtrinsic(data types.Extrinsic) ([]byte, error) { - return in.exec(runtime.BlockBuilderApplyExtrinsic, data) + return in.Exec(runtime.BlockBuilderApplyExtrinsic, data) } // FinalizeBlock calls runtime API function BlockBuilder_finalize_block func (in *Instance) FinalizeBlock() (*types.Header, error) { - data, err := in.exec(runtime.BlockBuilderFinalizeBlock, []byte{}) + data, err := in.Exec(runtime.BlockBuilderFinalizeBlock, []byte{}) if err != nil { return nil, err } @@ -161,7 +161,7 @@ func (in *Instance) ExecuteBlock(block *types.Block) ([]byte, error) { // DecodeSessionKeys decodes the given public session keys. Returns a list of raw public keys including their key type. func (in *Instance) DecodeSessionKeys(enc []byte) ([]byte, error) { - return in.exec(runtime.DecodeSessionKeys, enc) + return in.Exec(runtime.DecodeSessionKeys, enc) } // PaymentQueryInfo returns information of a given extrinsic @@ -171,7 +171,7 @@ func (in *Instance) PaymentQueryInfo(ext []byte) (*types.TransactionPaymentQuery return nil, err } - resBytes, err := in.exec(runtime.TransactionPaymentAPIQueryInfo, append(ext, encLen...)) + resBytes, err := in.Exec(runtime.TransactionPaymentAPIQueryInfo, append(ext, encLen...)) if err != nil { return nil, err } diff --git a/lib/runtime/wasmer/exports_test.go b/lib/runtime/wasmer/exports_test.go index 164dc9bb7e..a286bf43ae 100644 --- a/lib/runtime/wasmer/exports_test.go +++ b/lib/runtime/wasmer/exports_test.go @@ -5,7 +5,6 @@ package wasmer import ( "encoding/json" - "errors" "fmt" "math/big" "os" @@ -1022,15 +1021,14 @@ func TestInstance_DecodeSessionKeys(t *testing.T) { func TestInstance_PaymentQueryInfo(t *testing.T) { tests := []struct { - extB []byte - ext string - err error - expect *types.TransactionPaymentQueryInfo + extB []byte + ext string + errMessage string + expect *types.TransactionPaymentQueryInfo }{ { // Was made with @polkadot/api on https://github.com/danforbes/polkadot-js-scripts/tree/create-signed-tx ext: "0xd1018400d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d01bc2b6e35929aabd5b8bc4e5b0168c9bee59e2bb9d6098769f6683ecf73e44c776652d947a270d59f3d37eb9f9c8c17ec1b4cc473f2f9928ffdeef0f3abd43e85d502000000012844616e20466f72626573", //nolint:lll - err: nil, expect: &types.TransactionPaymentQueryInfo{ Weight: 1973000, Class: 0, @@ -1043,12 +1041,14 @@ func TestInstance_PaymentQueryInfo(t *testing.T) { { // incomplete extrinsic ext: "0x4ccde39a5684e7a56da23b22d4d9fbadb023baa19c56495432884d0640000000000000000000000000000000", - err: errors.New("Failed to call the `TransactionPaymentApi_query_info` exported function."), //nolint:revive + errMessage: "running runtime function: " + + "Failed to call the `TransactionPaymentApi_query_info` exported function.", }, { // incomplete extrinsic extB: nil, - err: errors.New("Failed to call the `TransactionPaymentApi_query_info` exported function."), //nolint:revive + errMessage: "running runtime function: " + + "Failed to call the `TransactionPaymentApi_query_info` exported function.", }, } @@ -1066,11 +1066,11 @@ func TestInstance_PaymentQueryInfo(t *testing.T) { ins := NewTestInstance(t, runtime.NODE_RUNTIME) info, err := ins.PaymentQueryInfo(extBytes) - if test.err != nil { - require.Error(t, err) - require.Equal(t, err.Error(), test.err.Error()) + if test.errMessage != "" { + assert.EqualError(t, err, test.errMessage) continue } + require.NoError(t, err) fmt.Println(info.PartialFee.String()) fmt.Println(test.expect.PartialFee.String()) diff --git a/lib/runtime/wasmer/imports_test.go b/lib/runtime/wasmer/imports_test.go index 2f1581711a..705f2052e3 100644 --- a/lib/runtime/wasmer/imports_test.go +++ b/lib/runtime/wasmer/imports_test.go @@ -41,8 +41,9 @@ func Test_ext_offchain_timestamp_version_1(t *testing.T) { res, err := runtimeFunc(0, 0) require.NoError(t, err) - offset, length := runtime.Int64ToPointerAndSize(res.ToI64()) - data := inst.load(offset, length) + outputPtr, outputLength := runtime.Int64ToPointerAndSize(res.ToI64()) + memory := inst.vm.Memory.Data() + data := memory[outputPtr : outputPtr+outputLength] var timestamp int64 err = scale.Unmarshal(data, ×tamp) require.NoError(t, err) @@ -718,10 +719,12 @@ func Test_ext_crypto_ed25519_generate_version_1(t *testing.T) { // we manually store and call the runtime function here since inst.exec assumes // the data returned from the function is a pointer-size, but for ext_crypto_ed25519_generate_version_1, // it's just a pointer - ptr, err := inst.malloc(uint32(len(params))) + ptr, err := inst.ctx.Allocator.Allocate(uint32(len(params))) require.NoError(t, err) - inst.store(params, int32(ptr)) + memory := inst.vm.Memory.Data() + copy(memory[ptr:ptr+uint32(len(params))], params) + dataLen := int32(len(params)) runtimeFunc, ok := inst.vm.Exports["rtm_ext_crypto_ed25519_generate_version_1"] @@ -921,7 +924,7 @@ func Test_ext_crypto_ecdsa_verify_version_2_Table(t *testing.T) { key: []byte{132, 2, 39, 55, 134, 131, 142, 43, 100, 63, 134, 96, 14, 253, 15, 222, 119, 154, 110, 188, 20, 159, 62, 125, 42, 59, 127, 19, 16, 0, 161, 236, 109}, //nolint:lll err: wasmer.NewExportedFunctionError( "rtm_ext_crypto_ecdsa_verify_version_2", - "Failed to call the `%s` exported function."), + "running runtime function: Failed to call the `%s` exported function."), }, "invalid message": { sig: []byte{5, 1, 187, 179, 88, 183, 46, 115, 242, 32, 9, 54, 141, 207, 44, 15, 238, 42, 217, 196, 111, 173, 239, 204, 128, 93, 49, 179, 137, 150, 162, 125, 226, 225, 28, 145, 122, 127, 15, 154, 185, 11, 3, 66, 27, 187, 204, 242, 107, 68, 26, 111, 245, 30, 115, 141, 85, 74, 158, 211, 161, 217, 43, 151, 120, 125, 1}, //nolint:lll @@ -929,7 +932,7 @@ func Test_ext_crypto_ecdsa_verify_version_2_Table(t *testing.T) { key: []byte{132, 2, 39, 206, 55, 134, 131, 142, 43, 100, 63, 134, 96, 14, 253, 15, 222, 119, 154, 110, 188, 20, 159, 62, 125, 42, 59, 127, 19, 16, 0, 161, 236, 109}, //nolint:lll err: wasmer.NewExportedFunctionError( "rtm_ext_crypto_ecdsa_verify_version_2", - "Failed to call the `%s` exported function."), + "running runtime function: Failed to call the `%s` exported function."), }, } for name, tc := range testCases { @@ -1592,7 +1595,8 @@ func Test_ext_default_child_storage_storage_kill_version_3(t *testing.T) { key: []byte(`fakekey`), limit: optLimit2, expected: []byte{0, 0, 0, 0, 0}, - errMsg: "Failed to call the `rtm_ext_default_child_storage_storage_kill_version_3` exported function.", + errMsg: "running runtime function: " + + "Failed to call the `rtm_ext_default_child_storage_storage_kill_version_3` exported function.", }, {key: testChildKey, limit: optLimit2, expected: []byte{1, 2, 0, 0, 0}}, {key: testChildKey, limit: nil, expected: []byte{0, 1, 0, 0, 0}}, diff --git a/lib/runtime/wasmer/instance.go b/lib/runtime/wasmer/instance.go index 59b3be6f2d..b3a7f014b1 100644 --- a/lib/runtime/wasmer/instance.go +++ b/lib/runtime/wasmer/instance.go @@ -264,67 +264,45 @@ func (in *Instance) Stop() { } } -// Store func -func (in *Instance) store(data []byte, location int32) { - mem := in.vm.Memory.Data() - copy(mem[location:location+int32(len(data))], data) -} - -// Load load -func (in *Instance) load(location, length int32) []byte { - mem := in.vm.Memory.Data() - return mem[location : location+length] -} +var ( + ErrInstanceIsStopped = errors.New("instance is stopped") + ErrExportFunctionNotFound = errors.New("export function not found") +) // Exec calls the given function with the given data -func (in *Instance) Exec(function string, data []byte) ([]byte, error) { - return in.exec(function, data) -} - -// Exec func -func (in *Instance) exec(function string, data []byte) ([]byte, error) { - if in.ctx.Storage == nil { - return nil, runtime.ErrNilStorage - } - +func (in *Instance) Exec(function string, data []byte) (result []byte, err error) { in.Lock() defer in.Unlock() if in.isClosed { - return nil, errors.New("instance is stopped") + return nil, ErrInstanceIsStopped } - ptr, err := in.malloc(uint32(len(data))) + dataLength := uint32(len(data)) + inputPtr, err := in.ctx.Allocator.Allocate(dataLength) if err != nil { - return nil, err + return nil, fmt.Errorf("allocating input memory: %w", err) } - defer in.clear() + defer in.ctx.Allocator.Clear() // Store the data into memory - in.store(data, int32(ptr)) - datalen := int32(len(data)) + memory := in.vm.Memory.Data() + copy(memory[inputPtr:inputPtr+dataLength], data) runtimeFunc, ok := in.vm.Exports[function] if !ok { - return nil, fmt.Errorf("could not find exported function %s", function) + return nil, fmt.Errorf("%w: %s", ErrExportFunctionNotFound, function) } - res, err := runtimeFunc(int32(ptr), datalen) + wasmValue, err := runtimeFunc(int32(inputPtr), int32(dataLength)) if err != nil { - return nil, err + return nil, fmt.Errorf("running runtime function: %w", err) } - offset, length := runtime.Int64ToPointerAndSize(res.ToI64()) - return in.load(offset, length), nil -} - -func (in *Instance) malloc(size uint32) (uint32, error) { - return in.ctx.Allocator.Allocate(size) -} - -func (in *Instance) clear() { - in.ctx.Allocator.Clear() + outputPtr, outputLength := runtime.Int64ToPointerAndSize(wasmValue.ToI64()) + memory = in.vm.Memory.Data() // call Data() again to get larger slice + return memory[outputPtr : outputPtr+outputLength], nil } // NodeStorage to get reference to runtime node service diff --git a/lib/runtime/wasmer/instance_test.go b/lib/runtime/wasmer/instance_test.go index 97b50c3bc5..8f1a9b3c9b 100644 --- a/lib/runtime/wasmer/instance_test.go +++ b/lib/runtime/wasmer/instance_test.go @@ -20,10 +20,10 @@ func TestConcurrentRuntimeCalls(t *testing.T) { // execute 2 concurrent calls to the runtime go func() { - _, _ = instance.exec(runtime.CoreVersion, []byte{}) + _, _ = instance.Exec(runtime.CoreVersion, []byte{}) }() go func() { - _, _ = instance.exec(runtime.CoreVersion, []byte{}) + _, _ = instance.Exec(runtime.CoreVersion, []byte{}) }() }