chore(lib/runtime): refactor version struct and codec by qdm12 · Pull Request #2673 · ChainSafe/gossamer

qdm12 · 2022-07-15T15:14:49Z

Changes

Version code
- 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

Tests

Issues

I had to change a whole bunch of version related code to persist the state version, and I thought I would simplify it since it was rather confusing as a reader.

Primary Reviewer

@timwu20

codecov · 2022-07-20T15:42:18Z

Codecov Report

Merging #2673 (3386e46) into development (8b0f6f0) will decrease coverage by 0.04%.
The diff coverage is 85.71%.

Additional details and impacted files

@@               Coverage Diff               @@
##           development    #2673      +/-   ##
===============================================
- Coverage        62.91%   62.86%   -0.05%     
===============================================
  Files              213      213              
  Lines            27081    26993      -88     
===============================================
- Hits             17037    16970      -67     
+ Misses            8491     8475      -16     
+ Partials          1553     1548       -5

noot

i think this is fine assuming the encoding is the same (should be cause tests pass :p)

dot/core/service.go

dot/rpc/subscription/listeners.go

lib/runtime/version.go

qdm12 · 2022-08-10T12:36:49Z

⚠️ do not merge, this needs more changes (to support N versions of the version since the new state_version field is yet another version).

qdm12 · 2022-08-11T14:21:40Z

This is ready for re-reviews again, thanks!

- Simplify testing in other packages - Remove version decode method (just use `scale.Unmarshal` directly) - Remove clone struct with exported fields for scale codec - Remove version runtime mockery mock - Refactor multiple `TestInstance_Version*` in one test with test cases - Add runtime API items as expected fields in tests - side-effect: add `Get` prefix to getter methods (sus but better than having messy double-structs-which-make-testing-hard)

- Remove `Version` interface - Use `legacy` bool field in version struct - Unexport `LegacyVersionData` - Remove all getter methods - Move decoding logic from wasmer to lib/runtime/version.go - Use struct value instead of its pointer

- Add `WithLegacy()` method to set unexported `legacy` field in wasmer test

- Always encode using all latest fields - Decode required fields then optional fields - Remove `legacyData` struct - Update `Encode` and `DecodeVersion` comments - Update unit tests

- Update comment for `scale.Marshal` on `Version` - Remove `Test_Version_Encode`

