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

Use strk as a default fee token #2749

Merged
merged 30 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
868699d
Init
kkawula Nov 28, 2024
2976595
Wip
kkawula Nov 28, 2024
66918c1
Add test
kkawula Dec 2, 2024
4f41255
Clean
kkawula Dec 2, 2024
8a85685
WIP
kkawula Dec 3, 2024
4e1cd17
Update logic
kkawula Dec 5, 2024
e4afecf
Update test
kkawula Dec 5, 2024
3757ed4
Remove `validate`
kkawula Dec 5, 2024
6d1ff43
Update docs
kkawula Dec 8, 2024
14ae382
Update docs
kkawula Dec 8, 2024
2444474
Update docs
kkawula Dec 8, 2024
2395caf
Merge branch 'master' into kkawula/2696-use-strk-as-a-default-fee-token
kkawula Dec 8, 2024
500ec0e
Update CHANGELOG.md
kkawula Dec 9, 2024
b04879b
Update CHANGELOG.md
kkawula Dec 10, 2024
87aa0d2
Add deprecation info
kkawula Dec 10, 2024
58cc85b
Merge branch 'master' into kkawula/2696-use-strk-as-a-default-fee-token
kkawula Dec 10, 2024
5dc8da5
Typos
kkawula Dec 10, 2024
539fd6c
Update test
kkawula Dec 10, 2024
4a184d7
Update deprecation message
kkawula Dec 10, 2024
9f59dae
Update tests
kkawula Dec 10, 2024
e82bb73
Merge branch 'master' into kkawula/2696-use-strk-as-a-default-fee-token
kkawula Dec 10, 2024
7d08eaf
Fmt + lint
kkawula Dec 10, 2024
0d3cde9
Update printing warnings
kkawula Dec 13, 2024
527579f
Update change log
kkawula Dec 13, 2024
810fe98
Update docs
kkawula Dec 13, 2024
0d30c81
Update printing warning logic
kkawula Dec 13, 2024
1141adb
Add `test_fee_token_deprecation_warning`
kkawula Dec 13, 2024
d526d73
Update README
kkawula Dec 13, 2024
10b22e0
Update CHANGELOG.md
kkawula Dec 13, 2024
d59aefe
Merge branch 'master' into kkawula/2696-use-strk-as-a-default-fee-token
kkawula Dec 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

#### Changed

- `snforge_scarb_plugin` will now also emit warnings when errors occur
- `snforge_scarb_plugin` will now also emit warnings when errors occur

### Cast

#### Deprecated

- `--fee-token` and `--version` flags are now optional, `strk` and `v3` will be used by default
kkawula marked this conversation as resolved.
Show resolved Hide resolved

## [0.34.0] - 2024-11-26

Expand Down
60 changes: 48 additions & 12 deletions crates/sncast/src/helpers/fee.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
use anyhow::{bail, ensure, Result};
use anyhow::{bail, ensure, Error, Result};
use clap::{Args, ValueEnum};
use conversions::serde::deserialize::CairoDeserialize;
use conversions::TryIntoConv;
use shared::print::print_as_warning;
use starknet::core::types::BlockId;
use starknet::providers::Provider;
use starknet_types_core::felt::{Felt, NonZeroFelt};
use std::str::FromStr;

#[derive(Args, Debug, Clone)]
pub struct FeeArgs {
/// Token that transaction fee will be paid in
#[clap(long)]
#[clap(long, value_parser = parse_fee_token)]
pub fee_token: Option<FeeToken>,

/// Max fee for the transaction. If not provided, will be automatically estimated.
Expand Down Expand Up @@ -50,9 +52,9 @@ impl From<ScriptFeeSettings> for FeeArgs {

impl FeeArgs {
#[must_use]
pub fn fee_token(self, fee_token: Option<FeeToken>) -> Self {
pub fn fee_token(self, fee_token: FeeToken) -> Self {
Self {
fee_token: fee_token.or(self.fee_token),
fee_token: Some(fee_token),
..self
}
}
Expand Down Expand Up @@ -138,9 +140,10 @@ impl FeeArgs {
}
}

#[derive(ValueEnum, Debug, Clone, PartialEq)]
#[derive(ValueEnum, Default, Debug, Clone, PartialEq)]
pub enum FeeToken {
Eth,
#[default]
Strk,
}

Expand Down Expand Up @@ -187,7 +190,7 @@ impl From<ScriptFeeSettings> for FeeSettings {

pub trait PayableTransaction {
fn error_message(&self, token: &str, version: &str) -> String;
fn validate(&self) -> Result<()>;
fn validate_and_get_token(&self) -> Result<FeeToken>;
fn token_from_version(&self) -> Option<FeeToken>;
}

Expand All @@ -199,21 +202,27 @@ macro_rules! impl_payable_transaction {
$err_func(token, version)
}

fn validate(&self) -> Result<()> {
fn validate_and_get_token(&self) -> Result<FeeToken> {
match (
&self.token_from_version(),
&self.fee_args.fee_token,
) {
(Some(token_from_version), Some(token)) if token_from_version != token => {
Err(anyhow!(self.error_message(
&format!("{token:?}").to_lowercase(),
&format!("{:?}", token).to_lowercase(),
&format!("{:?}", self.version.clone().unwrap()).to_lowercase()
)))
}
},
(None, Some(token)) => {
Ok(token.clone())
},
(Some(token_from_version), None) => {
Ok(token_from_version.clone())
},
(None, None) => {
Err(anyhow!("Either --fee-token or --version must be provided"))
}
_ => Ok(()),
Ok(FeeToken::default())
},
_ => Ok(self.fee_args.fee_token.clone().unwrap_or_else(|| self.token_from_version().unwrap_or_else(|| unreachable!())))
}
}

Expand All @@ -225,3 +234,30 @@ macro_rules! impl_payable_transaction {
}
};
}

impl FromStr for FeeToken {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s.to_lowercase().as_str() {
"eth" => Ok(FeeToken::Eth),
"strk" => Ok(FeeToken::Strk),
_ => Err(String::from("Possible values: eth, strk")),
}
}
}

