-
Notifications
You must be signed in to change notification settings - Fork 26
Accommodate new marine-js and avm interface #181
Conversation
FluenceJS version is 0.25.0-new-marine-avm-interface-e9dd423-22-1To install it run: npm login --registry https://npm.fluence.dev
npm i @fluencelabs/fluence@0.25.0-new-marine-avm-interface-e9dd423-22-1 --registry=https://npm.fluence.dev |
| avmCallResult = e instanceof Error ? e : new Error((e as any).toString()); | ||
| } |
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.
not critical of course, but I think there is no need for a separate newData variable
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.
This was left intentionally to show what is being swapped with what. Otherwise is hard to track what is data, prevdata and newdata
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. kind of agree
| return { | ||
| retCode: ResultCodes.success, | ||
| result: result as JSONValue, | ||
| }; |
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 think in the furture we should get rid of the typecasts because who knows what data can be sent by bad actors - I think everything should be checked and acted apon appropriatly instead of just failing
shamsartem
left a comment
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.
Thanks for the work!
Not sure, if I understand everything correctly, but the code itself seems fine
No description provided.