Skip to content

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
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
@JoshdfG JoshdfG requested a review from cptartur July 30, 2025 00:41
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

@github-actions
Copy link

github-actions bot commented Sep 1, 2025

Hi! This pull request hasn't had any activity for a while, so I am
marking it as stale. It will close in 14
days if it is not updated. Thanks for contributing!

@github-actions github-actions bot added the stale Stale Bot label Sep 1, 2025
@cptartur cptartur removed the stale Stale Bot label Sep 3, 2025
@JoshdfG JoshdfG requested a review from cptartur September 11, 2025 08:48
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.

Thanks for contributing, this will however not address the problem.
The added error handling duplicates the one we already have but the error still persists.

The problem is not due to missing error handling but from the fact that error comes from estimateFee endpoint and not declare.

Err(Provider(error)) => Err(StarknetCommandError::ProviderError(error.into())),
Err(Provider(ProviderError::StarknetError(StarknetError::ClassAlreadyDeclared))) => Err(
StarknetCommandError::ClassAlreadyDeclared(class_hash.into_()),
),
Copy link
Member

Choose a reason for hiding this comment

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

The error here is duplicating an handling of AlreadyDeclared we have couple of lines above

@cptartur
Copy link
Member

Thanks for contributing, this will however not address the problem. The added error handling duplicates the one we already have but the error still persists.

The problem is not due to missing error handling but from the fact that error comes from estimateFee endpoint and not declare.

I'll be investigating the problem with estimateFee

@cptartur cptartur closed this Sep 26, 2025
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.

Improve error message when trying to redeclare contract using sncast declare

2 participants