Add `generate-types` crate #63

JesseAbram · 2024-03-25T21:49:26Z

Adds a generate type crate that can help generate config and aux types

vercel · 2024-03-25T21:49:31Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
programs	❌ Failed (Inspect)			Mar 26, 2024 3:09pm

HCastano

Okay so I see the intention in this PR, but I think we can improve this. My high level
suggestions here:

Let's get rid of the generate-types crate. Instead if we want to show people in this
repo how to get the type definitions we can: add an example Rust code snippted to the
README, add a doc-test to the examples, or a normal test in the examples.

Let's add the actual generation to the test-cli in the Core repo (schemar annotations
are fine here). There we can add CLI options for outputing the JSON schema formatted
data, or hex-bytes depending on what the user prefers.

This should give us a more robust and usable solution that can be used across all
programs, not just one specific example program.

HCastano · 2024-03-27T01:27:48Z

generate-types/device_key_proxy_aux_data_type.txt

We don't need to commit generated artifacts. Anybody that needs them can just run the code and get this themselves

ya but this is an example so people can see

HCastano · 2024-03-27T01:29:36Z

generate-types/src/main.rs

+ fs::write(
+ "./device_key_proxy_config_type.txt",
+ format!(
+ "{:?}",
+ serde_json::to_vec(&schema_config_device_key_proxy)
+ .expect("error converting user config for device key proxy")
+ ),
+ )


We shouldn't be dumping bytes to a file.

Instead we should leave the JSON Schema object in tact, and then consumers can decide how to use that data themselves. One of those options can be transforming in to bytes (e.g using something like xxd, or another Rust program).

the bytes are the format we want to actually post on chain so people can copy paste them if they need or feed them into a script

HCastano · 2024-03-27T01:34:37Z

generate-types/src/main.rs

+ fs::write(
+ "./device_key_proxy_config_type.txt",
+ format!(
+ "{:?}",
+ serde_json::to_vec(&schema_config_device_key_proxy)
+ .expect("error converting user config for device key proxy")
+ ),
+ )
+ .expect("Failed to write to device key proxy config");


To my comment above, I would do something like this instead

Suggested change

fs::write(

"./device_key_proxy_config_type.txt",

format!(

"{:?}",

serde_json::to_vec(&schema_config_device_key_proxy)

.expect("error converting user config for device key proxy")

),

)

.expect("Failed to write to device key proxy config");

println!(

"{}",

serde_json::to_string_pretty(&schema_config_device_key_proxy)

.expect("error converting user config for device key proxy")

);

JesseAbram · 2024-03-27T14:27:15Z

Okay so I see the intention in this PR, but I think we can improve this. My high level suggestions here:

Let's get rid of the generate-types crate. Instead if we want to show people in this repo how to get the type definitions we can: add an example Rust code snippted to the README, add a doc-test to the examples, or a normal test in the examples.

Let's add the actual generation to the test-cli in the Core repo (schemar annotations are fine here). There we can add CLI options for outputing the JSON schema formatted data, or hex-bytes depending on what the user prefers.

This should give us a more robust and usable solution that can be used across all programs, not just one specific example program.

but the test cli does only feeds in the compiled wasm (which requires the serialized types at chain upload time) you can't get the needed types from the bytecode. As well I agree this is not perfect, the whole flow kinda sucks. We should have a package you can pull down to write a program and autogenerate the template code, that should also have a generate types functions, that I think is doable but takes a lot of steps. IMO this is a good incremental step before that, which tests out the thesis and moves us along

HCastano

but the test cli does only feeds in the compiled wasm (which requires the serialized
types at chain upload time) you can't get the needed types from the bytecode.

Good point. I guess this is something we can look into improving down the line.

As well I agree this is not perfect, the whole flow kinda sucks. We should have a
package you can pull down to write a program and autogenerate the template code, that
should also have a generate types functions, that I think is doable but takes a lot of
steps. IMO this is a good incremental step before that, which tests out the thesis and
moves us along

I don't think we should be shipping a crate for this very specific example. We can test
the thesis in a better way.

We're better off showing people how to do this themseleves for any program using a
doctests and/or an example in the README.

We can still include some portion of the byte-string to show people what is expected
on-chain, but I still wouldn't include the whole set of bytes (which can change at any
time anyways if fields change).

I've included a rough suggestion on how to do this. I can also make a PR into this PR if
you want me to flesh this out further.

ameba23 · 2024-03-27T21:03:59Z

I haven't got around to looking at this in detail but it looks good.

