Partisia Smart Contracts (Rust) - zkCross

The swap-router PBC lacks a robust error handling mechanism throughout the swap execution process. In the execute_swap_callback function, if a swap operation fails (indicated by success == false), the contract merely logs an error message "Swap operation failed: handle error case", without performing additional actions and actually handling the error:

- rust/swap-router/src/lib.rs

        } else {
            println!("Swap operation failed: handle error case");
        }

As from the code snippet above, no actions are taken to revert previous operations or refund tokens to the user. This inadequacy can result in the contract and user funds being left in an inconsistent state, potentially leading to loss of funds or tokens being locked in the contract indefinitely.

Impact:

• Loss of Funds: Users may not receive their tokens back after a failed swap.

• Token Lockup: Tokens may remain locked in the contract with no means of recovery.

• Security Risks: Attackers could exploit this weakness to disrupt swap operations.

Run the following tests with the proptest lib by running cargo test -p swap-router --test swap-router-test test_inadequate_error_handling_may_cause_loss_of_funds -vv.

• PoC Code:

proptest! {
    #[test]
    fn test_inadequate_error_handling_may_cause_loss_of_funds(
        token_a_bytes in any::<[u8; 20]>(),
        token_b_bytes in any::<[u8; 20]>(),
        user_bytes in any::<[u8; 20]>(),
        amount_in in 1u64..1000u64,
    ) {
        // Setup addresses
        let token_a = Address {
            address_type: AddressType::PublicContract,
            identifier: token_a_bytes,
        };
        let token_b = Address {
            address_type: AddressType::PublicContract,
            identifier: token_b_bytes,
        };
        let user = Address {
            address_type: AddressType::Account,
            identifier: user_bytes,
        };

        // Initialize contract state with permission allowing the user to add swaps
        let ctx_init = create_contract_context(&user);
        let permission_add_swap = Permission::Specific { addresses: vec![user.clone()] };
        let (mut state, _events) = initialize(ctx_init, permission_add_swap);

        // User adds swap contract
        let ctx_user = create_contract_context(&user);

        let swap_ab = Address {
            address_type: AddressType::PublicContract,
            identifier: [1u8; 20], // Placeholder
        };

        let (state, _events) = add_swap_route(
            ctx_user,
            state,
            swap_ab.clone(),
            token_a.clone(),
            token_b.clone(),
        );

        // Create swap route A -> B
        let swap_route = vec![swap_ab.clone()];

        // Define amount_out_minimum
        let amount_out_minimum = 1u128;

        // Attempt to invoke route_swap and simulate a failure
        let ctx = create_contract_context(&user);
        let result = std::panic::catch_unwind(|| {
            // Since we cannot simulate a swap failure directly in the test environment,
            // we'll assume that the swap execution fails due to inadequate error handling.

            let (_state, _events) = route_swap(
                ctx,
                state,
                swap_route.clone(),
                token_a.clone(),
                token_b.clone(),
                amount_in.into(),
                amount_out_minimum,
            );

            // The lack of proper error handling may cause loss of funds
        });

        // Assert that the swap router does not handle errors properly
        prop_assert!(
            result.is_ok(),
            "Swap router lacks proper error handling, potentially causing loss of funds"
        );
    }
}

• Logs:

Implement comprehensive error handling throughout the contract and perform state clean-ups:

Revert on Failure:

• In callbacks, if an operation fails, revert the transaction or perform necessary compensating actions.

• Ensure that any tokens transferred to the contract are returned to the user.

State Cleanup:

• Reset or clear any state variables (callback_params, etc.) associated with the failed swap.

User Notification:

• Provide meaningful error messages and events to inform users of the failure.

SOLVED: The zkCross team has solved this issue as recommended. The commit ID for reference is 270c72518c1b272f761111061cdaeeae4f394643.

The swap-router PBC does not acquire locks on swap contracts before executing swaps. Locks should be obtained using the SwapLockContract to ensure that all swaps in a route can be executed atomically.

This locking mechanism guarantees that if any part of the swap route cannot be executed, all previously acquired locks can be canceled, and the entire swap can be safely aborted without any token transfers occurring.

In the current implementation, swaps are executed sequentially without prior lock acquisition. If a swap in the middle of the route fails (e.g. due to insufficient liquidity or unexpected errors), previous swaps have already been executed.

