Update rlp-encoding lib to rlp
#94
Conversation
ryanio
added
PR state: needs review
dependencies
Codecov Report
@@ Coverage Diff @@
## master #94 +/- ##
==========================================
+ Coverage 83.33% 83.39% +0.05%
==========================================
Files 15 15
Lines 1158 1162 +4
Branches 191 191
==========================================
+ Hits 965 969 +4
Misses 73 73
Partials 120 120
Continue to review full report at Codecov.
|
| @@ -106,3 +107,9 @@ export class Deferred<T> { | |||
| export function createDeferred<T>(): Deferred<T> { | |||
| return new Deferred() | |||
| } | |||
|
|
|||
| export function unstrictDecode(value: Buffer) { | |||
| // rlp library throws on remainder.length !== 0 | |||
There was a problem hiding this comment.
Should this an issue of some kind? If so can you open on rlp?
There was a problem hiding this comment.
I'm not too sure, I think checking for 0 remainder is just a cautious check. Could we solicit feedback from some other team members?
This little bypass is just a trick in the rlp src that if stream is passed as true, it'll return the decoded data before it does the remainder check 😆 but it is fragile workaround: https://github.com/ethereumjs/rlp/blob/75f398193fef0c2416fd0f68b4e2794273105620/src/index.ts#L71-L76
There was a problem hiding this comment.
Ok, maybe we'll find this out along the way when we have more to do with the devp2p library and questions like this can benefit from the daily practical experience and some grown intuition. Will leave for now with this comment.
There was a problem hiding this comment.
Ok, this looks good, thanks Ryan, nice issue to tackle since open for ages! 😄 I linked this locally within the client, client is still syncing (my biggest fear these days on every day work, "is the client still syncing?" 😂 ). Will merge once the tests pass.
I fixed the build command along the way which was pointing to a non-exising script.
Also something I stumbled upon, just dropping here if someone has a better understanding what was going on and how to (permanently) fix:
I got these kind of errors when building the client, related to some @types/node version conflict:
../ethereumjs-devp2p/dist/rlpx/rlpx.d.ts:1:23 - error TS4090: Conflicting definitions for 'node' found at '/ethereumjs-devp2p/node_modules/@types/node/index.d.ts' and '/ethereumjs-client/node_modules/@types/node/index.d.ts'. Consider installing a specific version of this library to resolve the conflict.
1 /// <reference types="node" />Not sure if this only happened due to the local linking, I tried reinstalling node_modules (with a deleted package-lock.json 😋 ) but this didn't make any difference. At the end I "solved" this by doing a npm i @types/node@14.11.5 --no-save (the package version from the client) within the ethereumjs-devp2p folder.
Closes #49.