-
Notifications
You must be signed in to change notification settings - Fork 14
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
Feature support borsch deserialisation #53
Conversation
🦋 Changeset detectedLatest commit: 982b45a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
v: Buffer.from(c.valueBase64, "base64"), | ||
})); | ||
|
||
const authorToPostId = Object.fromEntries( |
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 @Tguntenaar ! I actually think it would be highly beneficial to be able to use complex schema definition in the implementation, so that you can take it almost directly from your smart contract code and plug here. Do I get it right that this will essentially eliminate the need for this code that essentially implements it manually?
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.
Hi @pkudinov, Yes that is right! Instead of fishing the post_id
and the author
out of the buffer by hand we can deserialize the entire state in one time and read it as a object.
I will also add the bosher
package to facilitate reproducing the format of the contract state more easily.
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.
@pkudinov As I was reconstructing the borsh schema. I found out that in this case it is actually easier to calculate it by hand because the post_id
is in the beginning of the structure. While the post structure is quiet complicated to rebuild as show below.
However for other use cases including borsher
is still usefull for deserialising so I included it in the primitives packages.
See here
const postSnapshot = BorshSchema.Struct({
// Reconstruction this schema is more compilcated than counting which bytes to skip if the property is in the beginning
});
const addPostSchema = BorshSchema.Struct({
id: BorshSchema.u64,
author_id: BorshSchema.String,
likes: BorshSchema.HashSet(
BorshSchema.Struct({
author_id: BorshSchema.String,
timestamp: BorshSchema.u64,
})
),
snapshot: postSnapshot,
snapshot_history: BorshSchema.Vec(postSnapshot),
});
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.
Very cool work!
My understanding is that the benefit is that you can serialize with borsch for a more compact storage, and deserialize it with a known schema for easy access of underlying features. And adding it here allows that usage without redefining code. Is that correct?
I believe I already know what changes are necessary but I'd be happy to work with you on ensuring QueryApi properly presents these borsch utilities for use.
I'll see if I can try making the two work together locally to validate. If that doesn't seem to work out, we can merge and I'll try again but with the actual package.
@darunrs a dev would still be required to imitate the state structure from the contract. Borsh-js makes it possible to deserialise based on a given schema. Borscher makes it easier to construct that structure. I think testing locally is a solid step to validate compatibility. Let me know what changes you had in mind. |
Hey @Tguntenaar I am going to try and see if I can add borsch to the frontend and backend of queryapi by linking it to the locally pulled PR branch, and then verify that I can use the method you added there. If it can recognize it, then I know my additions worked. Then, I you can provide me a block and indexer code which uses borsch and I can see if that runs, to verify these changes are sufficient too. It's possible that integrating locally like that is too time consuming or difficult. In which case, we'll just have to merge this, and I'll try again with the actual package version with the update. |
Using
borsh-js
will simplify deserialization of the statechanges significantly. As mentioned here: near/queryapi#593.Idea 💡
@pkudinov The test includes some code from an indexer from devhub. It doesn't show the full power of using borsh because there is complex schema used. We could also add bosher to simplify creating schema's.
Comment 📝
I changed the StateChangeWithCauseView type. If the type property of
StateChangeWithCauseView
is equal to data_update, the value property also includes a change property. However at least that is what I read somewhere but I can't find it back in the docs. Nevertheless the test works. 👍🏻I also added the changeset.