This leaves the user with intermediate tokens that they did not intend to hold, potentially causing financial loss or requiring additional actions to recover their original position.

- rust/swap-router/src/lib.rs

#[action(shortname = 0x01)]
pub fn route_swap(
    context: ContractContext,
    mut state: RouterState,
    swap_route: Vec<Address>,
    token_in: Address,
    token_out: Address,
    amount_in: TokenAmount,
    amount_out_minimum: TokenAmount,
) -> (RouterState, Vec<EventGroup>) {
    assert!(!swap_route.is_empty(), "The given route is empty.");

    validate_route_and_add_info(&swap_route, &state.swap_contracts, token_in, token_out);

    let mut transfer_event_builder = EventGroup::builder();

    MPC20Contract::at_address(token_in).transfer_from(
        &mut transfer_event_builder,
        &context.sender,
        &context.contract_address,
        amount_in,
    );

    // let total_cost = calculate_min_total_gas_cost(swap_route.len());

    state.callback_params = Some(CallbackParams {
        swap_route: swap_route.clone(),
        amount_in,
        amount_out_minimum,
        user: context.sender,
        token_in,
        token_out,
    });

    transfer_event_builder
        .with_callback(SHORTNAME_START_SWAP_CHAIN_CALLBACK)
        // .with_cost(total_cost)
        .done();

    let transfer_event = transfer_event_builder.build();
    let events = vec![transfer_event];
    println!("route_swap events: {:?}", events);

    (state, events)
}

Swap execution without locks:

- rust/swap-router/src/lib.rs

#[callback(shortname = 0x03)]
fn start_swap_chain_callback(
    _context: ContractContext,
    _callback_context: CallbackContext,
    state: RouterState,
) -> (RouterState, Vec<EventGroup>) {
    println!("Starting swap chain");

    let mut events = Vec::new();

    if let Some(params) = &state.callback_params {
        // log_input_sizes(params);
        let mut event_group_builder = EventGroup::builder();

        // let mut swap_route_iter = params.swap_route.iter();
        if let Some(first_swap) = params.swap_route.first() {
            MPC20Contract::at_address(params.token_in).approve(
                &mut event_group_builder,
                first_swap,
                params.amount_in,
            );

            SwapContract::at_address(*first_swap).execute_swap(
                &mut event_group_builder,
                params.token_in,
                params.amount_in,
                params.amount_out_minimum,
                if params.swap_route.len() == 1 {
                    params.user
                } else {
                    _context.contract_address
                },
            );
            if params.swap_route.len() > 1 {
                event_group_builder
                    .with_callback(SHORTNAME_EXECUTE_SWAP_CALLBACK)
                    // .with_cost(INTERNAL_GAS_COST_EXECUTE_SWAP_CALLBACK)
                    .done();
            }
            events.push(event_group_builder.build());
        }
    }

    println!("start_swap_chain_callback events: {:?}", events);

    (state, events)
}

Impact:

• Partial Execution Risk: Users may end up with unintended tokens if the swap route cannot be fully executed.

• Financial loss: Without atomicity, users may incur losses due to incomplete swaps.

• Deteriorated user experience: Users are exposed to complex recovery procedures and additional steps to recovery the funds from the incomplete swap operation.

Run the following tests with the proptest lib by running cargo test -p swap-router --test swap-router-test test_non_atomic_swaps_due_to_lack_of_locking_mechanism -vv.

• PoC Code:

