Skip to content

Commit

Permalink
chore(lib/runtime): refactor version struct and codec (#2673)
Browse files Browse the repository at this point in the history
- Single `Version` struct
- Encoding the `Version` always encode all the last fields
- Decoding the version data to `Version` sets possible absent fields as zero if not in the data (`transaction_version`)
- Remove `LegacyVersionData` struct, `Version` interface, getter methods, cloned structs for codec
- Move decoding logic from `lib/runtime/wasmer/exports.go` to `lib/runtime/version.go` in `DecodeVersion` function
- Use struct value for `Version` instead of its pointer (no reason to use a pointer)
- Test code
  - Refactor multiple `TestInstance_Version*` in one test with test cases
  - Add runtime API items as expected fields in tests
  - Remove version runtime mockery mock
  - Simplify testing in other packages since we no longer need to use a test-only exported constructor `NewVersionData`
  • Loading branch information
qdm12 authored Aug 16, 2022
1 parent 8b0f6f0 commit 5c8446e
Show file tree
Hide file tree
Showing 23 changed files with 575 additions and 793 deletions.
4 changes: 2 additions & 2 deletions dot/core/messages_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ func createExtrinsic(t *testing.T, rt runtime.Instance, genHash common.Hash, non
Era: ctypes.ExtrinsicEra{IsImmortalEra: false},
GenesisHash: ctypes.Hash(genHash),
Nonce: ctypes.NewUCompactFromUInt(nonce),
SpecVersion: ctypes.U32(rv.SpecVersion()),
SpecVersion: ctypes.U32(rv.SpecVersion),
Tip: ctypes.NewUCompactFromUInt(0),
TransactionVersion: ctypes.U32(rv.TransactionVersion()),
TransactionVersion: ctypes.U32(rv.TransactionVersion),
}

// Sign the transaction using Alice's key
Expand Down
9 changes: 5 additions & 4 deletions dot/core/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,26 +454,27 @@ func (s *Service) DecodeSessionKeys(enc []byte) ([]byte, error) {
}

// GetRuntimeVersion gets the current RuntimeVersion
func (s *Service) GetRuntimeVersion(bhash *common.Hash) (runtime.Version, error) {
func (s *Service) GetRuntimeVersion(bhash *common.Hash) (
version runtime.Version, err error) {
var stateRootHash *common.Hash

// If block hash is not nil then fetch the state root corresponding to the block.
if bhash != nil {
var err error
stateRootHash, err = s.storageState.GetStateRootFromBlock(bhash)
if err != nil {
return nil, err
return version, err
}
}

ts, err := s.storageState.TrieState(stateRootHash)
if err != nil {
return nil, err
return version, err
}

rt, err := s.blockState.GetRuntime(bhash)
if err != nil {
return nil, err
return version, err
}

rt.SetContextStorage(ts)
Expand Down
8 changes: 4 additions & 4 deletions dot/core/service_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ func TestService_HandleRuntimeChanges(t *testing.T) {
v, err := rt.Version()
require.NoError(t, err)

currSpecVersion := v.SpecVersion() // genesis runtime version.
currSpecVersion := v.SpecVersion // genesis runtime version.
hash := s.blockState.BestBlockHash() // genesisHash

digest := types.NewDigest()
Expand Down Expand Up @@ -615,7 +615,7 @@ func TestService_HandleRuntimeChanges(t *testing.T) {

v, err = parentRt.Version()
require.NoError(t, err)
require.Equal(t, v.SpecVersion(), currSpecVersion)
require.Equal(t, v.SpecVersion, currSpecVersion)

bhash1 := newBlock1.Header.Hash()
err = s.blockState.HandleRuntimeChanges(ts, parentRt, bhash1)
Expand All @@ -637,14 +637,14 @@ func TestService_HandleRuntimeChanges(t *testing.T) {

v, err = rt.Version()
require.NoError(t, err)
require.Equal(t, v.SpecVersion(), currSpecVersion)
require.Equal(t, v.SpecVersion, currSpecVersion)

rt, err = s.blockState.GetRuntime(&rtUpdateBhash)
require.NoError(t, err)

v, err = rt.Version()
require.NoError(t, err)
require.Equal(t, v.SpecVersion(), updatedSpecVersion)
require.Equal(t, v.SpecVersion, updatedSpecVersion)
}

func TestService_HandleCodeSubstitutes(t *testing.T) {
Expand Down
61 changes: 30 additions & 31 deletions dot/core/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
rtstorage "github.com/ChainSafe/gossamer/lib/runtime/storage"
"github.com/ChainSafe/gossamer/lib/runtime/wasmer"
"github.com/ChainSafe/gossamer/lib/transaction"
"github.com/ChainSafe/gossamer/lib/trie"
"github.com/ChainSafe/gossamer/pkg/scale"
cscale "github.com/centrifuge/go-substrate-rpc-client/v4/scale"
"github.com/centrifuge/go-substrate-rpc-client/v4/signature"
Expand Down Expand Up @@ -61,19 +62,18 @@ func generateExtrinsic(t *testing.T) (extrinsic, externalExtrinsic types.Extrins
t.Helper()
meta := generateTestCentrifugeMetadata(t)

testAPIItem := runtime.APIItem{
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
Ver: 99,
rv := runtime.Version{
SpecName: []byte("polkadot"),
ImplName: []byte("parity-polkadot"),
AuthoringVersion: authoringVersion,
SpecVersion: specVersion,
ImplVersion: implVersion,
APIItems: []runtime.APIItem{{
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
Ver: 99,
}},
TransactionVersion: transactionVersion,
}
rv := runtime.NewVersionData(
[]byte("polkadot"),
[]byte("parity-polkadot"),
authoringVersion,
specVersion,
implVersion,
[]runtime.APIItem{testAPIItem},
transactionVersion,
)

keyring, err := keystore.NewSr25519Keyring()
require.NoError(t, err)
Expand All @@ -95,9 +95,9 @@ func generateExtrinsic(t *testing.T) (extrinsic, externalExtrinsic types.Extrins
Era: ctypes.ExtrinsicEra{IsImmortalEra: true},
GenesisHash: testGenHash,
Nonce: ctypes.NewUCompactFromUInt(uint64(0)),
SpecVersion: ctypes.U32(rv.SpecVersion()),
SpecVersion: ctypes.U32(rv.SpecVersion),
Tip: ctypes.NewUCompactFromUInt(0),
TransactionVersion: ctypes.U32(rv.TransactionVersion()),
TransactionVersion: ctypes.U32(rv.TransactionVersion),
}

// Sign the transaction using Alice's default account
Expand Down Expand Up @@ -1008,21 +1008,20 @@ func TestService_DecodeSessionKeys(t *testing.T) {

func TestServiceGetRuntimeVersion(t *testing.T) {
t.Parallel()
testAPIItem := runtime.APIItem{
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
Ver: 99,
rv := runtime.Version{
SpecName: []byte("polkadot"),
ImplName: []byte("parity-polkadot"),
AuthoringVersion: authoringVersion,
SpecVersion: specVersion,
ImplVersion: implVersion,
APIItems: []runtime.APIItem{{
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
Ver: 99,
}},
TransactionVersion: transactionVersion,
}
rv := runtime.NewVersionData(
[]byte("polkadot"),
[]byte("parity-polkadot"),
authoringVersion,
specVersion,
implVersion,
[]runtime.APIItem{testAPIItem},
transactionVersion,
)

ts := rtstorage.NewTrieState(nil)
emptyTrie := trie.NewEmptyTrie()
ts := rtstorage.NewTrieState(emptyTrie)

execTest := func(t *testing.T, s *Service, bhash *common.Hash, exp runtime.Version, expErr error) {
res, err := s.GetRuntimeVersion(bhash)
Expand All @@ -1041,7 +1040,7 @@ func TestServiceGetRuntimeVersion(t *testing.T) {
service := &Service{
storageState: mockStorageState,
}
execTest(t, service, &common.Hash{}, nil, errDummyErr)
execTest(t, service, &common.Hash{}, runtime.Version{}, errDummyErr)
})

t.Run("trie state err", func(t *testing.T) {
Expand All @@ -1053,7 +1052,7 @@ func TestServiceGetRuntimeVersion(t *testing.T) {
service := &Service{
storageState: mockStorageState,
}
execTest(t, service, &common.Hash{}, nil, errDummyErr)
execTest(t, service, &common.Hash{}, runtime.Version{}, errDummyErr)
})

t.Run("get runtime err", func(t *testing.T) {
Expand All @@ -1069,7 +1068,7 @@ func TestServiceGetRuntimeVersion(t *testing.T) {
storageState: mockStorageState,
blockState: mockBlockState,
}
execTest(t, service, &common.Hash{}, nil, errDummyErr)
execTest(t, service, &common.Hash{}, runtime.Version{}, errDummyErr)
})

t.Run("happy path", func(t *testing.T) {
Expand Down
18 changes: 3 additions & 15 deletions dot/rpc/modules/api_mocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ 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/ChainSafe/gossamer/lib/runtime"
"github.com/ChainSafe/gossamer/lib/transaction"
"github.com/stretchr/testify/mock"
)
Expand Down Expand Up @@ -63,22 +63,10 @@ func NewMockCoreAPI() *modulesmocks.CoreAPI {
m := new(modulesmocks.CoreAPI)
m.On("InsertKey", mock.AnythingOfType("crypto.Keypair"), mock.AnythingOfType("string")).Return(nil)
m.On("HasKey", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(false, nil)
m.On("GetRuntimeVersion", mock.AnythingOfType("*common.Hash")).Return(NewMockVersion(), nil)
m.On("GetRuntimeVersion", mock.AnythingOfType("*common.Hash")).
Return(runtime.Version{SpecName: []byte(`mock-spec`)}, 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.Version {
m := new(runtimemocks.Version)
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
}
4 changes: 1 addition & 3 deletions dot/rpc/modules/mocks/core_api.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions dot/rpc/modules/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,13 +304,13 @@ func (sm *StateModule) GetRuntimeVersion(
return err
}

res.SpecName = string(rtVersion.SpecName())
res.ImplName = string(rtVersion.ImplName())
res.AuthoringVersion = rtVersion.AuthoringVersion()
res.SpecVersion = rtVersion.SpecVersion()
res.ImplVersion = rtVersion.ImplVersion()
res.TransactionVersion = rtVersion.TransactionVersion()
res.Apis = ConvertAPIs(rtVersion.APIItems())
res.SpecName = string(rtVersion.SpecName)
res.ImplName = string(rtVersion.ImplName)
res.AuthoringVersion = rtVersion.AuthoringVersion
res.SpecVersion = rtVersion.SpecVersion
res.ImplVersion = rtVersion.ImplVersion
res.TransactionVersion = rtVersion.TransactionVersion
res.Apis = ConvertAPIs(rtVersion.APIItems)

return nil
}
Expand Down
26 changes: 13 additions & 13 deletions dot/rpc/modules/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,25 +448,25 @@ func TestStateModuleGetReadProof(t *testing.T) {

func TestStateModuleGetRuntimeVersion(t *testing.T) {
hash := common.MustHexToHash("0x3aa96b0149b6ca3688878bdbd19464448624136398e3ce45b9e755d3ab61355a")
testAPIItem := runtime.APIItem{
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
Ver: 99,
version := runtime.Version{
SpecName: []byte("polkadot"),
ImplName: []byte("parity-polkadot"),
AuthoringVersion: 0,
SpecVersion: 25,
ImplVersion: 0,
APIItems: []runtime.APIItem{{
Name: [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
Ver: 99,
}},
TransactionVersion: 5,
}
version := runtime.NewVersionData(
[]byte("polkadot"),
[]byte("parity-polkadot"),
0,
25,
0,
[]runtime.APIItem{testAPIItem},
5,
)

mockCoreAPI := new(mocks.CoreAPI)
mockCoreAPI.On("GetRuntimeVersion", &hash).Return(version, nil)

mockCoreAPIErr := new(mocks.CoreAPI)
mockCoreAPIErr.On("GetRuntimeVersion", &hash).Return(nil, errors.New("GetRuntimeVersion Error"))
mockCoreAPIErr.On("GetRuntimeVersion", &hash).
Return(runtime.Version{}, errors.New("GetRuntimeVersion Error"))

type fields struct {
networkAPI NetworkAPI
Expand Down
28 changes: 14 additions & 14 deletions dot/rpc/subscription/listeners.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,13 @@ func (l *RuntimeVersionListener) Listen() {
return
}
ver := modules.StateRuntimeVersionResponse{}
ver.SpecName = string(rtVersion.SpecName())
ver.ImplName = string(rtVersion.ImplName())
ver.AuthoringVersion = rtVersion.AuthoringVersion()
ver.SpecVersion = rtVersion.SpecVersion()
ver.ImplVersion = rtVersion.ImplVersion()
ver.TransactionVersion = rtVersion.TransactionVersion()
ver.Apis = modules.ConvertAPIs(rtVersion.APIItems())
ver.SpecName = string(rtVersion.SpecName)
ver.ImplName = string(rtVersion.ImplName)
ver.AuthoringVersion = rtVersion.AuthoringVersion
ver.SpecVersion = rtVersion.SpecVersion
ver.ImplVersion = rtVersion.ImplVersion
ver.TransactionVersion = rtVersion.TransactionVersion
ver.Apis = modules.ConvertAPIs(rtVersion.APIItems)

go l.wsconn.safeSend(newSubscriptionResponse(stateRuntimeVersionMethod, l.subID, ver))

Expand All @@ -430,13 +430,13 @@ func (l *RuntimeVersionListener) Listen() {

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())
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))
}
Expand Down
2 changes: 1 addition & 1 deletion dot/rpc/subscription/listeners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ func TestRuntimeChannelListener_Listen(t *testing.T) {
SpecVersion: 25,
ImplVersion: 0,
TransactionVersion: 5,
Apis: modules.ConvertAPIs(version.APIItems()),
Apis: modules.ConvertAPIs(version.APIItems),
}

expectedUpdateResponse := newSubcriptionBaseResponseJSON()
Expand Down
4 changes: 2 additions & 2 deletions dot/state/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -598,15 +598,15 @@ func (bs *BlockState) HandleRuntimeChanges(newState *rtstorage.TrieState,

// only update runtime during code substitution if runtime SpecVersion is updated
previousVersion, _ := rt.Version()
if previousVersion.SpecVersion() == newVersion.SpecVersion() {
if previousVersion.SpecVersion == newVersion.SpecVersion {
logger.Info("not upgrading runtime code during code substitution")
bs.StoreRuntime(bHash, rt)
return nil
}

logger.Infof(
"🔄 detected runtime code change, upgrading with block %s from previous code hash %s and spec %d to new code hash %s and spec %d...", //nolint:lll
bHash, codeHash, previousVersion.SpecVersion(), currCodeHash, newVersion.SpecVersion())
bHash, codeHash, previousVersion.SpecVersion, currCodeHash, newVersion.SpecVersion)
}

rtCfg := wasmer.Config{
Expand Down
Loading

0 comments on commit 5c8446e

Please sign in to comment.