extract ABI primitives from near-sdk #1
Conversation
There was a problem hiding this comment.
Yet to look at the changes, but one thing I want to point out is that I would personally prefer this repo to be something more generic like near-abi-schema that will at some point contain meta schema, README explaining what ABI is and all "official" model implementations (crate + npm package for now). What do you think?
|
Yeah, we can do that. I'm just thinking this will be the repo that contains the official rust implementation of said schema. We can go ahead to create the |
src/lib.rs
Outdated
| /// Core ABI information, with schema version. | ||
| #[cfg(feature = "versioned-entries")] | ||
| #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] | ||
| pub struct VersionedAbiEntry { | ||
| /// Semver of the ABI schema format. | ||
| abi_schema_version: String, | ||
| #[serde(flatten)] | ||
| pub abi: AbiEntry, | ||
| } |
There was a problem hiding this comment.
I like the idea of statically known versions, but I wonder if there is a better way to represent this. Have you thought about something like this (very roughly outlined)?
pub enum VersionedAbiEntry {
#[serde(rename = "0.1")]
V0_1(AbiEntryDeprecated),
#[serde(rename = "0.2")]
V0_2(AbiEntryDeprecated),
#[serde(rename = "1.0")]
V1_0(AbiEntry),
}And then change abi.json to look like this:
{
"metadata": {
"name": "abi",
"version": "0.1.0",
"authors": [
"Near Inc <hello@nearprotocol.com>"
]
},
"1.0": {
... // AbiEntry contents
}
}I never liked that abi.json has a field named abi, so maybe it is time to change it? :)
There was a problem hiding this comment.
What's this? ... Okay I like it, Picasso. 1
There was a problem hiding this comment.
The only thing I'd think about this, is that supporting the different types might be awkward to work with. Seems like the version should be based on the whole schema, rather than just the ABI section, maybe. Would match JSON schema more closely. Supporting this would be straightforward in Rust, but unclear how this would work well with JS/TS or other languages without enums like this
I was just thinking that it would be better to have all schema models in one place since they will probably be rather small and depend on the metaschema (TBD) in some way. Not a big deal either way, just expressing an opinion. |
| [dependencies] | ||
| borsh = { version = "0.9", features = ["const-generics"] } | ||
| serde = { version = "1", features = ["derive"] } | ||
| schemars = "0.8.8" |
There was a problem hiding this comment.
Noting the unfortunate circumstance that if someone didn't want any JSON schemas in their ABI, this dependency still gets pulled in to every project (including near-sdk if the abi feature is enabled). Nothing has to change here, just mentioning
There was a problem hiding this comment.
approving because none of my comments are blocking, but @itegulov might have some other comments
|
|
||
| for entry in entries { | ||
| if let Some(ref schema_version) = schema_version { | ||
| // should probably only disallow major version mismatch |
There was a problem hiding this comment.
In theory the version mismatch should not happen at all, since they are all pulled from the same crate. So not sure if it makes sense to bother with distinguishing major/minor versions.
Exports the ABI primitives from SDK.