One thing we could do to encourage people to use this is add schemars to the Cargo.toml of the cargo-generate template: https://github.com/entropyxyz/programs/tree/master/templates/basic-template

Possibly we could also include the function there from generate-types/src/main.rs so that people can build the schemas right out of the box with their own program. But im not sure if we could have a main fn there without it meddling with building the program itself.

It would be cool if there was some way to make the generic-types crate more generic - as in have a way of specifying what program you want it to build from without manually changing the code. But i can't think of a way to do that.

JesseAbram · 2024-03-27T21:22:15Z

I haven't got around to looking at this in detail but it looks good.

One thing we could do to encourage people to use this is add schemars to the Cargo.toml of the cargo-generate template: https://github.com/entropyxyz/programs/tree/master/templates/basic-template

Possibly we could also include the function there from generate-types/src/main.rs so that people can build the schemas right out of the box with their own program. But im not sure if we could have a main fn there without it meddling with building the program itself.

It would be cool if there was some way to make the generic-types crate more generic - as in have a way of specifying what program you want it to build from without manually changing the code. But i can't think of a way to do that.

totally agreed, I know this is only a half step, maybe a macro to do it automatically or something, but either way dev ex here is way lacking, in fact this repo should not be used at all to make programs, there should be a cargo init and it have a template with this, not exactly sure, for me this is a half measure until we figure out a full devex plan

restructured

ameba23 · 2024-04-02T14:45:55Z

templates/basic-template/Cargo.toml

-
-[dependencies]
-entropy-programs-core={ git="https://github.com/entropyxyz/programs.git", tag="v0.8.0" }
+[workspace]


I would maybe keep the program itself in the top-level of the workspace - eg:

[workspace] members = [".", "generate-types"] [package] name = "{{project-name}}" version = "0.1.0"

etc.

This would also mean there would be just one readme - not one for the workspace and one for the program itself.

ehhhh idk, this can grow and Im just going off my recollections of eth dev, I liked you know tests and contracts being in separate repos, Im ok getting out voted here I just idk feels icky

@JesseAbram I'm with Peg here, I would prefer the program at the top level

ok bet I still disagree but will do

ameba23 · 2024-04-02T14:51:37Z

templates/basic-template/program/src/lib.rs

+#[cfg_attr(feature = "std", derive(schemars::JsonSchema))]
+#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
+pub struct AuxData {
+}


ameba23

Looks great - assuming generating schemas for a template works - i haven't tried it.

As i said in other comment, i would keep the program itself top-level, and have just one readme, which can include the instructions for generating types. I guess we can put more details about what you should actually do with these schemas when you want to publish a program in a separate PR once its possible to do that from the SDK or some other programs build tool. I guess eventually we'll have something which compiles the program, generates the schemas and bundles it all into a set_program extrinsic.

JesseAbram · 2024-04-02T17:00:34Z

Looks great - assuming generating schemas for a template works - i haven't tried it.

I have lol, super unfun to test

As i said in other comment, i would keep the program itself top-level, and have just one readme, which can include the instructions for generating types. I guess we can put more details about what you should actually do with these schemas when you want to publish a program in a separate PR once its possible to do that from the SDK or some other programs build tool. I guess eventually we'll have something which compiles the program, generates the schemas and bundles it all into a set_program extrinsic.

Pretty much what I was thinking, the generate can be its whole own tool that can take someone from cargo generate to deploy

HCastano

Main change I'd want here is the workspace this.

I like this approach better than the last one, specifically that it ties the type generation to the program a bit closer.

The workflow could still be improved, but I'm okay kicking that down the road a bit further.

HCastano · 2024-04-03T20:41:35Z

templates/basic-template/program/Cargo.toml

+license = "Unlicense"
+edition = "2021"
+
+# This is required to compile programs to a wasm module


We need to update the comment to specify that the Wasm required crate-type is cdylib and rlib is used for library-like uses

HCastano · 2024-04-03T20:41:44Z

examples/device-key-proxy/Cargo.toml

@@ -11,7 +11,7 @@ edition = "2021"

 # This is required to compile programs to a wasm module
 [lib]
-crate-type = ["cdylib"]
+crate-type = ["cdylib", "rlib"]


Same thing here

HCastano · 2024-04-03T20:43:32Z

templates/basic-template/Cargo.toml

-
-[dependencies]
-entropy-programs-core={ git="https://github.com/entropyxyz/programs.git", tag="v0.8.0" }
+[workspace]


@JesseAbram I'm with Peg here, I would prefer the program at the top level

HCastano · 2024-04-03T20:52:25Z