proptest! {
    #[test]
    fn test_non_atomic_swaps_due_to_lack_of_locking_mechanism(
        token_a_bytes in any::<[u8; 20]>(),
        token_b_bytes in any::<[u8; 20]>(),
        token_c_bytes in any::<[u8; 20]>(),
        user_bytes in any::<[u8; 20]>(),
        amount_in in 1u64..1000u64,
    ) {
        // Setup addresses
        let token_a = Address {
            address_type: AddressType::PublicContract,
            identifier: token_a_bytes,
        };
        let token_b = Address {
            address_type: AddressType::PublicContract,
            identifier: token_b_bytes,
        };
        let token_c = Address {
            address_type: AddressType::PublicContract,
            identifier: token_c_bytes,
        };
        let user = Address {
            address_type: AddressType::Account,
            identifier: user_bytes,
        };

        // Initialize contract state with permission allowing the user to add swaps
        let ctx_init = create_contract_context(&user);
        let permission_add_swap = Permission::Specific { addresses: vec![user.clone()] };
        let (mut state, _events) = initialize(ctx_init, permission_add_swap);

        // User adds swap contracts
        let ctx_user1 = create_contract_context(&user);
        let ctx_user2 = create_contract_context(&user);

        let swap_ab = Address {
            address_type: AddressType::PublicContract,
            identifier: [1u8; 20], // Placeholder
        };
        let swap_bc = Address {
            address_type: AddressType::PublicContract,
            identifier: [2u8; 20], // Placeholder
        };

        let (state, _events) = add_swap_route(
            ctx_user1,
            state,
            swap_ab.clone(),
            token_a.clone(),
            token_b.clone(),
        );

        let (state, _events) = add_swap_route(
            ctx_user2,
            state,
            swap_bc.clone(),
            token_b.clone(),
            token_c.clone(),
        );

        // Create swap route A -> B -> C
        let swap_route = vec![swap_ab.clone(), swap_bc.clone()];

        // Define amount_out_minimum
        let amount_out_minimum = 1u128;

        // Attempt to invoke route_swap
        let ctx = create_contract_context(&user);
        let result = std::panic::catch_unwind(|| {
            let (_state, _events) = route_swap(
                ctx,
                state,
                swap_route.clone(),
                token_a.clone(),
                token_c.clone(),
                amount_in.into(),
                amount_out_minimum,
            );
        });

        // Assert that the swap router proceeds without locking mechanism
        prop_assert!(
            result.is_ok(),
            "Swap router proceeded without locking mechanism, leading to potential non-atomic swaps"
        );
    }
}

• Logs:

In order to remediate this issue, it is recommended to implement a locking mechanism similar as the following:

Acquire Locks Before Execution:

• Use the SwapLockContract to acquire locks on all swap contracts in the route before executing any swaps.

• Ensure that locks guarantee the ability to execute swaps at expected rates.

Atomic Swap Execution:

• Only proceed with executing swaps if all locks are successfully acquired.

• If any lock acquisition fails, cancel all previously acquired locks and abort the swap.

Update Contract Logic:

• Modify the route_swap function to initiate lock acquisition.

• Implement callbacks to handle lock acquisition results and proceed accordingly.

SOLVED: The zkCross team has solved this issue by adding a minimum threshold for the output amounts, and also simplifying the swap-router PBC. The commit ID for reference is ea14cc113747707b5f6180a16371d0389f23d0df.

A Time-of-Check to Time-of-Use (TOCTOU) vulnerability was identified in the swap function of the liquidity-swap Partisia contract. This issue becomes evident because the contract calculates the amount of tokens to swap based on the liquidity pool's state at the time of the check but does not account for state changes that may occur before the swap execution.

This gap allows attackers to manipulate the pool's state between the calculation and execution phases, leading to inaccurate swaps and potential financial loss for users.

Time of Check:

In the swap function, the contract calculates the output token amount (swap_to_amount) based on the current liquidity pool balances:

- rust/liquidity-swap/src/lib.rs

    let liquidity_pool_balance = state
        .token_balances
        .balances
        .get(&state.liquidity_pool_address)
        .expect("Liquidity pool address should have a token balance");

    let (from_token_balance, to_token_balance, to_token_address) =
        if from_token_address == state.token_balances.token_a_address {
            (
                liquidity_pool_balance.a_tokens,
                liquidity_pool_balance.b_tokens,
                state.token_balances.token_b_address,
            )
        } else if from_token_address == state.token_balances.token_b_address {
            (
                liquidity_pool_balance.b_tokens,
                liquidity_pool_balance.a_tokens,
                state.token_balances.token_a_address,
            )
        } else {
            panic!("Invalid from_token_address provided.");
        };

    let swap_to_amount = calculate_swap_to_amount(
        from_token_balance,
        to_token_balance,
        from_token_amount,
        state.swap_fee_per_mille,
    );

External call delay:

The contract initiates an external call to transfer the input tokens from the user to the liquidity pool:

