-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add support for nativeToScVal
to convert strings to all number sizes.
#763
Conversation
Size Change: +1.48 kB (+0.05%) Total Size: 3.21 MB
|
expect(parseInt(newTx.operations[1].amount)).to.equal(2); | ||
}), | ||
1 | ||
); |
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.
Reviewers' note: this is just whitespace changes from a stray yarn fmt
(not sure how the unformatted versions snuck in)
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.
detailed pr description was very helpful for doing review, thanks!
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.
Good catch!
Previously,
i32
andu32
could not automatically come from string values when usingnativeToScVal
. Namely,nativeToScVal("4", { type: "i32" })
was explicitly forbidden. This was my design: the rationale was that if it's a 32-bit integer, it should be passed as aNumber
.However, this logic does not hold true for maps, because in JavaScript, all map keys are strings. Therefore, creating an
ScVal
representing an integer-to-integer map is impossible vianativeToScVal
, e.g., a function signature like this one:cannot be executed by
because
1234
would be interpreted as a string, and thus could not be automatically converted to anumber
for au32
inference.