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

StarkNet 0.11.0 adjustments #65

Merged
merged 1 commit into from
Feb 19, 2023
Merged

StarkNet 0.11.0 adjustments #65

merged 1 commit into from
Feb 19, 2023

Conversation

ArielElp
Copy link
Collaborator

@ArielElp ArielElp commented Dec 28, 2022

This change is Reviewable

Copy link
Contributor

@anatgstarkware anatgstarkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

                    },
                    {
                        "$ref": "#/components/schemas/BROADCASTED_DEPLOY_TXN"

Can we erase BROADCASTED_DEPLOY_TXN?


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

                ]
            },
            "BROADCASTED_DECLARE_V1_TXN": {

Can we change the name to BROADCASTED_DECLARE_TXN_V1? Same for V2?


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

                ]
            },
            "BROADCASTED_DECLARE_V2_TXN": {

Can this be BROADCASTED_DECLARE_V1_TXN + compiled_class_hash?


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

                    },
                    "entry_points_by_type": {
                        "$ref": "#/components/schemas/DEPRECATED_CAIRO_ENTRY_POINTS"

Do we use DEPRECATED_CAIRO_ENTRY_POINTS anywhere else? If not, can we flatten it here? Same for SIERRA_ENTRY_POINTS and CAIRO_ENTRY_POINTS?


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

                ]
            },
            "CONTRACT_ENTRY_POINTS": {

Where do we need CONTRACT_ENTRY_POINTS?


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

                "properties": {
                    "CONSTRUCTOR": {
                        "$ref": "#/components/schemas/CAIRO_ENTRY_POINT_LIST"

Can we avoid the list types as well?

Code quote:

CAIRO_ENTRY_POINT_LIST

Copy link
Collaborator Author

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1.
Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @anatgstarkware)


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

Previously, anatgstarkware (anatg) wrote…

Can we erase BROADCASTED_DEPLOY_TXN?

Yep, removed


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

Previously, anatgstarkware (anatg) wrote…

Can this be BROADCASTED_DECLARE_V1_TXN + compiled_class_hash?

Classes are a different object (deprecated vs new)


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

Previously, anatgstarkware (anatg) wrote…

Do we use DEPRECATED_CAIRO_ENTRY_POINTS anywhere else? If not, can we flatten it here? Same for SIERRA_ENTRY_POINTS and CAIRO_ENTRY_POINTS?

Flattened


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

Previously, anatgstarkware (anatg) wrote…

Where do we need CONTRACT_ENTRY_POINTS?

Currently, it's not used. This boils down to whether or not we want get_compiled_class in the RPC. Thinking more about it, I think that the answer is no.

  1. Full nodes will need it internally to execute (until there's some other way to execute Sierra, which is futuristic for now)
  2. This is more of an implementation detail; I'm not sure a consumer has an interest in the Cairo code, which only serves the purpose of being able to prove executions of that class.
  3. You can compile the Sierra locally to get it (hopefully, if you use the same compiler version, you get the same result)

Removed for now (this + list + entry point type), WDYT?


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

Previously, anatgstarkware (anatg) wrote…

Can we avoid the list types as well?

See above

@anatgstarkware
Copy link
Contributor

api/starknet_api_openrpc.json line 1158 at r3 (raw file):

                    },
                    {
                        "$ref": "#/components/schemas/DEPLOY_TXN"

Do we still need the DEPLOY_TXN? and the DEPLOY_TXN_RECEIPT?

Copy link
Collaborator Author

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


api/starknet_api_openrpc.json line 1158 at r3 (raw file):

Previously, anatgstarkware (anatg) wrote…

Do we still need the DEPLOY_TXN? and the DEPLOY_TXN_RECEIPT?

We're still gonna have them in historical blocks, these gotta stay until regenesis.

@anatgstarkware
Copy link
Contributor

api/starknet_api_openrpc.json line 1794 at r4 (raw file):

                }
            },
            "COMPILED_CONTRACT_CLASS": {

so we don't use this anywhere?

Copy link

@eitan-starkware eitan-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


api/starknet_api_openrpc.json line 1794 at r4 (raw file):

                }
            },
            "COMPILED_CONTRACT_CLASS": {

Not sure who this file is used by, but in the code, it is referenced as "compiled class" (without the contract)

Code quote:

COMPILED_CONTRACT_CLASS

api/starknet_api_openrpc.json line 1910 at r4 (raw file):

                    "poseidon",
                    "ecdsa",
                    "ec_op",

Theis enum is different order in the repo, maybe we should keep it in the same order.
The builtins order from left to write (according to os.cairo in starknet): pedersen range_check ecdsa bitwise ec_op poseidon

Copy link
Collaborator Author

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @anatgstarkware and @eitan-starkware)


api/starknet_api_openrpc.json line 1794 at r4 (raw file):

Previously, anatgstarkware (anatg) wrote…

so we don't use this anywhere?

removed


api/starknet_api_openrpc.json line 1794 at r4 (raw file):

Previously, eitan-starkware (Eitan) wrote…

Not sure who this file is used by, but in the code, it is referenced as "compiled class" (without the contract)

removed from the RPC


api/starknet_api_openrpc.json line 1910 at r4 (raw file):

Previously, eitan-starkware (Eitan) wrote…

Theis enum is different order in the repo, maybe we should keep it in the same order.
The builtins order from left to write (according to os.cairo in starknet): pedersen range_check ecdsa bitwise ec_op poseidon

removed this, currently compiled_class is not represented in the RPC at all

Copy link
Contributor

@anatgstarkware anatgstarkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @ArielElp and @eitan-starkware)

Copy link
Collaborator Author

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, 1 of 1 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ArielElp)

Copy link
Collaborator Author

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ArielElp)

@ArielElp ArielElp merged commit 9e5f53b into master Feb 19, 2023
@ArielElp ArielElp deleted the starknet-v0.11.0 branch February 19, 2023 09:31
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

Successfully merging this pull request may close these issues.

3 participants