-
Notifications
You must be signed in to change notification settings - Fork 44
fix(wasm-sdk): include groups and tokens in data contract serialization #2794
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
b6cb602
90451af
851bd5f
19ecdda
999d8d6
ce71548
64d590a
74a10aa
fd435e3
e58c315
1f5637a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -295,11 +295,18 @@ impl IdentityWasm { | |
| // } | ||
|
|
||
| #[wasm_bindgen] | ||
| pub struct DataContractWasm(DataContract); | ||
| pub struct DataContractWasm(DataContract, &'static PlatformVersion); | ||
|
|
||
| impl DataContractWasm { | ||
| pub fn new(data_contract: DataContract, platform_version: &'static PlatformVersion) -> Self { | ||
| Self(data_contract, platform_version) | ||
| } | ||
| } | ||
thephez marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| impl From<DataContract> for DataContractWasm { | ||
| fn from(value: DataContract) -> Self { | ||
| Self(value) | ||
| // Use first version as fallback for backward compatibility | ||
| Self(value, PlatformVersion::first()) | ||
|
||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
|
|
@@ -311,9 +318,7 @@ impl DataContractWasm { | |
|
|
||
| #[wasm_bindgen(js_name=toJSON)] | ||
| pub fn to_json(&self) -> Result<JsValue, WasmSdkError> { | ||
| let platform_version = PlatformVersion::first(); | ||
|
|
||
| let json = self.0.to_json(platform_version).map_err(|e| { | ||
| let json = self.0.to_json(self.1).map_err(|e| { | ||
| WasmSdkError::serialization(format!( | ||
| "failed to convert data contract convert to json: {}", | ||
| e | ||
|
|
||
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 went through DPP methods. Actually, I think your idea to pass the platfrom version to the constructor was the best option we have. But all constructors should always require platform version then. We shouldn't create with default or first version in any case. I'm sorry, that I wasn't clear enough in my first comment, so you reverted half correct code.
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.
What about fd435e3?
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.
No there actually is a trait FromWithVersion that should be used instead