-
Notifications
You must be signed in to change notification settings - Fork 173
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
Handle contract redeclaration in cast script #2614
base: master
Are you sure you want to change the base?
Conversation
dbf9e0e
to
fdd9840
Compare
This reverts commit fdd9840.
ba8a972
to
a5dfaaf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good 👍 left some minor remarks.
@@ -19,6 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|||
- You can skip `--name` flag when using `account import` - a default name will be generated. | |||
|
|||
#### Changed | |||
|
|||
- In scripts, changed return type of `declare` so it can handle already declared contracts without failing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that not everyone reading this would know what was meant by scripts
. Optionally, we can not include the link.
- In scripts, changed return type of `declare` so it can handle already declared contracts without failing | |
- Updated the return type of `declare` in [Cairo Deployment Scripts](https://foundry-rs.github.io/starknet-foundry/starknet/script.html) to handle already declared contracts without failing |
@@ -237,7 +238,7 @@ async fn test_strk_fee_settings() { | |||
let contract_dir = duplicate_contract_directory_with_salt( | |||
SCRIPTS_DIR.to_owned() + "/map_script/contracts/", | |||
"dummy", | |||
"100", | |||
"12345", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed?
declare_transaction_response | ||
} | ||
DeclareResponse::AlreadyDeclared(_) => { | ||
unreachable!("Argument skip_on_already_declared is false") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
unreachable!("Argument skip_on_already_declared is false") | |
unreachable!("Argument `skip_on_already_declared` is false") |
|
||
// Check if already declared contract was handled correctly | ||
match second_declare_result { | ||
DeclareResult::Success(_) => panic!("Should be already declare"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DeclareResult::Success(_) => panic!("Should be already declare"), | |
DeclareResult::Success(_) => panic!("Should be already declared"), |
Closes #2469
Introduced changes
declare
in cast scripts so it doesn't fail on already declared contracts.Checklist
CHANGELOG.md