-
Notifications
You must be signed in to change notification settings - Fork 53
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
[Suggestion] Switch Lumos validation and serialization to Duck Typing #594
Comments
Hi @phroi, the function import * as helpers from '@ckb-lumos/helpers'
function compatibilize<Fn extends (arg: any) => any>(originalFn: Fn): Fn {
return function (immutableOrPlain: any) {
const plain = immutableOrPlain.toJS
? immutableOrPlain.toJS()
: immutableOrPlain;
return originalFn(plain);
} as Fn;
}
export const encodeToAddress = compatibilize(helpers.encodeToAddress) |
On one side there are many other methods where the Lumos API makes the assumption that input objects are exclusively a type implementation of the interface. On the other side there are other methods that exclusively check if the correct fields are correctly defined on the object. See for example Keeping both forms of validation across Lumos is inconsistent and confusing for the final DApp developer as it's not documented in any way in the methods signature nor documentation... Thank you @homura for your suggestion, I appreciate you spending time to look for a good solution, but Lumos Utils is a small library itself, not the final DApp. I cannot possibly ask the final DApp developer to call I'll leave this issue open as in my opinion this issue should be gradually addressed |
Thank you for pointing out the design inconsistency in Lumos between CKB-related and non-CKB-related objects. This inconsistency can be confusing for developers who want to use Duck Typing. We will review whether the validation is too strict for the objects related to CKB types |
Brilliant!! That's only one side of the story tho, the other is serialization. Going by memory codecs handle serialization perfectly, but there are other times in which these structures are serialized as JSON. This happens for example when I call
|
Hey @homura, I wanted to let you know that I have dropped ImmutableJS and re-implemented the data structures as Typescript classes (frozen at construction time) and the additional properties as Symbols (hackish, but bypasses both validation and serialization so far). For example, now I implement a Script with:
With #598 I could probably move away from Symbols, but I don't really mind keeping them like this 🤔 What's your opinion? |
@phroi Thx for the update. Could you explain why all ckb-related objects are brozen in ckb-utils instead of using plain JS objects directly? |
Thank you for asking @homura 🙏 An unfrozen plain JS objects in
|
Exactly, however, it's important to prevent malicious overriding behavior. If someone is able to modify the object, they can also do other kinds of overriding. For example, they could override the window.Uint8Array = class extends Uint8Array {
constructor(bytes) {
if (isOverridingTarget(bytes)) {
override(bytes)
}
}
} Therefore, it's crucial to take measures to prevent such kinds of malicious behavior. To avoid these things, we'd better work with trustable environment, dependencies, etc. |
You are right, that's also why I try to minimize dependencies on external libraries and Lumos makes this easy, thanks again @homura 🙏 |
As per title, I have encountered the following issue: Lumos internally uses a deprecated types validator that doesn't validate ImmutableJS types.
In the current version of Lumos Utils I have extensively used ImmutableJS's Records for defining types implementing the Interfaces defines by Lumos such a Scripts, OutPoint... but then when I try to use them, often I encounter deprecated types validators flagging them as not valid.
For example, let's say I implement the
Script
interface with:Lumos errors out with:
This is due to Lumos Toolkit's
ValidateScript
that, while being marked as deprecate, it's still used to validate the Script parameter:So while
I8Script
correctly implements theScript
interface (as it correctly exporse the fieldscodeHash
,hashType
andargs
), it's flagged as invalid by this deprecated validator.Should I report all the errors of this kind that I encounter and ask them to be fixed?
As usual I'm asking here since GitHub issues are SEO friendly and very likely in the future there will be other L1 developers wondering the same 😉
Phroi
The text was updated successfully, but these errors were encountered: