Skip to content
Open
Show file tree
Hide file tree
Changes from 10 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
90 changes: 62 additions & 28 deletions tests/integration/common/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use starknet_rs_core::types::contract::{CompiledClass, SierraClass};
use starknet_rs_core::types::{
BlockId, BlockTag, ContractClass, ContractExecutionError, DeployAccountTransactionResult,
ExecutionResult, FeeEstimate, Felt, FlattenedSierraClass, FunctionCall,
InnerContractExecutionError, ResourceBounds, ResourceBoundsMapping,
InnerContractExecutionError, ResourceBounds, ResourceBoundsMapping, TransactionReceipt,
};
use starknet_rs_core::utils::{
UdcUniqueSettings, get_selector_from_name, get_udc_deployed_address,
Expand Down Expand Up @@ -102,16 +102,23 @@ pub fn get_timestamp_asserter_contract_artifacts() -> SierraWithCasmHash {
)
}

pub async fn assert_tx_succeeded_accepted<T: Provider>(tx_hash: &Felt, client: &T) {
let receipt = client.get_transaction_receipt(tx_hash).await.unwrap().receipt;
pub async fn assert_tx_succeeded_accepted<T: Provider>(
tx_hash: &Felt,
client: &T,
) -> Result<(), anyhow::Error> {
let receipt = match client.get_transaction_receipt(tx_hash).await {
Ok(receipt) => receipt.receipt,
Err(err) => return Err(err.into()),
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Can it be replaced with this?

Suggested change
let receipt = match client.get_transaction_receipt(tx_hash).await {
Ok(receipt) => receipt.receipt,
Err(err) => return Err(err.into()),
};
let receipt = client.get_transaction_receipt(tx_hash).await?.receipt;


match receipt.execution_result() {
ExecutionResult::Succeeded => (),
other => panic!("Should have succeeded; got: {other:?}"),
other => anyhow::bail!("Should have succeeded; got: {other:?}"),
}

match receipt.finality_status() {
starknet_rs_core::types::TransactionFinalityStatus::AcceptedOnL2 => (),
other => panic!("Should have been accepted on L2; got: {other:?}"),
starknet_rs_core::types::TransactionFinalityStatus::AcceptedOnL2 => Ok(()),
other => anyhow::bail!("Should have been accepted on L2; got: {other:?}"),
}
}

Expand All @@ -129,15 +136,18 @@ pub async fn assert_tx_succeeded_pre_confirmed<T: Provider>(_tx_hash: &Felt, _cl
// }
}

pub async fn get_contract_balance(devnet: &BackgroundDevnet, contract_address: Felt) -> Felt {
pub async fn get_contract_balance(
devnet: &BackgroundDevnet,
contract_address: Felt,
) -> Result<Felt, anyhow::Error> {
get_contract_balance_by_block_id(devnet, contract_address, BlockId::Tag(BlockTag::Latest)).await
}

pub async fn get_contract_balance_by_block_id(
devnet: &BackgroundDevnet,
contract_address: Felt,
block_id: BlockId,
) -> Felt {
) -> Result<Felt, anyhow::Error> {
let contract_call = FunctionCall {
contract_address,
entry_point_selector: get_selector_from_name("get_balance").unwrap(),
Expand All @@ -146,25 +156,33 @@ pub async fn get_contract_balance_by_block_id(
match devnet.json_rpc_client.call(contract_call, block_id).await {
Ok(res) => {
assert_eq!(res.len(), 1);
res[0]
Ok(res[0])
}
Err(e) => panic!("Call failed: {e}"),
Err(e) => anyhow::bail!("Call failed: {e}"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Keeping assertions in these helper functions does not completely address the issue targeted by this PR. Perhaps it can be restructured as:

        Ok(res) if res.len() == 1 => Ok(res[0]),
        other => anyhow::bail!("Expected a single-felt result; got: {other:?}"),

}
}

pub async fn assert_tx_reverted<T: Provider>(
tx_hash: &Felt,
client: &T,
expected_failure_reasons: &[&str],
) {
let receipt = client.get_transaction_receipt(tx_hash).await.unwrap().receipt;
) -> Result<(), anyhow::Error> {
let receipt: TransactionReceipt = match client.get_transaction_receipt(tx_hash).await {
Ok(receipt) => receipt.receipt,
Err(e) => return Err(e.into()),
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Can it be replaced with this?

Suggested change
let receipt: TransactionReceipt = match client.get_transaction_receipt(tx_hash).await {
Ok(receipt) => receipt.receipt,
Err(e) => return Err(e.into()),
};
let receipt = client.get_transaction_receipt(tx_hash).await?.receipt;


match receipt.execution_result() {
ExecutionResult::Reverted { reason } => {
for expected_reason in expected_failure_reasons {
assert_contains(reason, expected_reason);
match assert_contains(reason, expected_reason) {
Ok(_) => (),
Err(e) => return Err(e),
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can it be replaced with this?

Suggested change
match assert_contains(reason, expected_reason) {
Ok(_) => (),
Err(e) => return Err(e),
}
assert_contains(reason, expected_reason)?;

}
Ok(())
}
other => panic!("Should have reverted; got: {other:?}; receipt: {receipt:?}"),
other => anyhow::bail!("Should have reverted; got: {other:?}; receipt: {receipt:?}"),
}
}

Expand Down Expand Up @@ -436,14 +454,22 @@ pub async fn deploy_argent_account(

/// Assert that the set of elements of `iterable1` is a subset of the elements of `iterable2` and
/// vice versa.
pub fn assert_equal_elements<T>(iterable1: &[T], iterable2: &[T])
pub fn assert_equal_elements<T>(iterable1: &[T], iterable2: &[T]) -> Result<(), anyhow::Error>
where
T: PartialEq,
{
assert_eq!(iterable1.len(), iterable2.len());
match iterable1.len() == iterable2.len() {
true => (),
false => return Err(anyhow::anyhow!("Length mismatch")),
}
for e in iterable1 {
assert!(iterable2.contains(e));
match iterable2.contains(e) {
true => (),
false => return Err(anyhow::anyhow!("Element mismatch")),
}
}
Ok(())
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO matching with cases true and false is rarely acceptable. And keeping the length assertion as the first line of the function makes no sense.

Suggested change
assert_eq!(iterable1.len(), iterable2.len());
match iterable1.len() == iterable2.len() {
true => (),
false => return Err(anyhow::anyhow!("Length mismatch")),
}
for e in iterable1 {
assert!(iterable2.contains(e));
match iterable2.contains(e) {
true => (),
false => return Err(anyhow::anyhow!("Element mismatch")),
}
}
Ok(())
if iterable1.len() != iterable2.len() {
anyhow::bail!(
"Length mismatch: left = {}, right = {}",
iterable1.len(),
iterable2.len()
);
}
for e in iterable1 {
if !iterable2.contains(e) {
anyhow::bail!("Element {:?} from left not found in right", e);
}
}
Ok(())

}

pub fn felt_to_u256(f: Felt) -> U256 {
Expand Down Expand Up @@ -479,18 +505,20 @@ pub fn assert_json_rpc_errors_equal(e1: JsonRpcError, e2: JsonRpcError) {
}

/// Extract the message that is encapsulated inside the provided error.
pub fn extract_message_error(error: &ContractExecutionError) -> &String {
pub fn extract_message_error(error: &ContractExecutionError) -> Result<&String, anyhow::Error> {
match error {
ContractExecutionError::Message(msg) => msg,
other => panic!("Unexpected error: {other:?}"),
ContractExecutionError::Message(msg) => Ok(msg),
other => anyhow::bail!("Unexpected error: {other:?}"),
}
}

/// Extract the error that is nested inside the provided `error`.
pub fn extract_nested_error(error: &ContractExecutionError) -> &InnerContractExecutionError {
pub fn extract_nested_error(
error: &ContractExecutionError,
) -> Result<&InnerContractExecutionError, anyhow::Error> {
match error {
ContractExecutionError::Nested(nested) => nested,
other => panic!("Unexpected error: {other:?}"),
ContractExecutionError::Nested(nested) => Ok(nested),
other => anyhow::bail!("Unexpected error: {other:?}"),
}
}

Expand Down Expand Up @@ -581,11 +609,16 @@ pub async fn receive_notification(
Ok(notification["params"]["result"].take())
}

pub async fn assert_no_notifications(ws: &mut WebSocketStream<MaybeTlsStream<TcpStream>>) {
pub async fn assert_no_notifications(
ws: &mut WebSocketStream<MaybeTlsStream<TcpStream>>,
) -> Result<(), anyhow::Error> {
match receive_rpc_via_ws(ws).await {
Ok(resp) => panic!("Expected no notifications; found: {resp}"),
Err(e) if e.to_string().contains("deadline has elapsed") => { /* expected */ }
Err(e) => panic!("Expected to error out due to empty channel; found: {e}"),
Ok(resp) => anyhow::bail!("Expected no notifications; found: {resp}"),
Err(e) if e.to_string().contains("deadline has elapsed") => {
/* expected */
Ok(())
}
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
Err(e) if e.to_string().contains("deadline has elapsed") => {
/* expected */
Ok(())
}
Err(e) if e.to_string().contains("deadline has elapsed") => Ok(()), // expected

Err(e) => anyhow::bail!("Expected to error out due to empty channel; found: {e}"),
}
}

Expand Down Expand Up @@ -672,15 +705,16 @@ impl From<FeeEstimate> for LocalFee {
}

/// Panics if `text` does not contain `pattern`
pub fn assert_contains(text: &str, pattern: &str) {
pub fn assert_contains(text: &str, pattern: &str) -> Result<(), anyhow::Error> {
if !text.contains(pattern) {
panic!(
anyhow::bail!(
"Failed content assertion!
Pattern: '{pattern}'
not present in
Text: '{text}'"
);
}
Ok(())
}

/// Set time and generate a new block
Expand Down
11 changes: 8 additions & 3 deletions tests/integration/get_transaction_by_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ async fn get_declare_v3_transaction_by_hash_happy_path() {
.await
.unwrap();

assert_tx_succeeded_accepted(&declare_result.transaction_hash, &devnet.json_rpc_client).await;
assert_tx_succeeded_accepted(&declare_result.transaction_hash, &devnet.json_rpc_client)
.await
.unwrap();
}

#[tokio::test]
Expand Down Expand Up @@ -67,7 +69,8 @@ async fn get_deploy_account_transaction_by_hash_happy_path() {

let deploy_account_result = deployment.send().await.unwrap();
assert_tx_succeeded_accepted(&deploy_account_result.transaction_hash, &devnet.json_rpc_client)
.await;
.await
.unwrap();
}

#[tokio::test]
Expand Down Expand Up @@ -97,7 +100,9 @@ async fn get_invoke_v3_transaction_by_hash_happy_path() {
.await
.unwrap();

assert_tx_succeeded_accepted(&invoke_tx_result.transaction_hash, &devnet.json_rpc_client).await;
assert_tx_succeeded_accepted(&invoke_tx_result.transaction_hash, &devnet.json_rpc_client)
.await
.unwrap();
}

#[tokio::test]
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/get_transaction_receipt_by_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ async fn reverted_invoke_transaction_receipt() {
TransactionReceipt::Invoke(receipt) => {
match receipt.execution_result {
starknet_rs_core::types::ExecutionResult::Reverted { reason } => {
assert_contains(&reason, "Insufficient max L2Gas");
assert_contains(&reason, "Insufficient max L2Gas").unwrap();
}
_ => panic!("Invalid receipt {:?}", receipt),
}
Expand Down
12 changes: 8 additions & 4 deletions tests/integration/test_abort_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ async fn abort_blocks_error(
.send_custom_rpc("devnet_abortBlocks", json!({ "starting_block_id" : starting_block_id }))
.await
.unwrap_err();
assert_contains(&aborted_blocks_error.message, expected_message_substring);
assert_contains(&aborted_blocks_error.message, expected_message_substring).unwrap();
}

async fn assert_block_aborted(devnet: &BackgroundDevnet, block_hash: &Felt) {
Expand All @@ -33,13 +33,17 @@ async fn assert_block_aborted(devnet: &BackgroundDevnet, block_hash: &Felt) {
assert_eq!(err, RpcError { code: 24, message: "Block not found".into(), data: None })
}

async fn assert_txs_aborted(devnet: &BackgroundDevnet, tx_hashes: &[Felt]) {
async fn assert_txs_aborted(
devnet: &BackgroundDevnet,
tx_hashes: &[Felt],
) -> Result<(), anyhow::Error> {
for tx_hash in tx_hashes {
match devnet.json_rpc_client.get_transaction_by_hash(tx_hash).await {
Err(ProviderError::StarknetError(StarknetError::TransactionHashNotFound)) => (),
other => panic!("Unexpected tx response: {other:?}"),
other => anyhow::bail!("Unexpected tx response: {other:?}"),
}
}
Ok(())
}

#[tokio::test]
Expand Down Expand Up @@ -108,7 +112,7 @@ async fn abort_block_with_transaction() {
assert_eq!(aborted_blocks, vec![latest_block.block_hash]);

assert_block_aborted(&devnet, &latest_block.block_hash).await;
assert_txs_aborted(&devnet, &[mint_hash]).await;
assert_txs_aborted(&devnet, &[mint_hash]).await.unwrap();
}

#[tokio::test]
Expand Down
14 changes: 8 additions & 6 deletions tests/integration/test_accepting_blocks_on_l1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,22 @@ async fn assert_accepted_on_l1(
devnet: &BackgroundDevnet,
block_hashes: &[Felt],
tx_hashes: &[Felt],
) {
) -> Result<(), anyhow::Error> {
for block_hash in block_hashes {
match devnet.json_rpc_client.get_block_with_tx_hashes(BlockId::Hash(*block_hash)).await {
Ok(MaybePreConfirmedBlockWithTxHashes::Block(block)) => {
assert_eq!(block.status, BlockStatus::AcceptedOnL1)
}
other => panic!("Unexpected block: {other:?}"),
other => anyhow::bail!("Unexpected block: {other:?}"),
}
}

for tx_hash in tx_hashes {
let tx_status = devnet.json_rpc_client.get_transaction_status(tx_hash).await.unwrap();
assert_eq!(tx_status.finality_status(), SequencerTransactionStatus::AcceptedOnL1);
}

Ok(())
}

async fn assert_latest_accepted_on_l2(devnet: &BackgroundDevnet) {
Expand Down Expand Up @@ -63,7 +65,7 @@ async fn should_convert_accepted_on_l2_with_id_latest() {
let accepted_block_hashes = devnet.accept_on_l1(&BlockId::Tag(BlockTag::Latest)).await.unwrap();
assert_eq!(accepted_block_hashes, block_hashes);

assert_accepted_on_l1(&devnet, &block_hashes, &tx_hashes).await;
assert_accepted_on_l1(&devnet, &block_hashes, &tx_hashes).await.unwrap();
}

#[tokio::test]
Expand All @@ -85,7 +87,7 @@ async fn should_convert_all_txs_in_block_on_demand() {
let accepted_block_hashes = devnet.accept_on_l1(&BlockId::Tag(BlockTag::Latest)).await.unwrap();
assert_eq!(accepted_block_hashes, block_hashes);

assert_accepted_on_l1(&devnet, &block_hashes, &tx_hashes).await;
assert_accepted_on_l1(&devnet, &block_hashes, &tx_hashes).await.unwrap();
}

#[tokio::test]
Expand All @@ -107,7 +109,7 @@ async fn should_convert_accepted_on_l2_with_numeric_id() {
let accepted_block_hashes = devnet.accept_on_l1(&BlockId::Number(1)).await.unwrap();
assert_eq!(accepted_block_hashes, block_hashes);

assert_accepted_on_l1(&devnet, &block_hashes, &[tx_hash]).await;
assert_accepted_on_l1(&devnet, &block_hashes, &[tx_hash]).await.unwrap();
assert_latest_accepted_on_l2(&devnet).await;
}

Expand All @@ -130,7 +132,7 @@ async fn should_convert_accepted_on_l2_with_hash_id() {
let accepted_block_hashes = devnet.accept_on_l1(&BlockId::Hash(block_hash)).await.unwrap();
assert_eq!(accepted_block_hashes, block_hashes);

assert_accepted_on_l1(&devnet, &block_hashes, &[tx_hash]).await;
assert_accepted_on_l1(&devnet, &block_hashes, &[tx_hash]).await.unwrap();
assert_latest_accepted_on_l2(&devnet).await;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/integration/test_account_impersonation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ async fn test_simulate_transaction() {
account.execute_v3(invoke_calls.clone()).simulate(!do_validate, true).await;
if let Some(error_msg) = expected_error_message {
let simulation_err = simulation_result.expect_err("Expected simulation to fail");
assert_contains(&format!("{:?}", simulation_err).to_lowercase(), error_msg);
assert_contains(&format!("{:?}", simulation_err).to_lowercase(), error_msg).unwrap();
} else {
simulation_result.expect("Expected simulation to succeed");
}
Expand Down Expand Up @@ -268,5 +268,5 @@ async fn test_invoke_transaction(

fn assert_anyhow_error_contains_message(error: anyhow::Error, message: &str) {
let error_string = format!("{:?}", error.root_cause()).to_lowercase();
assert_contains(&error_string, message);
assert_contains(&error_string, message).unwrap();
}
Loading