-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Integrate scale package into runtime #1824
Conversation
Codecov Report
@@ Coverage Diff @@
## development #1824 +/- ##
===============================================
+ Coverage 59.40% 59.41% +0.01%
===============================================
Files 194 192 -2
Lines 19799 19521 -278
===============================================
- Hits 11761 11599 -162
+ Misses 6084 6013 -71
+ Partials 1954 1909 -45
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
require.NoError(t, err) | ||
tx := append([]byte{2}, enc...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I right in saying that the 2 is appended because its a IncludeDataType extrinsic? If so, since we are deleting that file, will we be able to know what the leading byte represents/what values it can have in the future? I was initially confused at what this 2 was and I'm not sure how I would figure that out with the extrinsic file gone. Maybe this isn't a huge concern but I wanted to ask
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah would be good to document this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there other possible prefixes that should be documented besides the 2 case? Or can those be found elsewhere if need be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they were in the old code that I had removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, just a comment/concern about the storageAppend
func
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just minor comment
d016c44
to
6b242b2
Compare
} else if err != nil { | ||
// error comes from scale now, so do a string check | ||
if err != nil { | ||
if strings.Contains(err.Error(), "EOF") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does errors.Is()
work here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no cause I don't use an exposed custom error type. I could probably do that to have some sort of scale.UnmarshalEOFError
or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work!
484f81d
to
4dc9e15
Compare
🎉 This PR is included in version 0.6.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
* revise APIItems * update scale integration for version * rm lib/runtime/extrinsic * lib/runtime/life updates * update wasmer * wip * update skipped test * cr feedback, fixed storageAppend * update test
Changes
lib/runtime/extrinsic
package, no longer neededib/runtime
to usepkg/scale
Tests
Issues
Primary Reviewer