cptartur marked this conversation as resolved.
Show resolved Hide resolved
fn parse_fee_token(s: &str) -> Result<FeeToken, String> {
let deprecation_message = "Specifying '--fee-token' flag is deprecated and will be removed in the future. Use '--version' instead";
print_as_warning(&Error::msg(deprecation_message));

let parsed_token: FeeToken = s.parse()?;

if parsed_token == FeeToken::Eth {
print_as_warning(&Error::msg(
"Eth transactions will stop being supported in the future due to 'SNIP-16'",
));
}

Ok(parsed_token)
}
20 changes: 9 additions & 11 deletions crates/sncast/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ async fn run_async_command(
Commands::Declare(declare) => {
let provider = declare.rpc.get_provider(&config).await?;

declare.validate()?;
let fee_token = declare.validate_and_get_token()?;

let account = get_account(
&config.account,
Expand All @@ -262,6 +262,7 @@ async fn run_async_command(
&artifacts,
wait_config,
false,
fee_token,
)
.await
.map_err(handle_starknet_command_error)
Expand All @@ -286,9 +287,7 @@ async fn run_async_command(
}

Commands::Deploy(deploy) => {
deploy.validate()?;

let fee_token = deploy.token_from_version();
let fee_token = deploy.validate_and_get_token()?;

let Deploy {
arguments,
Expand Down Expand Up @@ -378,9 +377,7 @@ async fn run_async_command(
}

Commands::Invoke(invoke) => {
invoke.validate()?;

let fee_token = invoke.token_from_version();
let fee_token = invoke.validate_and_get_token()?;

let Invoke {
contract_address,
Expand Down Expand Up @@ -457,8 +454,6 @@ async fn run_async_command(
starknet_commands::multicall::Commands::Run(run) => {
let provider = run.rpc.get_provider(&config).await?;

run.validate()?;

let account = get_account(
&config.account,
&config.accounts_file,
Expand Down Expand Up @@ -533,10 +528,12 @@ async fn run_async_command(
}

account::Commands::Deploy(deploy) => {
deploy.validate()?;

let provider = deploy.rpc.get_provider(&config).await?;

let fee_token = deploy.validate_and_get_token()?;

let fee_args = deploy.fee_args.clone().fee_token(fee_token);

let chain_id = get_chain_id(&provider).await?;
let keystore_path = config.keystore.clone();
let result = starknet_commands::account::deploy::deploy(
Expand All @@ -547,6 +544,7 @@ async fn run_async_command(
wait_config,
&config.account,
keystore_path,
fee_args,
)
.await;

Expand Down
6 changes: 1 addition & 5 deletions crates/sncast/src/starknet_commands/account/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,8 @@ pub async fn deploy(
wait_config: WaitForTx,
account: &str,
keystore_path: Option<Utf8PathBuf>,
fee_args: FeeArgs,
) -> Result<InvokeResponse> {
let fee_args = deploy_args
.fee_args
.clone()
.fee_token(deploy_args.token_from_version());

if let Some(keystore_path_) = keystore_path {
deploy_from_keystore(
provider,
Expand Down
3 changes: 2 additions & 1 deletion crates/sncast/src/starknet_commands/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,12 @@ pub async fn declare(
artifacts: &HashMap<String, StarknetContractArtifacts>,
wait_config: WaitForTx,
skip_on_already_declared: bool,
fee_token: FeeToken,
) -> Result<DeclareResponse, StarknetCommandError> {
let fee_settings = declare
.fee_args
.clone()
.fee_token(declare.token_from_version())
.fee_token(fee_token)
.try_into_fee_settings(account.provider(), account.block_id())
.await?;

Expand Down
4 changes: 3 additions & 1 deletion crates/sncast/src/starknet_commands/multicall/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ pub async fn run(
account: &SingleOwnerAccount<&JsonRpcClient<HttpTransport>, LocalWallet>,
wait_config: WaitForTx,
) -> Result<InvokeResponse> {
let fee_args = run.fee_args.clone().fee_token(run.token_from_version());
let fee_token = run.validate_and_get_token()?;

let fee_args = run.fee_args.clone().fee_token(fee_token);

let contents = std::fs::read_to_string(&run.path)?;
let items_map: HashMap<String, Vec<toml::Value>> =
Expand Down
6 changes: 4 additions & 2 deletions crates/sncast/src/starknet_commands/script/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use shared::utils::build_readable_text;
use sncast::get_nonce;
use sncast::helpers::configuration::CastConfig;
use sncast::helpers::constants::SCRIPT_LIB_ARTIFACT_NAME;
use sncast::helpers::fee::{FeeSettings, ScriptFeeSettings};
use sncast::helpers::fee::{FeeArgs, FeeSettings, ScriptFeeSettings};
use sncast::helpers::rpc::RpcArgs;
use sncast::response::structs::ScriptRunResponse;
use sncast::state::hashing::{
Expand Down Expand Up @@ -116,7 +116,8 @@ impl<'a> ExtensionLogic for CastScriptExtension<'a> {
}
"declare" => {
let contract: String = input_reader.read::<ByteArray>()?.to_string();
let fee_args = input_reader.read::<ScriptFeeSettings>()?.into();
let fee_args: FeeArgs = input_reader.read::<ScriptFeeSettings>()?.into();
let fee_token = fee_args.fee_token.clone().unwrap_or_default();
let nonce = input_reader.read()?;

let declare = Declare {
Expand Down Expand Up @@ -145,6 +146,7 @@ impl<'a> ExtensionLogic for CastScriptExtension<'a> {
wait_params: self.config.wait_params,
},
true,
fee_token,
));

self.state.maybe_insert_tx_entry(
Expand Down
88 changes: 74 additions & 14 deletions crates/sncast/tests/e2e/account/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ async fn test_invalid_version_and_token_combination(fee_token: &str, version: &s
}

#[tokio::test]
async fn test_no_version_and_token() {
async fn test_default_fee_token() {
let tempdir = create_account(false, &OZ_CLASS_HASH.into_hex_string(), "oz").await;
let accounts_file = "accounts.json";

Expand All @@ -367,11 +367,79 @@ async fn test_no_version_and_token() {

let snapbox = runner(&args).current_dir(tempdir.path());

let output = snapbox.assert().failure();
assert_stderr_contains(
output,
"Error: Either --fee-token or --version must be provided",
);
snapbox.assert().success().stdout_matches(indoc! {r"
Transaction hash: [..]
command: account deploy
transaction_hash: [..]

To see invocation details, visit:
transaction: [..]
"});
}

#[tokio::test]
async fn test_fee_token_deprecation_warning_eth() {
let tempdir = create_account(false, &OZ_CLASS_HASH.into_hex_string(), "oz").await;
let accounts_file = "accounts.json";

let args = vec![
"--accounts-file",
accounts_file,
"--wait",
"account",
"deploy",
"--url",
URL,
"--name",
"my_account",
"--fee-token",
"eth",
];

let snapbox = runner(&args).current_dir(tempdir.path());

snapbox.assert().success().stdout_matches(indoc! {r"
[WARNING] Specifying '--fee-token' flag is deprecated and will be removed in the future. Use '--version' instead
[WARNING] Eth transactions will stop being supported in the future due to 'SNIP-16'
Transaction hash: [..]
command: account deploy
transaction_hash: [..]

To see invocation details, visit:
transaction: [..]
"});
}

#[tokio::test]
async fn test_fee_token_deprecation_warning_strk() {
let tempdir = create_account(false, &OZ_CLASS_HASH.into_hex_string(), "oz").await;
let accounts_file = "accounts.json";

let args = vec![
"--accounts-file",
accounts_file,
"--wait",
"account",
"deploy",
"--url",
URL,
"--name",
"my_account",
"--fee-token",
"strk",
];

let snapbox = runner(&args).current_dir(tempdir.path());

snapbox.assert().success().stdout_matches(indoc! {r"
[WARNING] Specifying '--fee-token' flag is deprecated and will be removed in the future. Use '--version' instead
Transaction hash: [..]
command: account deploy
transaction_hash: [..]

To see invocation details, visit:
transaction: [..]
"});
}

#[tokio::test]
Expand All @@ -390,8 +458,6 @@ pub async fn test_valid_class_hash() {
"my_account",
"--max-fee",
"10000000000000000",
"--fee-token",
"eth",
];

let snapbox = runner(&args).current_dir(tempdir.path());
Expand Down Expand Up @@ -421,8 +487,6 @@ pub async fn test_valid_no_max_fee() {
URL,
"--name",
"my_account",
"--fee-token",
"eth",
];

let snapbox = runner(&args).current_dir(tempdir.path());
Expand Down Expand Up @@ -515,8 +579,6 @@ pub async fn test_happy_case_keystore(account_type: &str) {
URL,
"--max-fee",
"99999999999999999",
"--fee-token",
"eth",
];

let snapbox = runner(&args).current_dir(tempdir.path());
Expand Down Expand Up @@ -791,8 +853,6 @@ pub async fn test_deploy_keystore_other_args() {
"some-name",
"--max-fee",
"99999999999999999",
"--fee-token",
"eth",
];

let snapbox = runner(&args).current_dir(tempdir.path());
Expand Down
Loading
Loading