- rust/liquidity-swap/src/lib.rs

    let mut input_event_group_builder = EventGroup::builder();
    MPC20Contract::at_address(from_token_address).transfer_from(
        &mut input_event_group_builder,
        &context.sender,
        &state.liquidity_pool_address,
        from_token_amount,
    );

This call introduces a delay before the swap is executed, during which the liquidity pool's state can change.

Time of Use:

In the swap_input_callback function, the contract executes the swap using the previously calculated swap_to_amount:

- rust/liquidity-swap/src/lib.rs

    MPC20Contract::at_address(to_token_address).transfer(
        &mut output_event_group_builder,
        &recipient,
        swap_to_amount,
    );

Between the initial calculation and the execution of the swap, other transactions can alter the liquidity pool's balances. This can result in the swap_to_amount being outdated, causing users to receive incorrect amounts. Attackers can exploit this by front-running transactions to manipulate pool balances, leading to financial losses for users and potential market manipulation.

Impact:

• Financial Loss: Users may receive fewer tokens than expected or suffer unfavorable exchange rates due to outdated swap calculations.

• Market Manipulation: Attackers can manipulate token prices and pool balances by exploiting the timing gap, affecting the integrity of the market.

• Loss of Trust: Users may lose confidence in the protocol, due to inconsistent and unreliable swap outputs, harming the reputation of the platform.

In order to reproduce this vulnerability, consider following the steps:

• Deploy the liquidity-swap contract and initialize it with Token A and Token B.

• Provide initial liquidity.

• Victim calls the swap function to exchange a certain amount of Token A for Token B.

• Before the victim's transaction is executed, an attacker performs a large swap or removes liquidity to alter the pool's balances.

• The victim's swap uses the old pool balances, resulting in an incorrect swap_to_amount.

Consider running the following test case. Run the tests with the proptest lib by running cargo test -p liquidity-swap.

• PoC Code:

    // Test for TOC-TOU Vulnerability in the Swap Function
    #[test]
    fn test_swap_toc_tou_vulnerability(
        pool_a_tokens in 1_000_000u128..2_000_000u128,
        pool_b_tokens in 1_000_000u128..2_000_000u128,
        from_token_amount in 1_000u128..10_000u128,
        attacker_token_amount in 100_000u128..500_000u128,
    ) {
        let swap_fee_per_mille = 3;

        // Time of Check: Calculate initial swap_to_amount
        let swap_to_amount_initial = calculate_swap_to_amount(
            pool_a_tokens,
            pool_b_tokens,
            from_token_amount,
            swap_fee_per_mille,
        );

        // Simulate pool changes before swap execution (attacker's actions)
        let attacker_swap_to_amount = calculate_swap_to_amount(
            pool_a_tokens,
            pool_b_tokens,
            attacker_token_amount,
            swap_fee_per_mille,
        );

        // Update pool balances after attacker's swap
        let updated_pool_a_tokens = pool_a_tokens + attacker_token_amount;
        let updated_pool_b_tokens = pool_b_tokens - attacker_swap_to_amount;

        // Recalculate swap_to_amount with updated pool balances
        let swap_to_amount_updated = calculate_swap_to_amount(
            updated_pool_a_tokens,
            updated_pool_b_tokens,
            from_token_amount,
            swap_fee_per_mille,
        );

        // Assert that the user receives less due to outdated calculation
        prop_assert!(
            swap_to_amount_initial > swap_to_amount_updated,
            "User receives less than expected due to TOC-TOU vulnerability"
        );
    }

• Proptest Regressions:

cc 9913529d8a85fc10ca15ec7bba7020fcce63571386de514443c21713ab358671 # shrinks to pool_a_tokens = 2249, pool_b_tokens = 17173, token_a_amount = 2595

• Logs:

To address the Time-of-Check to Time-of-Use (TOCTOU) vulnerability in the swap function, it is crucial to ensure that the calculation of the swap_to_amount and the execution of the swap are based on the same, unaltered state of the liquidity pool.

This can be achieved by recalculating the swap_to_amount after the token transfer has occurred. By doing so, the contract can accurately reflect any state changes that occur between the initial calculation and execution, thereby preventing attackers from exploiting this timing gap.

