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

refactor(fee): tx_args holds valid resource bounds #508

Merged
merged 1 commit into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 19 additions & 29 deletions crates/blockifier/src/execution/syscalls/hint_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,35 +468,25 @@ impl<'a> SyscallHintProcessor<'a> {
let l2_gas_as_felt = Felt::from_hex(L2_GAS).map_err(SyscallExecutionError::from)?;
let l1_data_gas_as_felt = Felt::from_hex(L1_DATA).map_err(SyscallExecutionError::from)?;

let flat_resource_bounds = match tx_info.resource_bounds {
ValidResourceBounds::L1Gas(l1_bounds) => {
vec![
l1_gas_as_felt,
Felt::from(l1_bounds.max_amount),
Felt::from(l1_bounds.max_price_per_unit),
l2_gas_as_felt,
Felt::ZERO,
Felt::ZERO,
]
}
ValidResourceBounds::AllResources(AllResourceBounds {
l1_gas,
l2_gas,
l1_data_gas,
}) => {
vec![
l1_gas_as_felt,
Felt::from(l1_gas.max_amount),
Felt::from(l1_gas.max_price_per_unit),
l2_gas_as_felt,
Felt::from(l2_gas.max_amount),
Felt::from(l2_gas.max_price_per_unit),
l1_data_gas_as_felt,
Felt::from(l1_data_gas.max_amount),
Felt::from(l1_data_gas.max_price_per_unit),
]
}
};
let l1_gas_bounds = tx_info.resource_bounds.get_l1_bounds();
let l2_gas_bounds = tx_info.resource_bounds.get_l2_bounds();
let mut flat_resource_bounds = vec![
l1_gas_as_felt,
Felt::from(l1_gas_bounds.max_amount),
Felt::from(l1_gas_bounds.max_price_per_unit),
l2_gas_as_felt,
Felt::from(l2_gas_bounds.max_amount),
Felt::from(l2_gas_bounds.max_price_per_unit),
];
if let ValidResourceBounds::AllResources(AllResourceBounds { l1_data_gas, .. }) =
tx_info.resource_bounds
{
flat_resource_bounds.extend(vec![
l1_data_gas_as_felt,
Felt::from(l1_data_gas.max_amount),
Felt::from(l1_data_gas.max_price_per_unit),
])
}

self.allocate_data_segment(vm, &flat_resource_bounds)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/test_utils/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub fn declare_tx(declare_tx_args: DeclareTxArgs, class_info: ClassInfo) -> Acco
starknet_api::transaction::DeclareTransaction::V3(DeclareTransactionV3 {
signature: declare_tx_args.signature,
sender_address: declare_tx_args.sender_address,
resource_bounds: declare_tx_args.resource_bounds,
resource_bounds: declare_tx_args.resource_bounds.try_into().expect("todo"),
tip: declare_tx_args.tip,
nonce_data_availability_mode: declare_tx_args.nonce_data_availability_mode,
fee_data_availability_mode: declare_tx_args.fee_data_availability_mode,
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/test_utils/deploy_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub fn deploy_account_tx(
} else if deploy_tx_args.version == TransactionVersion::THREE {
starknet_api::transaction::DeployAccountTransaction::V3(DeployAccountTransactionV3 {
signature: deploy_tx_args.signature,
resource_bounds: deploy_tx_args.resource_bounds,
resource_bounds: deploy_tx_args.resource_bounds.try_into().expect("todo"),
tip: deploy_tx_args.tip,
nonce_data_availability_mode: deploy_tx_args.nonce_data_availability_mode,
fee_data_availability_mode: deploy_tx_args.fee_data_availability_mode,
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/test_utils/invoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub fn invoke_tx(invoke_args: InvokeTxArgs) -> InvokeTransaction {
})
} else if invoke_args.version == TransactionVersion::THREE {
starknet_api::transaction::InvokeTransaction::V3(InvokeTransactionV3 {
resource_bounds: invoke_args.resource_bounds,
resource_bounds: invoke_args.resource_bounds.try_into().expect("todo"),
calldata: invoke_args.calldata,
sender_address: invoke_args.sender_address,
nonce: invoke_args.nonce,
Expand Down
6 changes: 3 additions & 3 deletions crates/blockifier/src/transaction/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ impl TransactionInfoCreator for DeclareTransaction {
starknet_api::transaction::DeclareTransaction::V3(tx) => {
Ok(TransactionInfo::Current(CurrentTransactionInfo {
common_fields,
resource_bounds: tx.resource_bounds.clone().try_into()?,
resource_bounds: tx.resource_bounds.clone(),
tip: tx.tip,
nonce_data_availability_mode: tx.nonce_data_availability_mode,
fee_data_availability_mode: tx.fee_data_availability_mode,
Expand Down Expand Up @@ -411,7 +411,7 @@ impl TransactionInfoCreator for DeployAccountTransaction {
starknet_api::transaction::DeployAccountTransaction::V3(tx) => {
Ok(TransactionInfo::Current(CurrentTransactionInfo {
common_fields,
resource_bounds: tx.resource_bounds.clone().try_into()?,
resource_bounds: tx.resource_bounds.clone(),
tip: tx.tip,
nonce_data_availability_mode: tx.nonce_data_availability_mode,
fee_data_availability_mode: tx.fee_data_availability_mode,
Expand Down Expand Up @@ -535,7 +535,7 @@ impl TransactionInfoCreator for InvokeTransaction {
starknet_api::transaction::InvokeTransaction::V3(tx) => {
Ok(TransactionInfo::Current(CurrentTransactionInfo {
common_fields,
resource_bounds: tx.resource_bounds.clone().try_into()?,
resource_bounds: tx.resource_bounds.clone(),
tip: tx.tip,
nonce_data_availability_mode: tx.nonce_data_availability_mode,
fee_data_availability_mode: tx.fee_data_availability_mode,
Expand Down
4 changes: 2 additions & 2 deletions crates/gateway/src/gateway_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use mempool_test_utils::starknet_api_test_utils::{create_executable_tx, invoke_t
use mockall::predicate::eq;
use starknet_api::core::ContractAddress;
use starknet_api::rpc_transaction::RpcTransaction;
use starknet_api::transaction::TransactionHash;
use starknet_api::transaction::{TransactionHash, ValidResourceBounds};
use starknet_mempool_types::communication::MockMempoolClient;
use starknet_mempool_types::mempool_types::{Account, AccountState, MempoolInput};
use starknet_sierra_compile::config::SierraToCasmCompilationConfig;
Expand Down Expand Up @@ -72,7 +72,7 @@ async fn test_add_tx() {
tx_hash,
*tx.tip(),
*tx.nonce(),
tx.resource_bounds().clone().into(),
ValidResourceBounds::AllResources(tx.resource_bounds().clone()),
),
account: Account { sender_address, state: AccountState { nonce: *tx.nonce() } },
}))
Expand Down
7 changes: 4 additions & 3 deletions crates/gateway/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use starknet_api::transaction::{
DeployAccountTransactionV3,
InvokeTransactionV3,
TransactionHasher,
ValidResourceBounds,
};
use tracing::error;

Expand All @@ -35,7 +36,7 @@ pub fn rpc_tx_to_account_tx(
let declare_tx = DeclareTransaction::V3(DeclareTransactionV3 {
class_hash: ClassHash::default(), /* FIXME(yael 15/4/24): call the starknet-api
* function once ready */
resource_bounds: tx.resource_bounds.clone().into(),
resource_bounds: ValidResourceBounds::AllResources(tx.resource_bounds.clone()),
tip: tx.tip,
signature: tx.signature.clone(),
nonce: tx.nonce,
Expand Down Expand Up @@ -63,7 +64,7 @@ pub fn rpc_tx_to_account_tx(
}
RpcTransaction::DeployAccount(RpcDeployAccountTransaction::V3(tx)) => {
let deploy_account_tx = DeployAccountTransaction::V3(DeployAccountTransactionV3 {
resource_bounds: tx.resource_bounds.clone().into(),
resource_bounds: ValidResourceBounds::AllResources(tx.resource_bounds.clone()),
tip: tx.tip,
signature: tx.signature.clone(),
nonce: tx.nonce,
Expand Down Expand Up @@ -99,7 +100,7 @@ pub fn rpc_tx_to_account_tx(
}
RpcTransaction::Invoke(RpcInvokeTransaction::V3(tx)) => {
let invoke_tx = starknet_api::transaction::InvokeTransaction::V3(InvokeTransactionV3 {
resource_bounds: tx.resource_bounds.clone().into(),
resource_bounds: ValidResourceBounds::AllResources(tx.resource_bounds.clone()),
tip: tx.tip,
signature: tx.signature.clone(),
nonce: tx.nonce,
Expand Down
12 changes: 4 additions & 8 deletions crates/mempool/src/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::HashMap;

use starknet_api::core::{ContractAddress, Nonce};
use starknet_api::executable_transaction::Transaction;
use starknet_api::transaction::{DeprecatedResourceBoundsMapping, Resource, Tip, TransactionHash};
use starknet_api::transaction::{Tip, TransactionHash, ValidResourceBounds};
use starknet_mempool_types::errors::MempoolError;
use starknet_mempool_types::mempool_types::{Account, AccountState, MempoolInput, MempoolResult};

Expand Down Expand Up @@ -200,13 +200,13 @@ impl Mempool {
/// TODO(Mohammad): rename this struct to `ThinTransaction` once that name
/// becomes available, to better reflect its purpose and usage.
/// TODO(Mohammad): restore the Copy once ResourceBoundsMapping implements it.
#[derive(Clone, Debug, Default, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct TransactionReference {
pub sender_address: ContractAddress,
pub nonce: Nonce,
pub tx_hash: TransactionHash,
pub tip: Tip,
pub resource_bounds: DeprecatedResourceBoundsMapping,
pub resource_bounds: ValidResourceBounds,
}

impl TransactionReference {
Expand All @@ -224,10 +224,6 @@ impl TransactionReference {
}

pub fn get_l2_gas_price(&self) -> u128 {
self.resource_bounds
.0
.get(&Resource::L2Gas)
.map(|bounds| bounds.max_price_per_unit)
.expect("Expected a valid L2 gas resource bounds.")
self.resource_bounds.get_l2_bounds().max_price_per_unit
}
}
4 changes: 2 additions & 2 deletions crates/mempool/src/mempool_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rstest::{fixture, rstest};
use starknet_api::core::{ContractAddress, Nonce, PatriciaKey};
use starknet_api::executable_transaction::Transaction;
use starknet_api::hash::StarkHash;
use starknet_api::transaction::{Tip, TransactionHash};
use starknet_api::transaction::{Tip, TransactionHash, ValidResourceBounds};
use starknet_api::{contract_address, felt, patricia_key};
use starknet_mempool_types::errors::MempoolError;
use starknet_mempool_types::mempool_types::{Account, AccountState};
Expand Down Expand Up @@ -167,7 +167,7 @@ macro_rules! add_tx_input {
}};
(tip: $tip:expr, tx_hash: $tx_hash:expr, sender_address: $sender_address:expr,
tx_nonce: $tx_nonce:expr, account_nonce: $account_nonce:expr) => {{
add_tx_input!(tip: $tip, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, account_nonce: $account_nonce, resource_bounds: test_resource_bounds_mapping().into())
add_tx_input!(tip: $tip, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, account_nonce: $account_nonce, resource_bounds: ValidResourceBounds::AllResources(test_resource_bounds_mapping()))
}};
(tx_hash: $tx_hash:expr, sender_address: $sender_address:expr, tx_nonce: $tx_nonce:expr, account_nonce: $account_nonce:expr) => {
add_tx_input!(tip: 0, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, account_nonce: $account_nonce)
Expand Down
23 changes: 15 additions & 8 deletions crates/mempool/src/transaction_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@ use std::cmp::Ordering;
use std::collections::{BTreeSet, HashMap};

use starknet_api::core::{ContractAddress, Nonce};
use starknet_api::transaction::{Resource, ResourceBounds};
use starknet_api::transaction::{
AllResourceBounds,
ResourceBounds,
Tip,
TransactionHash,
ValidResourceBounds,
};

use crate::mempool::TransactionReference;

Expand Down Expand Up @@ -91,13 +97,14 @@ impl TransactionQueue {

fn _promote_txs_to_priority(&mut self, threshold: u128) {
let tmp_split_tx = PendingTransaction(TransactionReference {
resource_bounds: vec![
(Resource::L1Gas, ResourceBounds::default()),
(Resource::L2Gas, ResourceBounds { max_amount: 0, max_price_per_unit: threshold }),
]
.try_into()
.unwrap(),
..Default::default()
resource_bounds: ValidResourceBounds::AllResources(AllResourceBounds {
l2_gas: ResourceBounds { max_amount: 0, max_price_per_unit: threshold },
..Default::default()
}),
sender_address: ContractAddress::default(),
nonce: Nonce::default(),
tx_hash: TransactionHash::default(),
tip: Tip::default(),
});

// Split off the pending queue at the given transaction higher than the threshold.
Expand Down
4 changes: 2 additions & 2 deletions crates/mempool_test_utils/src/starknet_api_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ use starknet_api::transaction::{
AllResourceBounds,
Calldata,
ContractAddressSalt,
DeprecatedResourceBoundsMapping as ExecutableResourceBoundsMapping,
PaymasterData,
ResourceBounds,
Tip,
TransactionHash,
TransactionSignature,
TransactionVersion,
ValidResourceBounds,
};
use starknet_api::{calldata, felt};
use starknet_types_core::felt::Felt;
Expand Down Expand Up @@ -567,7 +567,7 @@ pub fn create_executable_tx(
tx_hash: TransactionHash,
tip: Tip,
nonce: Nonce,
resource_bounds: ExecutableResourceBoundsMapping,
resource_bounds: ValidResourceBounds,
) -> Transaction {
Transaction::Invoke(InvokeTransaction {
tx: starknet_api::transaction::InvokeTransaction::V3(
Expand Down
3 changes: 1 addition & 2 deletions crates/native_blockifier/src/py_declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use starknet_api::transaction::{
DeclareTransactionV0V1,
DeclareTransactionV2,
DeclareTransactionV3,
DeprecatedResourceBoundsMapping,
Fee,
PaymasterData,
Tip,
Expand Down Expand Up @@ -88,7 +87,7 @@ impl TryFrom<PyDeclareTransactionV3> for DeclareTransactionV3 {
type Error = NativeBlockifierInputError;
fn try_from(tx: PyDeclareTransactionV3) -> Result<Self, Self::Error> {
Ok(Self {
resource_bounds: DeprecatedResourceBoundsMapping::try_from(tx.resource_bounds)?,
resource_bounds: tx.resource_bounds.try_into()?,
tip: Tip(tx.tip),
signature: TransactionSignature(from_py_felts(tx.signature)),
nonce: Nonce(tx.nonce.0),
Expand Down
3 changes: 1 addition & 2 deletions crates/native_blockifier/src/py_deploy_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use starknet_api::transaction::{
ContractAddressSalt,
DeployAccountTransactionV1,
DeployAccountTransactionV3,
DeprecatedResourceBoundsMapping,
Fee,
PaymasterData,
Tip,
Expand Down Expand Up @@ -64,7 +63,7 @@ impl TryFrom<PyDeployAccountTransactionV3> for DeployAccountTransactionV3 {
type Error = NativeBlockifierInputError;
fn try_from(tx: PyDeployAccountTransactionV3) -> Result<Self, Self::Error> {
Ok(Self {
resource_bounds: DeprecatedResourceBoundsMapping::try_from(tx.resource_bounds)?,
resource_bounds: tx.resource_bounds.try_into()?,
tip: Tip(tx.tip),
signature: TransactionSignature(from_py_felts(tx.signature)),
nonce: Nonce(tx.nonce.0),
Expand Down
3 changes: 1 addition & 2 deletions crates/native_blockifier/src/py_invoke_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use starknet_api::data_availability::DataAvailabilityMode;
use starknet_api::transaction::{
AccountDeploymentData,
Calldata,
DeprecatedResourceBoundsMapping,
Fee,
InvokeTransactionV0,
InvokeTransactionV1,
Expand Down Expand Up @@ -87,7 +86,7 @@ impl TryFrom<PyInvokeTransactionV3> for InvokeTransactionV3 {
type Error = NativeBlockifierInputError;
fn try_from(tx: PyInvokeTransactionV3) -> Result<Self, Self::Error> {
Ok(Self {
resource_bounds: DeprecatedResourceBoundsMapping::try_from(tx.resource_bounds)?,
resource_bounds: tx.resource_bounds.try_into()?,
tip: Tip(tx.tip),
signature: TransactionSignature(from_py_felts(tx.signature)),
nonce: Nonce(tx.nonce.0),
Expand Down
14 changes: 13 additions & 1 deletion crates/native_blockifier/src/py_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ use blockifier::transaction::transaction_execution::Transaction;
use blockifier::transaction::transaction_types::TransactionType;
use pyo3::exceptions::PyValueError;
use pyo3::prelude::*;
use starknet_api::transaction::{Resource, ResourceBounds};
use starknet_api::transaction::{
DeprecatedResourceBoundsMapping,
Resource,
ResourceBounds,
ValidResourceBounds,
};
use starknet_api::StarknetApiError;

use crate::errors::{NativeBlockifierInputError, NativeBlockifierResult};
Expand Down Expand Up @@ -84,6 +89,13 @@ impl TryFrom<PyResourceBoundsMapping>
}
}

impl TryFrom<PyResourceBoundsMapping> for ValidResourceBounds {
type Error = StarknetApiError;
fn try_from(py_resource_bounds_mapping: PyResourceBoundsMapping) -> Result<Self, Self::Error> {
DeprecatedResourceBoundsMapping::try_from(py_resource_bounds_mapping)?.try_into()
}
}

#[derive(Clone)]
pub enum PyDataAvailabilityMode {
L1 = 0,
Expand Down
15 changes: 6 additions & 9 deletions crates/papyrus_common/src/transaction_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,17 @@ use starknet_api::transaction::{
DeployAccountTransactionV1,
DeployAccountTransactionV3,
DeployTransaction,
DeprecatedResourceBoundsMapping,
InvokeTransaction,
InvokeTransactionV0,
InvokeTransactionV1,
InvokeTransactionV3,
L1HandlerTransaction,
Resource,
ResourceBounds,
Tip,
Transaction,
TransactionHash,
TransactionVersion,
ValidResourceBounds,
};
use starknet_api::StarknetApiError;
use starknet_types_core::felt::Felt;
Expand Down Expand Up @@ -220,16 +219,14 @@ pub(crate) fn ascii_as_felt(ascii_str: &str) -> Result<Felt, StarknetApiError> {

// An implementation of the SNIP: https://github.com/EvyatarO/SNIPs/blob/snip-8/SNIPS/snip-8.md
fn get_tip_resource_bounds_hash(
resource_bounds_mapping: &DeprecatedResourceBoundsMapping,
resource_bounds: &ValidResourceBounds,
tip: &Tip,
) -> Result<Felt, StarknetApiError> {
let l1_resource_bounds =
resource_bounds_mapping.0.get(&Resource::L1Gas).expect("Missing l1 resource");
let l1_resource = get_concat_resource(l1_resource_bounds, L1_GAS)?;
let l1_resource_bounds = resource_bounds.get_l1_bounds();
let l2_resource_bounds = resource_bounds.get_l2_bounds();

let l2_resource_bounds =
resource_bounds_mapping.0.get(&Resource::L2Gas).expect("Missing l2 resource");
let l2_resource = get_concat_resource(l2_resource_bounds, L2_GAS)?;
let l1_resource = get_concat_resource(&l1_resource_bounds, L1_GAS)?;
let l2_resource = get_concat_resource(&l2_resource_bounds, L2_GAS)?;

Ok(HashChain::new()
.chain(&tip.0.into())
Expand Down
Loading
Loading