Updated starknet_api_openrpc to support code autogeneration

[phklive]

Autogeneration of typings would be of real utility to the entirety of the ecosystem from the open-rpc specs of Starknet. Unfortunately the specs had wrongly been created ( Ex. title was used as description, results were optionals, etc...).
Hence I have updated as much as i could to correctly add titles, correct return types, correct return options, and others.

With this PR we can now use open-rpc-typings to create auto generated types for rust.

Previous behavior:
Code auto-generation was broken and outputed types with random titles.
Also, trace and write are have errors in them hence code auto-generation is not possible.
We will need to re-write them and fix bugs to enable it.

New behavior:
The auto generated code is correct and usable

IMO:
1st step - Make open-rpc functional with code auto-generation
2nd step - Improve this functional open-rpc
3rd step - Re-write and update to newer version ( api / trace / write )

How to use this pr:
git clone repo
npm install @open-rpc/typings
open-rpc-typing -d ./api/starknet_api_openrpc.json --output-rs .

---

This change is 

[ArielElp]

Reviewable status: 0 of 2 files reviewed, 9 unresolved discussions (waiting on @phklive)

---

api/starknet_api_openrpc.json line 19 at r1 (raw file):

                    "required": true,
                    "schema": {
                        "title": "Block id",

Are titles in schemas defined by a reference required? I think that if BLOCK_ID has a title then this reference doesn't require title duplication (same elsewhere)

---

api/starknet_api_openrpc.json line 19 at r1 (raw file):

                    "required": true,
                    "schema": {
                        "title": "Block id",

Do titles with spaces work with auto generations? e.g. in return results for some methods you used underscores rather than spaces, we should be consistent here

---

api/starknet_api_openrpc.json line 469 at r1 (raw file):

                "schema": {
                    "type": "array",
                    "title": "Field element",

CallResult?

Code quote:

Field element

---

api/starknet_api_openrpc.json line 778 at r1 (raw file):

            },
            "EMITTED_EVENT": {
                "title": "Emitted event",

Choosing this example but true for all other instances, do we go with EmittedEvent? I would guess spaces mess up generation

Code quote:

Emitted event

---

api/starknet_api_openrpc.json line 787 at r1 (raw file):

                    },
                    {
                        "title": "Event emission",

Not a very clear name, how about EventContext?

Code quote:

Event emission

---

api/starknet_api_openrpc.json line 820 at r1 (raw file):

                "allOf": [
                    {
                        "title": "Params",

I would expect generation tools to handle allOf types, have a separate title for every parts looks weird IMO. In any case, Params isn't saying much, I suggest "EventEmitter" or something of the sort.

Code quote:

Params

---

api/starknet_api_openrpc.json line 887 at r1 (raw file):

                        "type": "array",
                        "items": {
                            "title": "Keys",

does an array item need a title or can this be removed?

---

api/starknet_api_openrpc.json line 1339 at r1 (raw file):

                    {
                        "type": "object",
                        "title": "Params",

same allOf comment (is it needed for generation), plus I would avoid Params as a title

---

api/starknet_api_openrpc.json line 2731 at r1 (raw file):

                ]
            },
            "FEE_ESTIMATE": {

Missing required?

[phklive]

I will fix what need to be fixed, indeed names can be improved to be more explicit, some titles are required.

Reviewable status: 0 of 2 files reviewed, 9 unresolved discussions (waiting on @ArielElp)

---

api/starknet_api_openrpc.json line 19 at r1 (raw file):

Previously, ArielElp wrote…

Do titles with spaces work with auto generations? e.g. in return results for some methods you used underscores rather than spaces, we should be consistent here

Titles with spaces do work, I will update the starknet_ with spaces to be consistent.

---

api/starknet_api_openrpc.json line 19 at r1 (raw file):

Previously, ArielElp wrote…

Are titles in schemas defined by a reference required? I think that if BLOCK_ID has a title then this reference doesn't require title duplication (same elsewhere)

Unfortunately the name that is used is not the field that is considered by the open-rpc playground or the code auto-generation, hence without the "title" field the title field is missing in the open-rpc playground and titles are auto generated in the types hence creating unusable types.

As follows:

---

api/starknet_api_openrpc.json line 469 at r1 (raw file):

Previously, ArielElp wrote…

CallResult?

In the Starknet specs right now, starknet_call is returning an array of Field elements, as follows:

I have just added the title field to annotate and help with code-autogeneration and playground.

---

api/starknet_api_openrpc.json line 778 at r1 (raw file):

Previously, ArielElp wrote…

Choosing this example but true for all other instances, do we go with EmittedEvent? I would guess spaces mess up generation

Spaces or underscores do not mess up code-autogeneration hence i have chosen to use spaces as a rule in all titles, imo titles should be human readable.

---

api/starknet_api_openrpc.json line 787 at r1 (raw file):

Previously, ArielElp wrote…

Not a very clear name, how about EventContext?

Will change to event context 👍

---

api/starknet_api_openrpc.json line 820 at r1 (raw file):

Previously, ArielElp wrote…

I would expect generation tools to handle allOf types, have a separate title for every parts looks weird IMO. In any case, Params isn't saying much, I suggest "EventEmitter" or something of the sort.

Code-autogeneration doesn't handle allOf types, it generates types as follows starting with "AllOf":

---

api/starknet_api_openrpc.json line 887 at r1 (raw file):

Previously, ArielElp wrote…

does an array item need a title or can this be removed?

I will make sure they are absolutely necessary if not they will be removed, but through my experience they did help with code-autogeneration.

---

api/starknet_api_openrpc.json line 1339 at r1 (raw file):

Previously, ArielElp wrote…

same allOf comment (is it needed for generation), plus I would avoid Params as a title

title is required, what title would you recommend?

---

api/starknet_api_openrpc.json line 2731 at r1 (raw file):

Previously, ArielElp wrote…

Missing required?

Indeed.

[phklive]

Also, trace and write are have errors in them hence code auto-generation is not possible.
We will need to re-write them and fix bugs to enable it.

[phklive]

Also here are examples of the files auto-generated before and after pr:

before: https://gist.github.com/phklive/58a2cacb33eae9d0b66fbfcb1fd03b93

after: https://gist.github.com/phklive/1b81127fce118f30872850dda899ef8e

[ArielElp]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @drspacemn, @greged93, and @phklive)

[phklive]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @drspacemn, @greged93, and @phklive)

Great 🚀

[drspacemn]

Is there a way to avoid this generation?
https://gist.github.com/phklive/1b81127fce118f30872850dda899ef8e#file-index-rs-L935

[phklive]

Is there a way to avoid this generation? https://gist.github.com/phklive/1b81127fce118f30872850dda899ef8e#file-index-rs-L935

Hey, unfortunately this is the only name i wasn't able to fix, it's globally created from all the method names and there doesn't seem to be a place to add a global title that would enable this auto-generated placeholder to be correctly named.

I should have pointed it out earlier before you found it.

[ArielElp]

Reviewed 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @greged93 and @phklive)

[phklive]

Reviewed 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @greged93)

[greged93]

There seems to be an issue on the auto generation for DeprecatedContractClass and ContractClass:
both contain a field entry_points_by_type which during auto generation maps to the same type (EntryPointsByType). DeprecatedContractClass should have DeprecatedCairoEntryPoint for the type of entry_points_by_type and not SierraEntryPoint.
Links provided for clarity:

• Specs CONTRACT_CLASS
• Specs DEPRECATED_CONTRACT_CLASS
• Generated types

[Mirko-von-Leipzig]

Yeah honestly the codegen tool (for rust) really is not great..

One of our team members attempted his own version here which seems a bit more sane.

[phklive]

There seems to be an issue on the auto generation for DeprecatedContractClass and ContractClass: both contain a field entry_points_by_type which during auto generation maps to the same type (EntryPointsByType). DeprecatedContractClass should have DeprecatedCairoEntryPoint for the type of entry_points_by_type and not SierraEntryPoint. Links provided for clarity:

• Specs CONTRACT_CLASS
• Specs DEPRECATED_CONTRACT_CLASS
• Generated types

Indeed here the code auto-generation is confused because both use similar names, hence during generation it does not know how to use the type.

The iamgroot code-autogeneration tool makes the same error except reversed:

The potential fix that I found would be to rename the titles to make the distinguishable from "entry points by type" to "deprecated entry points by type" and "constructor" to "deprecated constructor", etc... Which will not change the underlying types in the Starknet specs but only disambiguate those parameters enabling code auto-generation and imo unchanged or clearer understanding by the users.

[Mirko-von-Leipzig]

@sergey-melnychuk btw ^^.

One solution: namespace these inner types within modules, which at least prevents this legal-spec-but-doesn't-work-with-bad-tools dance.

It does make me skeptical that this official tool works at all though. Feels like one would have to audit every codegen every time.

[sergey-melnychuk]

@Mirko-von-Leipzig

The iamgroot does not need such kind of fix because it simply works. The generated code actually works against real endpoints, including proxy mode.

This PR just tries to optimize the spec for specific tool, which in my opinion makes no sense, as it is clearly an indicator of a problem with a tool, not a spec.

UPDATE:
I don't want to brag, but generating meaningful code from an existing (not optimized for a specific tool) I personally consider as a solved problem. The iamgroot generates meaningful (and working!!!!11oneone) code for Starknet and Ethereum specs (slightly patched though).

UPDATE:
By "slightly patched" spec I mean actually de-anonimising a [DEPRECATED_]CONTRACT_CLASS_ENTRY_POINT types: sergey-melnychuk/iamgroot@f228044

[phklive]

I am confused by your response, I am obviously not trying to optimize the specs for any type of auto-generation tool, I am just making sure that it generates correct code every time ( playground, code-generation, etc... ) by adding correct labels and correcting errors, the only thing I optimize is our Starknet-specs.

I understand that you have built iamgroot and I am sure that it's a great tool and that you did a great work but here as you can see in my previous response your tool had outputed incorrect code, and unfortunately after my fix too, which the official tool didn't.

I think iamgroot is good but we wouldn't be able to say that "it simply works", no software "simply works" every time.

And we never talked about fixing your tool...

@Mirko-von-Leipzig

The iamgroot does not need such kind of fix because it simply works. The generated code actually works against real endpoints, including proxy mode.

This PR just tries to optimize the spec for specific tool, which in my opinion makes no sense, as it is clearly an indicator of a problem with a tool, not a spec.

UPDATE: I don't want to brag, but generating meaningful code from an existing (not optimized for a specific tool) I personally consider as a solved problem. The iamgroot generates meaningful (and working!!!!11oneone) code for Starknet and Ethereum specs (slightly patched though).

UPDATE: By "slightly patched" spec I mean actually de-anonimising a [DEPRECATED_]CONTRACT_CLASS_ENTRY_POINT types: sergey-melnychuk/iamgroot@f228044

[sergey-melnychuk]

@phklive

Sorry, I got too defensive for no reason. I misread Mirko's comment as a suggestion of a fix for iamgroot.

I have wasted spent too much time on code-generation topic, trying out tools and even building one. What I've learnt is that if a spec isn't built with a code-gen in mind (this is mostly about naming types and avoiding anonymous ones), no tool can "fix" it into meaningful code.

Unfortunately, in practice it means that without actually optimizing a spec for a specific tool, chances for a meaningful code to be generated are very slim.

UPDATE:
Just to summarize, there is nothing wrong about this PR and spec optimization. It was me complaining about existing code-generation tools. But if a spec is too vague, IMHO no tool can actually make it work.

[phklive]

Hey guys, I don't know what workflow is awaiting approval, what do we miss for a merge?

Thanks

[ArielElp]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @greged93 and @phklive)

[ArielElp]

Dismissed @greged93 from a discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @phklive)

[VeryDustyBot]

This item belongs to payment request #E92DCD on OnlyDust:

• from ivpavici to phklive
• 1 item included
• $500 for 1 day of work