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

Remove --version and --fee-token flags #2865

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

cptartur
Copy link
Member

@cptartur cptartur commented Jan 21, 2025

Closes #2856

Introduced changes

  • Removed --version and --fee-token flags in sncast
  • Removed ETH token support in cast scripts
  • Updated and/or removed related documentation

Checklist

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

@cptartur cptartur force-pushed the 2856-remove-deprecated-txs branch from 1b76089 to 3a3e168 Compare January 29, 2025 10:40
@cptartur cptartur changed the base branch from master to fix-cast-script-deploy-conversion January 29, 2025 10:41
@cptartur cptartur force-pushed the 2856-remove-deprecated-txs branch from 3a3e168 to 996952d Compare January 29, 2025 10:43
github-merge-queue bot pushed a commit that referenced this pull request Jan 29, 2025
… omitted (#2888)

<!-- Reference any GitHub issues resolved by this PR -->

Closes #

## Introduced changes

<!-- A brief description of the changes -->

This came up from #2865 

Conversion from `ScriptFeeSettings` to `FeeSettings` resulted in value
for `max_fee` being dropped completely. This conversion was only in cast
script "deploy" function.

If deploy was used with Strk token, any value passed to `max_fee` was
completely ignored.

Removed the conversion entirely and used correct structure in deploy.

## Checklist

<!-- Make sure all of these are complete -->

- [x] Linked relevant issue
- [x] Updated relevant documentation
- [x] Added relevant tests
- [x] Performed self-review of the code
- [x] Added changes to `CHANGELOG.md`
Base automatically changed from fix-cast-script-deploy-conversion to master January 29, 2025 13:53
@cptartur cptartur force-pushed the 2856-remove-deprecated-txs branch from 996952d to 180dc68 Compare January 30, 2025 15:09
@cptartur cptartur marked this pull request as ready for review January 30, 2025 15:46
@@ -85,7 +85,7 @@ pub async fn create(
.context("Invalid address")?
.parse()?;

let mut message = "Account successfully created. Prefund generated address with at least <max_fee> STRK tokens or an equivalent amount of ETH tokens. It is good to send more in the case of higher demand.".to_string();
let mut message = "Account successfully created. Prefund generated address with at least <max_fee> STRK tokens. It is good to send more in the case of higher demand.".to_string();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let mut message = "Account successfully created. Prefund generated address with at least <max_fee> STRK tokens. It is good to send more in the case of higher demand.".to_string();
let mut message = format!("Account successfully created. Prefund generated address with at least {max_fee} STRK tokens. It is good to send more in case of higher demand.");

And actually we should either replace STRK with FRI in the message or divide this value by 10^18. It could also be renamed to estimated_fee or something like that.

@@ -390,6 +390,7 @@ async fn test_run_script_twice_with_state_file_enabled() {
}

#[tokio::test]
// #[ignore]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// #[ignore]

@@ -537,6 +537,7 @@ pub fn assert_tx_entry_failed(
status: ScriptTransactionStatus,
msg_contains: Vec<&str>,
) {
dbg!(&tx_entry);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dbg!(&tx_entry);

Copy link
Collaborator

@franciszekjob franciszekjob left a comment

Choose a reason for hiding this comment

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

This line should be updated.

Looks like in this file we should also update mentioning eth to strk?

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.

Remove deprecated txs versions and tokens
3 participants