feat: `near_abi!` macro #831

itegulov · 2022-06-03T06:59:34Z

Basically just the macro and ABI models without all other extra stuff from daniyar/aci like cargo extension, cross contract calls etc.

We have not decided in what branch this should live in, so I am targeting master for now.

examples/abi/README.md

examples/abi/res/abi.json

austinabell · 2022-06-06T03:17:53Z

near-sdk-macros/src/core_impl/abi/abi_visitor.rs

+            const _: () = {
+                #[no_mangle]
+                #[cfg(not(target_arch = "wasm32"))]
+                pub fn __near_abi() -> near_sdk::__private::AbiRoot {


Note that this will fail to compile when there are multiple impl sections with near_bindgen, right? Will double check tomorrow

Hmm, it should not as long as all impl sections are wrapped in the same near_abi! call.

didn't realize that case is handled. It limits the ability to have impl sections in diff modules, though

True. I am unsure how to handle this though... I have tried depending on modules like this:

#[near_abi] mod abi { #[path = "utility.rs"] mod abi_utility; #[near_bindgen] #[derive(Default, BorshDeserialize, BorshSerialize)] pub struct Contract {} ... }

But apparently including non-inline modules in proc macro input is an unstable feature.

I was thinking I could maybe create a global state that would be modified by all #[near_abi] annotations and then add another macro that would output the accumulated state into one ABI structure. The problem is that I think there is no way to enforce macro execution order. Do you have any ideas I could explore?

Another very vague idea I have considered is to generate a bunch of symbols (e.g. __near_abi_lib_rs, __near_abi_utility_rs etc) and then somehow get a list of them from cargo extension and call all of them. The main problem here is that I cannot find if there is a rustc/cargo API that would allow me to list all symbols from a crate.

near-sdk-macros/src/core_impl/abi/mod.rs

near-sdk/src/private/abi.rs

austinabell · 2022-06-06T03:28:31Z

near-sdk/src/promise.rs

+    }
+
+    fn json_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema {
+        T::json_schema(gen)


Note that right now nothing restricts a developer from having the promise return a different schema type than T.

Yeah I also thought about this. Returning different things feels like a strange pattern to me, but I guess since we are not discouraging this pattern I should not presume that there is only one possible return type. I could represent this as an equivalent of Either<Any, T> or just simply Any. WDYT?

What about a Result<T> since this is looks to be fallible and we're expecting a type T. Maybe not for this function, but another one for that can return a Result

Well, it is not really fallible as it is non-restrictive to just T, so I don't think Result suits unless I am missing something here

I think that making the assumption as one type is fine, it would nudge developers in the correct pattern, but I'm just making the comment so we don't ignore the cases where maybe someone might return different types that serialize as the same thing. Can't immediately think of an implication that would be an issue, though

examples/abi/src/lib.rs

near-sdk-macros/src/core_impl/abi/abi_visitor.rs

austinabell · 2022-06-06T03:36:26Z

We have not decided in what branch this should live in, so I am targeting master for now.

Hmm, yeah either master becomes the 5.0 path and we backport non-breaking changes to some 4.x branch, or we put these changes into a branch that targets the main branch and only merge those commits in when we are ready to release the next version. Seems like maybe the first option might be cleaner, and then we can just cherry-pick commits when we want to release any 4.x branch. Might be more work, but seems okay

itegulov · 2022-06-06T07:10:18Z

We have not decided in what branch this should live in, so I am targeting master for now.

Hmm, yeah either master becomes the 5.0 path and we backport non-breaking changes to some 4.x branch, or we put these changes into a branch that targets the main branch and only merge those commits in when we are ready to release the next version. Seems like maybe the first option might be cleaner, and then we can just cherry-pick commits when we want to release any 4.x branch. Might be more work, but seems okay

Yeah sounds good to me.

Do we want to put this macro and ABI models under unstable for now?

ChaoticTempest · 2022-06-06T21:49:44Z

near-sdk/src/promise.rs

+    }
+
+    fn json_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema {
+        T::json_schema(gen)


What about a Result<T> since this is looks to be fallible and we're expecting a type T. Maybe not for this function, but another one for that can return a Result

near-sdk/src/private/abi.rs

austinabell · 2022-06-07T00:00:37Z

Do we want to put this macro and ABI models under unstable for now?

Yes, please.

near-sdk/src/lib.rs

austinabell · 2022-06-14T15:37:07Z

near-sdk-macros/src/core_impl/abi/abi_generator.rs

+            .attr_signature_info
+            .args
+            .iter()
+            .filter(|arg| matches!(arg.bindgen_ty, BindgenArgType::CallbackArg))


This handles callback, but does not handle CallbackResultArg. Am I missing something with this?

Oh, yeah, you are right. I took it from here: https://github.com/near/near-sdk-rs/blob/master/near-sdk-macros/src/core_impl/metadata/metadata_generator.rs#L69, so I guess the previous metadata implementation is not accounting for this case either. Should I fix it in a separate PR or do you think it is inconsequential at this point?

I guess it depends what we do with metadata, but my assumption is that it would be deprecated.

As for callback result, will this not have to be handled more specifically than ccfba2e (#831) to ignore the wrapping Result type in the schema?

Yep, good catch. I have refactored the entire argument processing flow because it was getting out of hand, this case should be handled now.

examples/abi/Cargo.toml

examples/abi/src/lib.rs

near-sdk/Cargo.toml

austinabell · 2022-06-15T00:03:46Z

near-sdk/src/lib.rs

+#[doc(hidden)]
+pub use schemars;


let's just remove this altogether, re-exporting things like this is generally a bad pattern and the others would be purged if it wasn't very breaking of a change

Noting again this would be ideal not to export and require that we keep this API stable

Gotcha, moved behind __private for now since I can't figure out how to remove it

itegulov · 2022-06-21T10:48:49Z

@austinabell so after our last conversation I am not sure if it makes sense to leave abi under unstable feature now. My vision is that there is going to be an abi feature that controls whether near_bindgen generates ABI symbols. So I don't know if it makes sense to bundle it up with unstable, should I create something like abi_unstable for it for now?

austinabell · 2022-06-21T16:39:14Z

@austinabell so after our last conversation I am not sure if it makes sense to leave abi under unstable feature now. My vision is that there is going to be an abi feature that controls whether near_bindgen generates ABI symbols. So I don't know if it makes sense to bundle it up with unstable, should I create something like abi_unstable for it for now?

no, if we are adding a feature, it should be something long-term because removing is unergonomic for anyone who specified it previously (breaking change). If you wanted it to be abi_unstable, instead you can just require someone enable the abi feature and unstable. Up to you which direction this goes into between putting it under abi, unstable, or both. My guess is abi is probably the best, we just need to make sure that name is stable and set before releasing the next SDK version

itegulov · 2022-06-22T04:20:29Z

no, if we are adding a feature, it should be something long-term because removing is unergonomic for anyone who specified it previously (breaking change). If you wanted it to be abi_unstable, instead you can just require someone enable the abi feature and unstable. Up to you which direction this goes into between putting it under abi, unstable, or both. My guess is abi is probably the best, we just need to make sure that name is stable and set before releasing the next SDK version

Makes sense, I am putting everything behind abi then.

austinabell

Looks generally good! Just a few small comments and should be good

austinabell · 2022-06-22T16:50:47Z

near-sdk-macros/src/core_impl/abi/abi_generator.rs

+    /// If args are serialized with Borsh it will not include `#[derive(borsh::BorshSchema)]`.
+    pub fn abi_struct(&self) -> TokenStream2 {
+        let function_name_str = self.attr_signature_info.ident.to_string();
+        let is_view = matches!(&self.attr_signature_info.method_type, &MethodType::View);


Note that we should start thinking of ways to avoid false positives for this. The MethodType::View is just used for codegen, but can't reliably indicate a function is view externally. Wonder if analysis of the wasm binary to see if any non-view functions are used (state write, promises) will be necessary.

This is probably fine for now, though. We should open an issue for this before or after this comes in

Good point, created an issue: #850

near-sdk-macros/src/core_impl/abi/abi_generator.rs

near-sdk-macros/src/core_impl/abi/abi_visitor.rs

near-sdk/src/private/abi.rs

austinabell

sorry, one thing I overlooked and then good

austinabell · 2022-06-23T19:07:12Z

near-sdk-macros/src/core_impl/abi/abi_visitor.rs

+            const _: () = {
+                #[no_mangle]
+                pub fn #near_abi_symbol() -> near_sdk::__private::AbiRoot {
+                    let mut gen = schemars::gen::SchemaGenerator::default();


this should use the re-export from the SDK. Ideally behind a private module. This forces a developer to import schemars even if they don't use

This should be near_sdk::__private::schemars:: or near_sdk::schemars:: if that re-export is not moved from other comment

austinabell · 2022-06-23T19:08:42Z

near-sdk/src/lib.rs

+#[doc(hidden)]
+pub use schemars;


Noting again this would be ideal not to export and require that we keep this API stable

itegulov added 3 commits June 3, 2022 15:55

add abi models and schemars dependency

30fd61c

implement near_abi macro

4f017dd

add abi example

c2fe70b

itegulov requested review from austinabell and ChaoticTempest as code owners June 3, 2022 06:59

clippy

05af560

austinabell reviewed Jun 6, 2022

View reviewed changes

itegulov added 2 commits June 6, 2022 16:22

use cargo near abi instead

f3b6b35

add serde default for all optional fields

9abc6e5

itegulov added 4 commits June 6, 2022 17:17

ignore empty params as well

eb48446

update example

ac4cfbc

add parameter names to ABI

c1c97d0

rename ABI models and move name inside AbiParameter

28cb425

ChaoticTempest reviewed Jun 6, 2022

View reviewed changes

hide macro and abi models behind unstable

8c6f2ea

itegulov commented Jun 8, 2022

View reviewed changes

near-sdk/src/lib.rs Outdated Show resolved Hide resolved

itegulov added 6 commits June 8, 2022 11:42

re-generate wasm files

e5f5f66

make near_abi an attribute macro

88e9234

clippy

b0580c9

add unstable feature for near-sdk-macros

6425c76

rename metainfo to metadata

8a1fea1

empty

428b1d5

austinabell reviewed Jun 14, 2022

View reviewed changes

austinabell reviewed Jun 15, 2022

View reviewed changes

itegulov added 3 commits June 15, 2022 13:50

handle result callbacks for ABI

ccfba2e

improve abi example

a2f7b1d

make schemars an optional dependency

ddc6824

itegulov force-pushed the daniyar/abi-macro branch from bc59ec3 to ddc6824 Compare June 16, 2022 01:09

itegulov added 7 commits June 20, 2022 14:05

implement JsonSchema for a few more types

43d991e

handle init methods in a special way

4c037db

fix optional dependencies

2be9534

derive JsonSchema for storage types

0570f78

generate multiple symbols

24350f7

get rid of type registry

9014d6d

refactor function generation & support for result callbacks

6779174

itegulov added 5 commits June 22, 2022 10:34

rebuild examples

7a9bae0

add is_private & is_payable, make those optional

71713ac

decouple abi feature from unstable

51aed1f

fix test-contract-defi

49565dd

Merge remote-tracking branch 'origin/master' into daniyar/abi-macro

efce965

austinabell reviewed Jun 22, 2022

View reviewed changes

address comments

3206c70

austinabell approved these changes Jun 23, 2022

View reviewed changes

austinabell suggested changes Jun 23, 2022

View reviewed changes

austinabell mentioned this pull request Jun 23, 2022

feat: MVP of ABI generation near/cargo-near#2

Merged

itegulov added 2 commits June 24, 2022 05:21

use schemars from near sdk

4203424

hide schemars behind __private

e6b31bb

austinabell approved these changes Jun 23, 2022

View reviewed changes

rebuild examples

37aaa23

itegulov merged commit 9ab6df8 into master Jun 23, 2022

itegulov deleted the daniyar/abi-macro branch June 23, 2022 23:51

austinabell mentioned this pull request Jun 24, 2022

Generate docs from the functions #851

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `near_abi!` macro #831

feat: `near_abi!` macro #831

itegulov commented Jun 3, 2022

austinabell Jun 6, 2022

itegulov Jun 6, 2022

austinabell Jun 9, 2022

itegulov Jun 13, 2022 •

edited

Loading

itegulov Jun 13, 2022

austinabell Jun 6, 2022

itegulov Jun 6, 2022

ChaoticTempest Jun 6, 2022

itegulov Jun 8, 2022

austinabell Jun 9, 2022

austinabell commented Jun 6, 2022 •

edited

Loading

itegulov commented Jun 6, 2022

ChaoticTempest Jun 6, 2022

austinabell commented Jun 7, 2022

austinabell Jun 14, 2022

itegulov Jun 15, 2022

austinabell Jun 15, 2022

itegulov Jun 21, 2022

austinabell Jun 15, 2022

austinabell Jun 23, 2022

itegulov Jun 23, 2022

itegulov commented Jun 21, 2022

austinabell commented Jun 21, 2022 •

edited

Loading

itegulov commented Jun 22, 2022

austinabell left a comment

austinabell Jun 22, 2022

itegulov Jun 22, 2022

austinabell left a comment

austinabell Jun 23, 2022

austinabell Jun 23, 2022

feat: near_abi! macro #831

feat: near_abi! macro #831

Conversation

itegulov commented Jun 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itegulov Jun 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

austinabell commented Jun 6, 2022 • edited Loading

itegulov commented Jun 6, 2022

Choose a reason for hiding this comment

austinabell commented Jun 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itegulov commented Jun 21, 2022

austinabell commented Jun 21, 2022 • edited Loading

itegulov commented Jun 22, 2022

austinabell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

austinabell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat: `near_abi!` macro #831

feat: `near_abi!` macro #831

itegulov Jun 13, 2022 •

edited

Loading

austinabell commented Jun 6, 2022 •

edited

Loading

austinabell commented Jun 21, 2022 •

edited

Loading