chore: Integrate scale package into runtime#1824
Conversation
timwu20
requested review from
EclesioMeloJunior,
arijitAD,
edwardmack,
jimjbrettj and
noot
as code owners
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.
|
| tx, err := ext.Encode() | ||
| enc, err := scale.Marshal([]byte("nootwashere")) | ||
| require.NoError(t, err) | ||
| tx := append([]byte{2}, enc...) |
There was a problem hiding this comment.
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.
yeah would be good to document this
There was a problem hiding this comment.
I'll add documentation
There was a problem hiding this comment.
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.
they were in the old code that I had removed.
noot
left a comment
noot
left a comment
There was a problem hiding this comment.
looks good, just a comment/concern about the storageAppend func
jimjbrettj
left a comment
jimjbrettj
left a comment
There was a problem hiding this comment.
Looks good! Just minor comment
timwu20
force-pushed
the
tim-1749
branch
2 times, most recently
from
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.
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.
timwu20
force-pushed
the
tim-1749
branch
2 times, most recently
from
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
3 participants
Changes
lib/runtime/extrinsicpackage, no longer neededib/runtimeto usepkg/scaleTests
Issues
Primary Reviewer