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

v0.6.0: introduce v3 transactions #149

Merged
merged 4 commits into from
Nov 5, 2023
Merged

v0.6.0: introduce v3 transactions #149

merged 4 commits into from
Nov 5, 2023

Conversation

ArielElp
Copy link
Collaborator

@ArielElp ArielElp commented Oct 16, 2023

This change is Reviewable

Copy link
Collaborator

@ShahakShama ShahakShama 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, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ArielElp and @yair-starkware)


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

                                "title": "Version",
                                "description": "Version of the transaction scheme",
                                "$ref": "#/components/schemas/NUM_AS_HEX"

Change into enum with single option (to avoid cases where more than one schema is valid)


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

                        "title": "Version",
                        "description": "Version of the transaction scheme",
                        "$ref": "#/components/schemas/NUM_AS_HEX"

Same as above


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

                                "title": "Version",
                                "description": "Version of the transaction scheme",
                                "$ref": "#/components/schemas/NUM_AS_HEX"

Same as above

Copy link
Contributor

@yair-starkware yair-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: all files reviewed, 5 unresolved discussions (waiting on @ArielElp)

a discussion (no related file):
In some cases you use allOf and in some cases you don't



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

                "title": "Invoke transaction V3",
                "description": "initiates a transaction from a given account",
                "allOf": [

Add "additionalProperties": false
Preferably to all of the objects

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 2 files at r2.
Reviewable status: 1 of 2 files reviewed, 5 unresolved discussions (waiting on @ShahakShama and @yair-starkware)


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

Previously, ShahakShama wrote…

Change into enum with single option (to avoid cases where more than one schema is valid)

Done.


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

Previously, ShahakShama wrote…

Same as above

Done.


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

Previously, yair-starkware (Yair) wrote…

Add "additionalProperties": false
Preferably to all of the objects

are potential collisions solved by explicit requirements in version?


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

Previously, ShahakShama wrote…

Same as above

Done.

Copy link
Contributor

@yair-starkware yair-starkware left a comment

Choose a reason for hiding this comment

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

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


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

Previously, ArielElp wrote…

are potential collisions solved by explicit requirements in version?

That works too

Copy link
Collaborator

@ShahakShama ShahakShama left a comment

Choose a reason for hiding this comment

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

:lgtm:

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

@ArielElp ArielElp merged commit 5a90e1d into master Nov 5, 2023
1 check passed
@ArielElp ArielElp deleted the v0.6.0 branch November 5, 2023 15:16
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