This approach will protect users from potential financial losses due to inaccurate swaps and enhance the overall security and trust in the protocol.

SOLVED: The zkCross team has solved this issue as recommended. The commit ID for reference is ea14cc113747707b5f6180a16371d0389f23d0df.

In the delist_swap_contract function of the dex-swap-factory PBC, the permission check uses permission_deploy_swap instead of the appropriate permission permission_delist_swap. This misalignment can lead to unauthorized access, unintended permission restrictions or other unpredicted behavior.

- rust/dex-swap-factory/src/lib.rs

#[action(shortname = 0x02)]
pub fn delist_swap_contract(
    ctx: ContractContext,
    mut state: SwapFactoryState,
    address: Address,
) -> SwapFactoryState {
    state
        .permission_deploy_swap
        .assert_permission_for(&ctx.sender, "delist swap");
    state.swap_contracts.remove(&address);
    state
}

Impact:

• Access control inconsistency: Unauthorized users may perform actions beyond their intended permissions.

• Operational confusion: Misconfigured permissions can lead to administrative difficulties and user confusion.

• Inconsistency with original design: Deviates from the original Permissions proposed by the PBC.

Consider using the following PoC code to visualize the incorrect permission check issue.

• PoC Code:

proptest! {
    #[test]
    fn test_incorrect_permission_check_in_delist_swap_contract(
        token_a_address_bytes in any::<[u8; 20]>(),
        token_b_address_bytes in any::<[u8; 20]>(),
        deployer_address_bytes in any::<[u8; 20]>(),
        delister_address_bytes in any::<[u8; 20]>(),
        swap_fee_per_mille in 0u16..=1000,
    ) {
        // Setup addresses
        let token_a_address = Address {
            address_type: AddressType::PublicContract,
            identifier: token_a_address_bytes,
        };

        let token_b_address = Address {
            address_type: AddressType::PublicContract,
            identifier: token_b_address_bytes,
        };

        let deployer_address = Address {
            address_type: AddressType::Account,
            identifier: deployer_address_bytes,
        };

        let delister_address = Address {
            address_type: AddressType::Account,
            identifier: delister_address_bytes,
        };

        // Ensure deployer and delister are different
        prop_assume!(deployer_address != delister_address);

        // Initialize contract state
        let state = SwapFactoryState {
            permission_update_swap: Permission::Specific { addresses: vec![deployer_address.clone()] },
            permission_deploy_swap: Permission::Specific { addresses: vec![deployer_address.clone()] },
            permission_delist_swap: Permission::Specific { addresses: vec![delister_address.clone()] },
            swap_contracts: AvlTreeMap::new(),
            swap_contract_binary: Some(DeployableContract::new(
                vec![1u8],
                vec![2u8],
                1,
            )),
            token_pair_map: AvlTreeMap::new(),
        };

        // Create token pair
        let token_pair = TokenPair {
            token_a_address: token_a_address.clone(),
            token_b_address: token_b_address.clone(),
        };

        // Deployer deploys swap contract
        let ctx_deployer = create_contract_context(&deployer_address);

        let (mut state, _events) = deploy_swap_contract(
            ctx_deployer,
            state,
            token_pair.clone(),
            swap_fee_per_mille,
        );

        // Simulate successful deployment
        let swap_contract_address = Address {
            address_type: AddressType::PublicContract,
            identifier: [1u8; 20], // Placeholder address
        };

        state.swap_contracts.insert(
            swap_contract_address.clone(),
            SwapContractInfo {
                contract_version: 1,
                token_pair: token_pair.clone(),
                successfully_deployed: true,
                supports_locks: false,
            },
        );

        state.token_pair_map.insert(token_pair.clone(), true);

        // Delister attempts to delist the swap contract
        let ctx_delister = create_contract_context(&delister_address);

        let result = std::panic::catch_unwind(|| {
            delist_swap_contract(
                ctx_delister,
                state,
                swap_contract_address.clone(),
            )
        });

        // Assert that delisting succeeds
        prop_assert!(
            !result.is_ok(),
            "Delisting failed even though delister has permission_delist_swap, indicating incorrect permission check."
        );
    }
}

• Logs:

Modify the delist_swap_contract function to use the correct permission_delist_swap for the permission check.

SOLVED: The zkCross team has solved this issue as recommended. The commit ID for reference is ea14cc113747707b5f6180a16371d0389f23d0df.

In the dex-swap-factory PBC, when a swap contract is delisted using the delist_swap_contract action, the associated token pair is not removed from the token_pair_map. This map tracks deployed token pairs and prevents the deployment of duplicate swap contracts for the same token pair.

- rust/dex-swap-factory/src/lib.rs

#[action(shortname = 0x02)]
pub fn delist_swap_contract(
    ctx: ContractContext,
    mut state: SwapFactoryState,
    address: Address,
) -> SwapFactoryState {
    state
        .permission_deploy_swap
        .assert_permission_for(&ctx.sender, "delist swap");
    state.swap_contracts.remove(&address);
    state
}

As a result, after delisting a swap contract, the token pair remains marked as active in token_pair_map. This prevents any future deployment of a new swap contract for the same token pair, effectively causing a Denial of Service (Dos).

This behavior is inconsistent with the handling of failed deployments in the swap_contract_exists_callback function, where the token pair is correctly removed from token_pair_map if the deployment fails.

- rust/dex-swap-factory/src/lib.rs

#[callback(shortname = 0x02)]
pub fn swap_contract_exists_callback(
    ctx: ContractContext,
    callback_ctx: CallbackContext,
    mut state: SwapFactoryState,
    swap_address: Address,
) -> SwapFactoryState {
    if !callback_ctx.results[0].succeeded {
        let token_pair = state
            .swap_contracts
            .get(&swap_address)
            .unwrap()
            .token_pair
            .clone();
        state.swap_contracts.remove(&swap_address);
        state.token_pair_map.remove(&token_pair);
    } else {
        let mut swap_contract_info = state.swap_contracts.get(&swap_address).unwrap();
        swap_contract_info.successfully_deployed = true;
        state
            .swap_contracts
            .insert(swap_address, swap_contract_info);
    }
    state
}

Impact:

• Inability to re-deploy swap contracts: The deployment of new swap contracts for token pairs whose contracts have been delisted will fail consistently.

• Permanent loss of functionality: Affected token pairs become permanently unusable within the swap factory unless the issue is corrected.

• User frustration and loss of trust: Users may lose confidence in the platform due to unexpected limitations and inability to trade certain token pairs.

Run the following tests with the proptest lib by running cargo test -p dex-swap-factory --test swap-factory-test test_delist_swap_contract_does_not_remove_token_pair -vv.

Steps:

• Create a token pair and deploy a swap contract.

• Delist the swap contract.

• Try to list again using the same token pair, which will fail.

• PoC Code:

proptest! {
    #[test]
    fn test_delist_swap_contract_does_not_remove_token_pair(
        token_a_address_bytes in any::<[u8; 20]>(),
        token_b_address_bytes in any::<[u8; 20]>(),
        deployer_address_bytes in any::<[u8; 20]>(),
        swap_fee_per_mille in 0u16..=1000,
    ) {
        // Setup addresses
        let token_a_address = Address {
            address_type: AddressType::PublicContract,
            identifier: token_a_address_bytes,
        };

        let token_b_address = Address {
            address_type: AddressType::PublicContract,
            identifier: token_b_address_bytes,
        };

        let deployer_address = Address {
            address_type: AddressType::Account,
            identifier: deployer_address_bytes,
        };

        // Initialize contract state
        let state = SwapFactoryState {
            permission_update_swap: Permission::Specific{addresses: (vec![deployer_address.clone()])},
            permission_deploy_swap: Permission::Specific{addresses: (vec![deployer_address.clone()])},
            permission_delist_swap: Permission::Specific{addresses: (vec![deployer_address.clone()])},
            swap_contracts: AvlTreeMap::new(),
            swap_contract_binary: Some(defi_common::deploy::DeployableContract::new(
                vec![1u8],
                vec![2u8],
                1,
            )),
            token_pair_map: AvlTreeMap::new(),
        };

        // Create token pair
        let token_pair = TokenPair {
            token_a_address: token_a_address.clone(),
            token_b_address: token_b_address.clone(),
        };

        // Deploy swap contract
        let ctx = create_contract_context(&deployer_address);

        let (mut state, _events) = deploy_swap_contract(
            ctx,
            state,
            token_pair.clone(),
            swap_fee_per_mille,
        );

        // Simulate successful deployment
        let swap_contract_address = Address {
            address_type: AddressType::PublicContract,
            identifier: [1u8; 20], // Placeholder address
        };

        state.swap_contracts.insert(
            swap_contract_address.clone(),
            SwapContractInfo {
                contract_version: 1,
                token_pair: token_pair.clone(),
                successfully_deployed: true,
                supports_locks: false,
            },
        );

        state.token_pair_map.insert(token_pair.clone(), true);

        // Delist the swap contract
        let ctx = create_contract_context(&deployer_address);

        let state = delist_swap_contract(
            ctx,
            state,
            swap_contract_address.clone(),
        );

        // Attempt to deploy a new swap contract for the same token pair
        let ctx = create_contract_context(&deployer_address);

        let result = std::panic::catch_unwind(|| {
            deploy_swap_contract(
                ctx,
                state,
                token_pair.clone(),
                swap_fee_per_mille,
            );
        });

        // Assert that deployment fails due to token pair still being present
        prop_assert!(
            result.is_err(),
            "Able to deploy a new swap contract for a delisted token pair, indicating token_pair_map was not updated."
        );
    }
}

fn create_contract_context(sender: &Address) -> ContractContext {
    ContractContext {
        contract_address: Address {
            address_type: AddressType::PublicContract,
            identifier: [0u8; 20],
        },
        sender: sender.clone(),
        block_time: 0,
        block_production_time: 0,
        current_transaction: pbc_contract_common::Hash { bytes: [255; 32] },
        original_transaction: pbc_contract_common::Hash { bytes: [255; 32] }
    }
}

• Logs:

Update the delist_swap_contract function to remove the associated token pair from the token_pair_map when a swap contract is delisted. This ensures that the token pair is no longer considered active, allowing new swap contracts to be deployed for that pair.

SOLVED: The zkCross team has solved this issue as recommended. The commit ID for reference is ea14cc113747707b5f6180a16371d0389f23d0df.

In the provided repository, a privatekey.key file was identified. It is not best practice to keep private/secret keys in repositories.

Consider removing private keys and other potential sensitive files from the repository.

ACKNOWLEDGED: The zkCross team has acknowledged this finding.

In the swap-router PBC the code estimation logic is commented-out, as follows:

// const INTERNAL_GAS_COST_APPROVE_CALLBACK: GasCost = 100;
// const INTERNAL_GAS_COST_EXECUTE_SWAP_CALLBACK: GasCost = 450000;
// const INTERNAL_GAS_COST_CANCEL_ROUTE_CALLBACK: GasCost = 100;

// fn calculate_min_total_gas_cost(number_of_swaps: usize) -> GasCost {
//     let number_of_swaps = number_of_swaps as u64;
//     let approve_cost = MPC20Contract::GAS_COST_APPROVE + INTERNAL_GAS_COST_APPROVE_CALLBACK;
//     let execute_swap_cost =
//         SwapContract::GAS_COST_EXECUTE_SWAP + INTERNAL_GAS_COST_EXECUTE_SWAP_CALLBACK;
//     let cancel_route_cost =
//         SwapContract::GAS_COST_CANCEL_SWAP + INTERNAL_GAS_COST_CANCEL_ROUTE_CALLBACK;

//     let total_execute_cost =
//         number_of_swaps * (approve_cost + execute_swap_cost) + MPC20Contract::GAS_COST_TRANSFER;

//     let total_cancel_cost = (number_of_swaps + 1) * cancel_route_cost;

//     std::cmp::max(total_execute_cost, total_cancel_cost)
// }

Without proper gas cost estimation, the contract cannot ensure that sufficient gas is allocated for swap execution. Transactions may run out of gas and fail unexpectedly, especially for longer swap routes involving multiple swaps.

It is recommended to implement gas cost estimation and specify in events.

Calculate Gas Costs:

• Implement functions to estimate the minimum required gas based on the number of swaps and expected operations.

  
  

Specify Gas Costs in Events:

• Use .with_cost(total_cost) in event builders to ensure adequate gas allocation.

ACKNOWLEDGED: The zkCross team has acknowledged this finding.