I am Quentin Mc Gaw, a software engineer working the Go Polkadot host **Gossamer**. I have been working full time on Gossamer since October 2021, mostly on the state trie and storage. I have also made a [few minor pull requests](https://github.com/w3f/polkadot-spec/pulls?q=is%3Apr+is%3Aclosed+author%3Aqdm12) to the Polkadot specification repository. I am requesting to join the Fellowship at rank 1. ## Main contributions ### Gossamer - Fix memory leaks - Trie encoding buffer pools usage fixed [#2009](ChainSafe/gossamer#2009) - Fix state map of tries memory leak [#2286](ChainSafe/gossamer#2286) - Fix sync benchmark [#2234](ChainSafe/gossamer#2234) - Trie proof fixes ([#2604](ChainSafe/gossamer#2604), [#2661](ChainSafe/gossamer#2661)) - Fix end to end tests orchestration ([#2470](ChainSafe/gossamer#2470), [#2452](ChainSafe/gossamer#2452), [#2385](ChainSafe/gossamer#2385), [#2370](ChainSafe/gossamer#2370)) - State trie statistics ([#2378](ChainSafe/gossamer#2378), [#2310](ChainSafe/gossamer#2310), [#2272](ChainSafe/gossamer#2272)) - State trie fixes and improvements - Only deep copy nodes when mutation is certain [#2352](ChainSafe/gossamer#2352) and [#2223](ChainSafe/gossamer#2223) - Only deep copy necessary fields of a node [#2384](ChainSafe/gossamer#2384) - Use Merkle values for database keys instead of always hash [#2725](ChainSafe/gossamer#2725) - Opportunistic parallel Merkle value commputing [#2081](ChainSafe/gossamer#2081) - Grandpa capped number of tracked messages ([#2490](ChainSafe/gossamer#2490), [#2485](ChainSafe/gossamer#2485)) - Add pprof HTTP service for profiling [#1991](ChainSafe/gossamer#1991) Ongoing work: - State trie lazy loading and caching - State trie v1 support ([#2736](ChainSafe/gossamer#2736), [#2747](ChainSafe/gossamer#2747), [#2687](ChainSafe/gossamer#2687), [#2686](ChainSafe/gossamer#2686), [#2685](ChainSafe/gossamer#2685), [#2673](ChainSafe/gossamer#2673), [#2611](ChainSafe/gossamer#2611), [#2530](ChainSafe/gossamer#2530)) ### Polkadot specification ➡️ [Pull requests from qdm12](https://github.com/w3f/polkadot-spec/pulls?q=is%3Apr+is%3Aclosed+author%3Aqdm12)

github-actions · 2022-11-23T18:27:50Z

🎉 This PR is included in version 0.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch 2 times, most recently from a82106d to 358f997 Compare July 19, 2022 15:22

qdm12 marked this pull request as ready for review July 19, 2022 15:51

qdm12 requested review from EclesioMeloJunior, edwardmack, jimjbrettj, kishansagathiya, noot and timwu20 as code owners July 19, 2022 15:51

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch 4 times, most recently from ad971fa to 5704086 Compare July 20, 2022 15:16

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch 2 times, most recently from 0bfc86f to acff5db Compare July 20, 2022 18:52

qdm12 added the Status: Review Needed label Jul 25, 2022

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch from acff5db to ecfdd74 Compare July 28, 2022 14:54

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch from ecfdd74 to 9675efa Compare August 8, 2022 15:24

noot approved these changes Aug 8, 2022

View reviewed changes

EclesioMeloJunior approved these changes Aug 9, 2022

View reviewed changes

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch from aa3bad5 to b187971 Compare August 9, 2022 17:04

timwu20 requested changes Aug 9, 2022

View reviewed changes

dot/core/service.go Show resolved Hide resolved

dot/rpc/subscription/listeners.go Show resolved Hide resolved

lib/runtime/version.go Outdated Show resolved Hide resolved

qdm12 mentioned this pull request Aug 9, 2022

chore(dot/rpc): NewStateRuntimeVersionResponse #2740

Merged

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch from b187971 to ab9721f Compare August 9, 2022 22:26

qdm12 mentioned this pull request Aug 10, 2022

feat(pkg/scale): add Encoder with Encode method #2741

Merged

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch 3 times, most recently from 901376b to c4e418d Compare August 10, 2022 18:51

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch from c4e418d to a2b9f95 Compare August 10, 2022 18:57

qdm12 requested review from noot and timwu20 August 11, 2022 00:09

qdm12 changed the title ~~chore(lib/runtime): export fields of VersionData~~ chore(lib/runtime): refactor version struct and codec Aug 11, 2022

This was referenced Aug 11, 2022

feat(wasmer): get and cache state version in instance context #2747

Merged

feat(pkg/scale): add use of pkg/error Wrap for error handling #2708

Merged

feat(scale): add range checks to decodeUint function #2683

Merged

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch 2 times, most recently from 11e5200 to 6486321 Compare August 15, 2022 19:22

qdm12 added 9 commits August 16, 2022 00:07

Only single VersionData struct

242bb5d

- Remove `Version` interface - Use `legacy` bool field in version struct - Unexport `LegacyVersionData` - Remove all getter methods - Move decoding logic from wasmer to lib/runtime/version.go - Use struct value instead of its pointer

Rename VersionData to Version

3f6c09c

Decode method -> DecodeVersion function

07c9d3e

Add more comments to version.Encode usage

756d00e

Fix Test_Instance_Version for Kusama

930c938

- Add `WithLegacy()` method to set unexported `legacy` field in wasmer test

More tests

41c705f

Core_Version codec changes

f48ee8e

- Always encode using all latest fields - Decode required fields then optional fields - Remove `legacyData` struct - Update `Encode` and `DecodeVersion` comments - Update unit tests

Remove version Encode method

3386e46

- Update comment for `scale.Marshal` on `Version` - Remove `Test_Version_Encode`

qdm12 force-pushed the qdm12/runtime/version-structs-refactor branch from 6486321 to 3386e46 Compare August 16, 2022 00:08

timwu20 approved these changes Aug 16, 2022

View reviewed changes

qdm12 merged commit 5c8446e into development Aug 16, 2022

qdm12 deleted the qdm12/runtime/version-structs-refactor branch August 16, 2022 12:09

qdm12 mentioned this pull request Sep 29, 2022

Member Request polkadot-fellows/seeding#28

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(lib/runtime): refactor version struct and codec#2673

chore(lib/runtime): refactor version struct and codec#2673
qdm12 merged 9 commits intodevelopmentfrom
qdm12/runtime/version-structs-refactor

qdm12 commented Jul 15, 2022 •

edited

Loading

Uh oh!

codecov bot commented Jul 20, 2022 •

edited

Loading

Uh oh!

noot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qdm12 commented Aug 10, 2022

Uh oh!

qdm12 commented Aug 11, 2022

Uh oh!

github-actions bot commented Nov 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

qdm12 commented Jul 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Issues

Primary Reviewer

Uh oh!

codecov bot commented Jul 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

noot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qdm12 commented Aug 10, 2022

Uh oh!

qdm12 commented Aug 11, 2022

Uh oh!

github-actions bot commented Nov 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

qdm12 commented Jul 15, 2022 •

edited

Loading

codecov bot commented Jul 20, 2022 •

edited

Loading