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

Added abi failure + warning for account contract functions. #4575

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

orizi
Copy link
Collaborator

@orizi orizi commented Dec 17, 2023

Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.


This change is Reviewable

@orizi orizi force-pushed the pr/orizi/starknet-diags/b72324ca branch from fc4de28 to ceef0f5 Compare December 17, 2023 05:53
@orizi orizi force-pushed the pr/orizi/starknet-diags/0ce94ba3 branch from 179552f to 97608e5 Compare December 17, 2023 05:53
@orizi orizi changed the base branch from pr/orizi/starknet-diags/b72324ca to main December 17, 2023 08:20
@orizi orizi force-pushed the pr/orizi/starknet-diags/0ce94ba3 branch from 97608e5 to de5d102 Compare December 17, 2023 08:20
@orizi orizi changed the base branch from main to pr/orizi/starknet-diags/b72324ca December 17, 2023 08:21
@orizi orizi force-pushed the pr/orizi/starknet-diags/b72324ca branch from 9949ba4 to 8f61fbd Compare December 17, 2023 08:34
@orizi orizi force-pushed the pr/orizi/starknet-diags/0ce94ba3 branch 2 times, most recently from 3c65990 to bc44841 Compare December 17, 2023 13:04
@orizi orizi force-pushed the pr/orizi/starknet-diags/b72324ca branch from 8f61fbd to 0b163e0 Compare December 17, 2023 13:04
Copy link
Contributor

@gilbens-starkware gilbens-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 1 of 4 files at r1, all commit messages.
Reviewable status: 1 of 4 files reviewed, 1 unresolved discussion (waiting on @orizi)


crates/cairo-lang-starknet/src/abi.rs line 261 at r1 (raw file):

                .find_attr(db, CONTRACT_ATTR)?
                .into_iter()
                .any(|attr| !attr.args.is_empty())

Suggestion:

            if submodule_id
                .find_attr(db, CONTRACT_ATTR)?
                .map_or(false, |attr| attr.args(db).is_empty())

Copy link
Collaborator Author

@orizi orizi 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: 1 of 4 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware)


crates/cairo-lang-starknet/src/abi.rs line 261 at r1 (raw file):

                .find_attr(db, CONTRACT_ATTR)?
                .into_iter()
                .any(|attr| !attr.args.is_empty())

Done.

@orizi orizi force-pushed the pr/orizi/starknet-diags/0ce94ba3 branch from bc44841 to 15e700b Compare December 17, 2023 13:54
@orizi orizi force-pushed the pr/orizi/starknet-diags/b72324ca branch from 0b163e0 to cedee5c Compare December 17, 2023 13:54
Copy link
Contributor

@gilbens-starkware gilbens-starkware 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 2 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @orizi)

@orizi orizi force-pushed the pr/orizi/starknet-diags/0ce94ba3 branch from 15e700b to 10c8a04 Compare December 18, 2023 09:46
@orizi orizi changed the base branch from pr/orizi/starknet-diags/b72324ca to main December 18, 2023 09:46
@orizi orizi force-pushed the pr/orizi/starknet-diags/0ce94ba3 branch from 10c8a04 to 12ce3a4 Compare December 18, 2023 09:50
Copy link
Contributor

@gilbens-starkware gilbens-starkware 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 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @orizi)

@orizi orizi force-pushed the pr/orizi/starknet-diags/0ce94ba3 branch 3 times, most recently from 51bea34 to 2ecbb84 Compare December 18, 2023 14:17
@orizi orizi force-pushed the pr/orizi/starknet-diags/0ce94ba3 branch from 2ecbb84 to 6f20b95 Compare December 19, 2023 08:26
Copy link
Contributor

@gilbens-starkware gilbens-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 4 of 4 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @orizi)

@orizi orizi force-pushed the pr/orizi/starknet-diags/0ce94ba3 branch from 6f20b95 to 2a1df48 Compare December 19, 2023 13:19
@orizi orizi force-pushed the pr/orizi/starknet-diags/0ce94ba3 branch from 2a1df48 to 10b473e Compare December 19, 2023 13:37
@orizi orizi enabled auto-merge December 19, 2023 13:37
Copy link
Collaborator Author

@orizi orizi left a comment

Choose a reason for hiding this comment

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

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

@orizi orizi added this pull request to the merge queue Dec 19, 2023
Merged via the queue into main with commit 207eb36 Dec 19, 2023
@orizi orizi deleted the pr/orizi/starknet-diags/0ce94ba3 branch December 21, 2023 09:11
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.

2 participants