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.

Sign up for GitHub

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: remove optional class info #518

Closed
wants to merge 1 commit into from
Closed

refactor: remove optional class info #518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 2 additions & 11 deletions crates/gateway/src/gateway.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,25 +128,16 @@ fn process_tx(
// Perform stateless validations.
stateless_tx_validator.validate(&tx)?;

let executable_tx = external_tx_to_executable_tx(
let _executable_tx = external_tx_to_executable_tx(
&tx,
&gateway_compiler,
&stateful_tx_validator.config.chain_info.chain_id,
)?;
let optional_class_info = match executable_tx {
starknet_api::executable_transaction::Transaction::Declare(tx) => {
Some(tx.class_info.try_into().map_err(|e| {
error!("Failed to convert Starknet API ClassInfo to Blockifier ClassInfo: {:?}", e);
GatewaySpecError::UnexpectedError { data: "Internal server error.".to_owned() }
})?)
}
_ => None,
};

let validator = stateful_tx_validator.instantiate_validator(state_reader_factory)?;
// TODO(Yael 31/7/24): refactor after IntrnalTransaction is ready, delete validate_info and
// compute all the info outside of run_validate.
let validate_info = stateful_tx_validator.run_validate(&tx, optional_class_info, validator)?;
let validate_info = stateful_tx_validator.run_validate(&tx, &gateway_compiler, validator)?;

// TODO(Arni): Add the Sierra and the Casm to the mempool input.
Ok(MempoolInput {
Expand Down
6 changes: 3 additions & 3 deletions crates/gateway/src/stateful_transaction_validator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use blockifier::blockifier::stateful_validator::{
};
use blockifier::bouncer::BouncerConfig;
use blockifier::context::BlockContext;
use blockifier::execution::contract_class::ClassInfo;
use blockifier::state::cached_state::CachedState;
use blockifier::transaction::account_transaction::AccountTransaction;
use blockifier::versioned_constants::VersionedConstants;
Expand All @@ -17,6 +16,7 @@ use starknet_api::transaction::TransactionHash;
use starknet_types_core::felt::Felt;
use tracing::error;

use crate::compilation::GatewayCompiler;
use crate::config::StatefulTransactionValidatorConfig;
use crate::errors::{GatewaySpecError, StatefulTransactionValidatorResult};
use crate::state_reader::{MempoolStateReader, StateReaderFactory};
Expand Down Expand Up @@ -68,12 +68,12 @@ impl StatefulTransactionValidator {
pub fn run_validate<V: StatefulTransactionValidatorTrait>(
&self,
external_tx: &RpcTransaction,
optional_class_info: Option<ClassInfo>,
gateway_compiler: &GatewayCompiler,
mut validator: V,
) -> StatefulTransactionValidatorResult<ValidateInfo> {
let account_tx = external_tx_to_account_tx(
external_tx,
optional_class_info,
gateway_compiler,
&self.config.chain_info.chain_id,
)?;
let tx_hash = account_tx.tx_hash();
Expand Down
22 changes: 8 additions & 14 deletions crates/gateway/src/stateful_transaction_validator_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use blockifier::blockifier::stateful_validator::{
StatefulValidatorResult as BlockifierStatefulValidatorResult,
};
use blockifier::context::BlockContext;
use blockifier::execution::contract_class::ClassInfo;
use blockifier::test_utils::CairoVersion;
use blockifier::transaction::errors::{TransactionFeeError, TransactionPreValidationError};
use mempool_test_utils::invoke_tx_args;
Expand Down Expand Up @@ -82,17 +81,8 @@ fn test_stateful_tx_validator(
#[case] expected_result: BlockifierStatefulValidatorResult<ValidateInfo>,
stateful_validator: StatefulTransactionValidator,
) {
let optional_class_info = match &external_tx {
RpcTransaction::Declare(declare_tx) => Some(
ClassInfo::try_from(
GatewayCompiler::new_cairo_lang_compiler(SierraToCasmCompilationConfig::default())
.process_declare_tx(declare_tx)
.unwrap(),
)
.unwrap(),
),
_ => None,
};
let gateway_compiler =
GatewayCompiler::new_cairo_lang_compiler(SierraToCasmCompilationConfig::default());

let expected_result_as_stateful_transaction_result =
expected_result.as_ref().map(|validate_info| *validate_info).map_err(|blockifier_error| {
Expand All @@ -103,7 +93,7 @@ fn test_stateful_tx_validator(
mock_validator.expect_validate().return_once(|_, _| expected_result.map(|_| ()));
mock_validator.expect_get_nonce().returning(|_| Ok(Nonce(Felt::ZERO)));

let result = stateful_validator.run_validate(&external_tx, optional_class_info, mock_validator);
let result = stateful_validator.run_validate(&external_tx, &gateway_compiler, mock_validator);
assert_eq!(result, expected_result_as_stateful_transaction_result);
}

Expand Down Expand Up @@ -174,5 +164,9 @@ fn test_skip_stateful_validation(
.expect_validate()
.withf(move |_, skip_validate| *skip_validate == should_skip_validate)
.returning(|_, _| Ok(()));
let _ = stateful_validator.run_validate(&external_tx, None, mock_validator);
let _ = stateful_validator.run_validate(
&external_tx,
&GatewayCompiler::new_cairo_lang_compiler(Default::default()),
mock_validator,
);
}
114 changes: 31 additions & 83 deletions crates/gateway/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use blockifier::execution::contract_class::ClassInfo;
use blockifier::transaction::account_transaction::AccountTransaction;
use blockifier::transaction::transactions::{
DeclareTransaction as BlockifierDeclareTransaction,
Expand Down Expand Up @@ -138,105 +137,54 @@ pub fn external_tx_to_executable_tx(
})
}

// TODO(Arni): Delete this function.
pub fn external_tx_to_account_tx(
external_tx: &RpcTransaction,
// FIXME(yael 15/4/24): calculate class_info inside the function once compilation code is ready
optional_class_info: Option<ClassInfo>,
chain_id: &ChainId,
) -> StatefulTransactionValidatorResult<AccountTransaction> {
match external_tx {
RpcTransaction::Declare(RpcDeclareTransaction::V3(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(),
tip: tx.tip,
signature: tx.signature.clone(),
nonce: tx.nonce,
compiled_class_hash: tx.compiled_class_hash,
sender_address: tx.sender_address,
nonce_data_availability_mode: tx.nonce_data_availability_mode,
fee_data_availability_mode: tx.fee_data_availability_mode,
paymaster_data: tx.paymaster_data.clone(),
account_deployment_data: tx.account_deployment_data.clone(),
});
let tx_hash = declare_tx
.calculate_transaction_hash(chain_id, &declare_tx.version())
.map_err(|e| {
error!("Failed to calculate tx hash: {}", e);
GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() }
})?;
let class_info =
optional_class_info.expect("declare transaction should contain class info");
let declare_tx = BlockifierDeclareTransaction::new(declare_tx, tx_hash, class_info)
.map_err(|e| {
error!("Failed to convert declare tx hash to blockifier tx type: {}", e);
// TODO(Arni): Move this function into the blockifier - use a try into. This is exactly the code
// dedup we are working on.
pub fn executable_transaction_to_account_tx(
tx: ExecutableTransaction,
) -> Result<AccountTransaction, GatewaySpecError> {
match tx {
ExecutableTransaction::Declare(declare_tx) => {
let declare_tx =
BlockifierDeclareTransaction::try_from(declare_tx).map_err(|error| {
error!("Failed to convert declare tx: {}", error);
GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() }
})?;
Ok(AccountTransaction::Declare(declare_tx))
}
RpcTransaction::DeployAccount(RpcDeployAccountTransaction::V3(tx)) => {
let deploy_account_tx = DeployAccountTransaction::V3(DeployAccountTransactionV3 {
resource_bounds: tx.resource_bounds.clone().into(),
tip: tx.tip,
signature: tx.signature.clone(),
nonce: tx.nonce,
class_hash: tx.class_hash,
contract_address_salt: tx.contract_address_salt,
constructor_calldata: tx.constructor_calldata.clone(),
nonce_data_availability_mode: tx.nonce_data_availability_mode,
fee_data_availability_mode: tx.fee_data_availability_mode,
paymaster_data: tx.paymaster_data.clone(),
});
let contract_address = calculate_contract_address(
deploy_account_tx.contract_address_salt(),
deploy_account_tx.class_hash(),
&deploy_account_tx.constructor_calldata(),
ContractAddress::default(),
)
.map_err(|e| {
error!("Failed to calculate contract address: {}", e);
GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() }
})?;
let tx_hash = deploy_account_tx
.calculate_transaction_hash(chain_id, &deploy_account_tx.version())
.map_err(|e| {
error!("Failed to calculate tx hash: {}", e);
GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() }
})?;
ExecutableTransaction::DeployAccount(ExecutableDeployAccountTransaction {
tx: deploy_account_tx,
tx_hash,
contract_address,
}) => {
let deploy_account_tx = BlockifierDeployAccountTransaction::new(
deploy_account_tx,
tx_hash,
contract_address,
);
Ok(AccountTransaction::DeployAccount(deploy_account_tx))
}
RpcTransaction::Invoke(RpcInvokeTransaction::V3(tx)) => {
let invoke_tx = InvokeTransaction::V3(InvokeTransactionV3 {
resource_bounds: tx.resource_bounds.clone().into(),
tip: tx.tip,
signature: tx.signature.clone(),
nonce: tx.nonce,
sender_address: tx.sender_address,
calldata: tx.calldata.clone(),
nonce_data_availability_mode: tx.nonce_data_availability_mode,
fee_data_availability_mode: tx.fee_data_availability_mode,
paymaster_data: tx.paymaster_data.clone(),
account_deployment_data: tx.account_deployment_data.clone(),
});
let tx_hash = invoke_tx
.calculate_transaction_hash(chain_id, &invoke_tx.version())
.map_err(|e| {
error!("Failed to calculate tx hash: {}", e);
GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() }
})?;
ExecutableTransaction::Invoke(ExecutableInvokeTransaction { tx: invoke_tx, tx_hash }) => {
let invoke_tx = BlockifierInvokeTransaction::new(invoke_tx, tx_hash);
Ok(AccountTransaction::Invoke(invoke_tx))
}
}
}

// TODO(Arni): Remove this function.
pub fn external_tx_to_account_tx(
external_tx: &RpcTransaction,
gateway_compiler: &GatewayCompiler,
chain_id: &ChainId,
) -> StatefulTransactionValidatorResult<AccountTransaction> {
let executable_tx = external_tx_to_executable_tx(
external_tx,
// WARNING: this is bad design.
gateway_compiler,
chain_id,
)?;
executable_transaction_to_account_tx(executable_tx)
}

// TODO(yael 9/5/54): Should be implemented as part of InternalTransaction in starknet-api
pub fn get_sender_address(tx: &AccountTransaction) -> ContractAddress {
match tx {
Expand Down
Loading