templates/basic-template/generate-types/src/main.rs

+fn main() {
+ let schema_config = schema_for!(UserConfig);
+ fs::write(
+ "./config_type.txt",


This should be in the .gitignore

HCastano · 2024-04-03T20:52:31Z

templates/basic-template/generate-types/src/main.rs

+
+ let schema_aux_data = schema_for!(AuxData);
+ fs::write(
+ "./aux_data_type.txt",


HCastano · 2024-04-03T20:53:16Z

templates/basic-template/generate-types/src/main.rs

I would look into attaching a name to the generated files as well, like {{ project_name }}_serialized_configuration_data

HCastano · 2024-04-03T20:54:03Z

templates/basic-template/program/Cargo.toml

+crate-type = ["cdylib", "rlib"]
+
+[dependencies]
+entropy-programs-core={ git="https://github.com/entropyxyz/programs.git", tag="v0.8.0" }


We should probably release this to crates.io at some point 😅

templates/basic-template/generate-types/Cargo.toml

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

add generate types crate

7438801

vercel bot had a problem deploying to Preview March 25, 2024 22:03 Failure

generate types

bdd4918

vercel bot had a problem deploying to Preview March 26, 2024 15:09 Failure

JesseAbram marked this pull request as ready for review March 26, 2024 15:43

JesseAbram requested review from ameba23, HCastano and frankiebee March 26, 2024 15:43

HCastano changed the title ~~add generate types crate~~ Add generate-types crate Mar 27, 2024

HCastano requested changes Mar 27, 2024

View reviewed changes

HCastano previously requested changes Mar 27, 2024

View reviewed changes

JesseAbram added 6 commits April 1, 2024 12:57

add userconfig and aux data config to template

dd4848e

add generate types to cargo generate

5ccbc76

add better template

8db4010

clean

ac728ff

readme

c7d73c2

readme

139af9d

JesseAbram requested a review from HCastano April 1, 2024 18:20

workspace fix

c8bfe9a

ameba23 reviewed Apr 2, 2024

View reviewed changes

ameba23 approved these changes Apr 2, 2024

View reviewed changes

HCastano approved these changes Apr 3, 2024

View reviewed changes

Apply suggestions from code review

37bc13b

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

JesseAbram added 2 commits April 4, 2024 10:46

move programs to root of basic template

117a7e7

fixes

4885fa7

JesseAbram merged commit db7ecd3 into master Apr 4, 2024
2 checks passed

HCastano deleted the generate-types branch April 5, 2024 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `generate-types` crate #63

Add `generate-types` crate #63

JesseAbram commented Mar 25, 2024

vercel bot commented Mar 25, 2024 •

edited

Loading

HCastano left a comment

HCastano Mar 27, 2024

JesseAbram Mar 27, 2024

HCastano Mar 27, 2024

JesseAbram Mar 27, 2024

HCastano Mar 27, 2024

JesseAbram commented Mar 27, 2024

HCastano left a comment

ameba23 commented Mar 27, 2024 •

edited

Loading

JesseAbram commented Mar 27, 2024

ameba23 Apr 2, 2024 •

edited

Loading

JesseAbram Apr 2, 2024

HCastano Apr 3, 2024

JesseAbram Apr 3, 2024

ameba23 Apr 2, 2024

ameba23 left a comment •

edited

Loading

JesseAbram commented Apr 2, 2024

HCastano left a comment

HCastano Apr 3, 2024

HCastano Apr 3, 2024

HCastano Apr 3, 2024

HCastano Apr 3, 2024

HCastano Apr 3, 2024

HCastano Apr 3, 2024

HCastano Apr 3, 2024

Add generate-types crate #63

Add generate-types crate #63

Conversation

JesseAbram commented Mar 25, 2024

vercel bot commented Mar 25, 2024 • edited Loading

HCastano left a 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

JesseAbram commented Mar 27, 2024

HCastano left a comment

Choose a reason for hiding this comment

ameba23 commented Mar 27, 2024 • edited Loading

JesseAbram commented Mar 27, 2024

ameba23 Apr 2, 2024 • 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

ameba23 left a comment • edited Loading

Choose a reason for hiding this comment

JesseAbram commented Apr 2, 2024

HCastano left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add `generate-types` crate #63

Add `generate-types` crate #63

vercel bot commented Mar 25, 2024 •

edited

Loading

ameba23 commented Mar 27, 2024 •

edited

Loading

ameba23 Apr 2, 2024 •

edited

Loading

ameba23 left a comment •

edited

Loading