Skip to content

feat: Improve error message when trying to redeclare contract using sncast declare #3253 #3578

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

feat: Improve error message when trying to redeclare contract using sncast declare #3253 #3578

wants to merge 8 commits into from
+13 −2

Conversation

JoshdfG
Copy link

@JoshdfG JoshdfG commented Jul 27, 2025

Closes #3253

Introduced changes

  • Added a more descriptive error message instead of dumping the whole rpc error directly
  • Added a new custom error in sncast/error.rs called ClassAlreadyDeclared(Felt252)

Checklist

  • Linked relevant issue
  • Updated relevant documentation
  • Added relevant tests
  • Performed self-review of the code
  • Added changes to CHANGELOG.md

@JoshdfG JoshdfG requested a review from a team as a code owner July 27, 2025 14:42
cptartur
cptartur reviewed Jul 28, 2025
View reviewed changes
cptartur
cptartur requested changes Jul 28, 2025
View reviewed changes
Copy link
Member

@cptartur cptartur left a comment

Choose a reason for hiding this comment

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

Some of cast tests are failing, probably asserts for outputs need to be updated.

Also run cargo fmt.

@JoshdfG
Copy link
Author

JoshdfG commented Jul 29, 2025

@cptartur I'll check those out I ran cargo fmt already that was an oversight

@JoshdfG JoshdfG requested a review from cptartur July 29, 2025 02:10
cptartur
cptartur reviewed Jul 29, 2025
View reviewed changes
JoshdfG and others added 2 commits July 29, 2025 17:30
@JoshdfG @cptartur
Update crates/sncast/src/response/errors.rs
220d8f6 
Co-authored-by: Artur Michałek <52135326+cptartur@users.noreply.github.com>
@JoshdfG
feat: update files to match requirement
305aa30
@JoshdfG JoshdfG requested a review from cptartur July 30, 2025 00:41
cptartur
cptartur reviewed Jul 31, 2025
View reviewed changes
Copy link
Member

@cptartur cptartur left a comment

Choose a reason for hiding this comment

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

Looks good, let's wait for the tests to pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cptartur cptartur cptartur requested changes

@franciszekjob franciszekjob Awaiting requested review from franciszekjob franciszekjob is a code owner automatically assigned from foundry-rs/starknet-foundry

@MKowalski8 MKowalski8 Awaiting requested review from MKowalski8 MKowalski8 is a code owner automatically assigned from foundry-rs/starknet-foundry

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve error message when trying to redeclare contract using sncast declare
2 participants
@JoshdfG @cptartur