Skip to content
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

Upgrade rpctypegen to new Borsh Schema format #2242

Closed
MaksymZavershynskyi opened this issue Mar 10, 2020 · 5 comments
Closed

Upgrade rpctypegen to new Borsh Schema format #2242

MaksymZavershynskyi opened this issue Mar 10, 2020 · 5 comments
Labels
A-RPC Area: rpc P-high Priority: High

Comments

@MaksymZavershynskyi
Copy link
Contributor

rpctypegen generates Borsh schema in TS format for all RPC errors.

Now that we have implemented BorshSchema macro in near/borsh#57 and near/borsh#56 we can use #[derive(BorshSchema)] to generate schema for RPC errors and reduce our code base. We would also need to do two more things:

  • Switch nearlib to new Borsh Schema format that is slightly different from the one introduced by rpctypegen (this can be easily done after we upgrade borsh TS/JS to new Borsh Schema format, see: Consolidate and update Borsh for TS/JS near-api-js#252)
  • Add a single executable in nearcore/tools that has a hardcoded list of error types. The executable would iterate over the error types and for each of them generate Borsh Schema using BorshSchema trait it then would dump the schema into a file that we can hardcode in nearlib.
@frol
Copy link
Collaborator

frol commented Mar 19, 2020

Borsh schema does not necessarily match the JSON schema (e.g. some values can be base64-encoded, some fields/enum variants are renamed, etc). Unfortunately, I don't think it is feasible to try to fit Borsh schema for JSON schema. Please, reopen if you think otherwise.

@frol frol closed this as completed Mar 19, 2020
@MaksymZavershynskyi
Copy link
Contributor Author

Borsh schema is not Borsh-specific. We call it Borsh schema because it is defined in Borsh library but it can be used by many serializers. Our current schema generated by rpctypegen has nothing specific to JSON either. The point of the proposed changes is to reconcile the schemas that we currently use in nearlib. I am reopening this issue. However, I don't think it is blocking for the Mainnet. Therefore I am removing it from the Mainnet release and moving it to IceBox. I am also doing the same with the Epic.

@frol
Copy link
Collaborator

frol commented Mar 30, 2020

Nice! Then, for the future context, we need to make sure that whatever schema-generation we use should respect the serialization annotations (currently, we extensively use serde renamings, enum representations, and skips). There are OpenAPI spec generators for serde out there for inspiration: https://docs.rs/rweb/0.4.3/rweb/openapi/index.html

@frol frol unassigned lexfrl Apr 1, 2020
@stale
Copy link

stale bot commented Jul 1, 2021

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months.
It will be closed in 7 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the S-stale label Jul 1, 2021
@bowenwang1996
Copy link
Collaborator

looks like a duplicate of #1972

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-RPC Area: rpc P-high Priority: High
Projects
None yet
Development

No branches or pull requests

4 participants