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

field "title" usage #81

Open
tdelabro opened this issue Mar 28, 2023 · 4 comments
Open

field "title" usage #81

tdelabro opened this issue Mar 28, 2023 · 4 comments

Comments

@tdelabro
Copy link

I'm using this repository openRPC files for a project (madara lead by Abdel).

Rather than creating all the types myself in rust, I use this binary provided by openRPC: https://github.com/open-rpc/typings
It does work but I have two remarks
If there is no "title" field in the object definition of the type, it gets a random placeholder name:

/// String0SecgXNU
///
/// A tag specifying a dynamic reference to a block
///
#[derive(Serialize, Deserialize, Clone, Debug, Eq, PartialEq)]
pub enum String0SecgXNU {
    #[serde(rename = "latest")]
    Latest,
    #[serde(rename = "pending")]
    Pending,
}

This is the code generated for the BLOCK_TAG entry.

But some of the types have this field and it generates types like this one:

/// AnIntegerNumberInHexFormat0X
///
/// The number (height) of the estimated highest block to be synchronized
///
pub type AnIntegerNumberInHexFormat0X = String;

Here it was created from:

"NUM_AS_HEX": {
                "title": "An integer number in hex format (0x...)",
                "type": "string",
                "pattern": "^0x[a-fA-F0-9]+$"
            }

My point is that the "title" field is not used in a good way in these files.
"An integer number in hex format (0x...)" should be the content of the description field.
And all items should have a title, so we can avoid randomly generated names.

Do you want to do it yourselves, or would you appreciate it if I (or somebody else working at OnlyDust) come up with a pull request that fixes and harmonize all of this?

@tdelabro
Copy link
Author

@tabaktoni just explained to me that he had to spend two days manually creating each type himself in typescript last time he had to interact with starnknet-rpc because the automated tool outputted the kind of jibberish I described in this issue

@tabaktoni
Copy link
Contributor

If this get's fixed it will also reduce the time to implement RPC changes on starknetjs as we could export it directly to TS, instead of manually compare/re-type.

@tdelabro
Copy link
Author

Also there are methods like this one:

 {
            "name": "starknet_blockHashAndNumber",
            "summary": "Get the most recent accepted block hash and number",
            "params": [],
            "result": {
                "name": "result",
                "description": "The latest block hash and number",
                "schema": {
                    "type": "object",
                    "properties": {
                        "block_hash": {
                            "$ref": "#/components/schemas/BLOCK_HASH"
                        },
                        "block_number": {
                            "$ref": "#/components/schemas/BLOCK_NUMBER"
                        }
                    }
                }
            },
            "errors": [
                {
                    "$ref": "#/components/errors/NO_BLOCKS"
                }
            ]
        },

Which generates the following rust type as the response object:

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Default)]
pub struct BlockHashAndNumber {
    #[serde(skip_serializing_if = "Option::is_none")]
    pub block_hash: Option<FieldElement>,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub block_number: Option<BlockNumber>,
}

You can see that both the fields are optional, it's because none has been tagged as required in the initial json object.
I can't help to wonder if that is really what is intended or if you just forgot to specify it. Because it means that {} is a valid answer when the RPC users would probably prefer an error to be returned.
Is it a mistake or a design choice?

@tdelabro
Copy link
Author

@ArielElp wdyt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants