Atlas NEAR - Atlas Protocol

The Atlas contract incorporates deposit status rollback functionality for deposits in which the user successfully deposited native BTC into the Atlas Bitcoin wallet, but the execution of the atBTC minting logic for that user failed. These types of deposit records meet the following criteria:

• A non-empty remarks field, which contains any error messages encountered during the deposit execution.

• A status of either DEP_BTC_PENDING_DEPOSIT_INTO_BABYLON (currently not utilized) or DEP_BTC_PENDING_MINTED_INTO_ABTC, indicating that the deposit has been executed by calling the create_mint_abtc_signed_tx function.

This process is managed through the rollback_all_deposit_status and rollback_deposit_status_by_btc_txn_hash functions:

pub fn rollback_all_deposit_status(&mut self) {
    // Collect the keys and deposits that need to be updated
    let updates: Vec<(String, DepositRecord)> = self
        .deposits
        .iter()
        .filter_map(|(key, deposit)| {
            let mut deposit = deposit.clone(); // Clone the deposit to modify it
            if !deposit.btc_sender_address.is_empty()
                && !deposit.receiving_chain_id.is_empty()
                && !deposit.receiving_address.is_empty()
                && !deposit.remarks.is_empty()
            {
                match deposit.status {
                    DEP_BTC_PENDING_DEPOSIT_INTO_BABYLON => {
                        deposit.status = DEP_BTC_DEPOSITED_INTO_ATLAS;
                        deposit.remarks.clear();
                        ome((key.clone(), deposit)) // Clone the key and return the updated deposit
                    }
                    DEP_BTC_PENDING_MINTED_INTO_ABTC => {
                        deposit.status = DEP_BTC_DEPOSITED_INTO_ATLAS;
                        deposit.remarks.clear();
                        Some((key.clone(), deposit)) // Clone the key and return the updated deposit
                    }
                    _ => None,
                }
            } else {
                None
            }
        })
        .collect();

    // Apply the updates
    for (key, deposit) in updates {
        self.deposits.insert(key, deposit);
    }
}

pub fn rollback_deposit_status_by_btc_txn_hash(&mut self, btc_txn_hash: String) {
    if btc_txn_hash.is_empty() {
        env::panic_str("BTC transaction hash cannot be empty");
    }
        
    // Retrieve the deposit record based on btc_txn_hash
    if let Some(mut deposit) = self.deposits.get(&btc_txn_hash).cloned() {
        if !deposit.btc_sender_address.is_empty()
            && !deposit.receiving_chain_id.is_empty()
            && !deposit.receiving_address.is_empty()
            && !deposit.remarks.is_empty()
        {
            match deposit.status {
                DEP_BTC_PENDING_DEPOSIT_INTO_BABYLON => {
                    deposit.status = DEP_BTC_DEPOSITED_INTO_ATLAS;
                    deposit.remarks.clear();
                }
                DEP_BTC_PENDING_MINTED_INTO_ABTC => {
                    deposit.status = DEP_BTC_DEPOSITED_INTO_ATLAS;
                    deposit.remarks.clear();
                }
                _ => {
                    // No action needed for other statuses
                }
            }

            // Update the deposit record in the map
            self.deposits.insert(btc_txn_hash, deposit);
        }
    } else {
        env::log_str("Deposit record not found for the given BTC txn hash");
    }
}

These functions are accessible to any user, enabling public participation in the decentralization process by allowing users to trigger the re-execution of failed deposit records associated with successful Bitcoin transaction hashes. However, these functions do not check the error type that caused the initial execution attempt to fail.

Some of these errors are related to invalid record fields that cannot be modified by anyone, such as an incorrect receiving address for the deposit. As a result, invoking the rollback_all_deposit_status function could potentially roll back the statuses of a large number of failed deposits that are guaranteed to fail.

The rolled-back deposits are likely to be re-executed by the Atlas admin from the backend via an automated process, leading to additional gas costs incurred by the admin and negatively impacting the protocol's efficiency in processing valid deposits, as it will take longer to execute.

It is recommended to either:

• Implement checks that prevent the rollback of deposits failing due to persistent errors, such as invalid receiver addresses for the specified chain, based on the type of error encountered, or

• Restrict access control for the rollback features and implement the error-checking logic to the backend, ensuring it is integrated into an automated process before initiating the rollback

SOLVED: The Atlas Protocol team solved the issue in the specified commit id. It is now not possible to roll back the deposit status for deposits with EVM as the receiving chain if the address is invalid.

NEAR uses a system of Access Keys to simplify handling accounts. There are two types of keys: Full Access, that have full control over an account (i.e. can perform all actions), and Function Call, that only have permission to call a specified smart contract's method(s) that do not attach Ⓝ as a deposit.

When a user signs in on a website to interact with a contract, a Function Call key is created and stored on the website. Since the website has access to the Function Call key, it can use it to call the authorized methods as it pleases. While this is very user-friendly for most cases, it is important to be careful in scenarios involving transferring of valuable assets.

According to NEAR's documentation, it is essential to use the assert_one_yocto() function at the beginning of important functions where valuable assets can be transferred, which ensures the user has attached exactly one yoctoNEAR to the call. This is a security measure to ensure that the user is signing the transaction with a Full Accesskey.

However, atBTC privileged functions lack the two-factor authentication (2FA) mechanism. These functions are:

• atBTC::burn_redeem

• atBTC::burn_bridge

The potential impact of exploiting this vulnerability is the theft of the victim's native Bitcoin (BTC).

It’s important to note that the atBTC::mint_deposit and atBTC::mint_bridge functions can only be invoked by the owner of the atBTC contract, which should be the Atlas contract account. Additionally, only the mint_deposit function can be accessed through the atlas::create_mint_abtc_signed_tx function, called by the admin, and the entire process is likely automated from the backend. As a result, the risk of exploiting the absence of two-factor authentication (2FA) for these functions is unlikely.

Consider making the aforementioned functions payable and requiring the attachment of one yoctoNEAR to the function call by implementing the following logic:

#[payable]
pub fn function_name(...) {
    assert_one_yocto();
    // function logic
    ...
}

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

The Atlas contract includes several potentially dangerous functions that could have severe consequences on the contract state if accidentally invoked by the current contract owner. These functions are located in the src/atlas.rs file and include the following:

• clear_all_deposits

• clear_all_redemptions

• clear_all_validators

• clear_all_verifications

• clear_all_chain_configs

While these functions appear to be intended for testing purposes, as indicated by the comments in the src/atlas.rs file, it is crucial to remove them before deploying the contract to prevent any potential incidents. In addition to malicious actors compromising the owner account, insiders could leverage this functionality to cause griefing on their users and the organization and delete the contract.

Here are a few scenarios illustrating the potential impact of executing these functions on the Atlas contract:

• Executing the clear_all_deposits function creates an opportunity for re-minting atBTC by allowing the use of the same BTC transaction hash.

• Invoking the clear_all_redemptions function enables the possibility of re-redeeming BTC by permitting the use of the same NEAR/EVM transaction hash.

• Executing the clear_all_chain_configs function renders it impossible to access core features that depend on the receiving chain being properly configured. This includes functions such as create_mint_abtc_signed_tx, update_deposit_minted, update_redemption_start, and update_redemption_pending_btc_mempool, among others.

• Executing the clear_all_validators function delays the collection of necessary verifications for pending deposits and redemptions until new validators are added and can assist in generating those verifications.

• Executing the clear_all_verifications function postpones the execution of already validated deposits and redemptions until new verifications are collected by the validators.

PoC Explanation: The PoC performs the following steps:

1. Implement a helper function called simulate_deposit_mint_process that creates a new deposit, updates its status to Deposited collects the necessary verifications from two validators, and then updates the deposit status to Minted. This should be done without actually minting the deposit, utilizing an existing vulnerability solely for testing purposes

2. Setup an instance of the Atlas contract

3. Execute a deposit mint process by utilizing the simulate_deposit_mint_process helper function

4. Clear the deposits by the contract owner

5. Clear the verifications by the contract owner, as we have configured the contract with only two validators, and at least two verifications are required

6. Execute a deposit mint process for the same BTC transaction hash by utilizing the simulate_deposit_mint_process helper function

The following test was added to the contract/tests/update_deposit_tests.rs file:

fn simulate_deposit_mint_process(mut atlas: Atlas) -> Atlas {
    // Set the transaction caller as the contract admin
    testing_env!(VMContextBuilder::new().predecessor_account_id(accounts(1)).build());

    // Insert a new deposit
    let btc_txn_hash = "btc_txn_hash".to_string();
    atlas.insert_deposit_btc(
        btc_txn_hash.clone(),
        "btc_sender_address".to_string(),
        SIGNET.to_string(),
        "receiving_address".to_string(),
        1000,
        "".to_string(),
        1234567890,
        "".to_string(),
        1234567890,
    );

    // Update the deposit status to "deposited"
    atlas.update_deposit_btc_deposited(btc_txn_hash.clone(), 1234567891);
    
    // Simulate verification by two validators
    let deposit = atlas.get_deposit_by_btc_txn_hash(btc_txn_hash.clone()).unwrap();
    testing_env!(VMContextBuilder::new().predecessor_account_id(accounts(1)).build());
    atlas.increment_deposit_verified_count(deposit.clone());
    testing_env!(VMContextBuilder::new().predecessor_account_id(accounts(2)).build());
    atlas.increment_deposit_verified_count(deposit);

    // Update the deposit status to "minted"
    testing_env!(VMContextBuilder::new().predecessor_account_id(accounts(1)).build());
    let minted_txn_hash = "minted_txn_hash".to_string();
    atlas.update_deposit_minted(btc_txn_hash.clone(), minted_txn_hash.clone());

    // Verify that the deposit status has been updated successfully, along with the minted transaction hash
    let updated_deposit = atlas.get_deposit_by_btc_txn_hash(btc_txn_hash.clone()).unwrap();
    assert_eq!(updated_deposit.status, DEP_BTC_MINTED_INTO_ABTC);
    assert_eq!(updated_deposit.minted_txn_hash, minted_txn_hash);

    // Return the Atlas contract instance
    atlas
}

#[tokio::test]
async fn test_double_minting_using_the_same_tx_hash() {
    // Setup atlas contract
    let mut atlas = setup_atlas();
    
    // Mint a deposit
    atlas = simulate_deposit_mint_process(atlas);
    
    // Clear deposits and verifications by Atlas contract owner
    testing_env!(VMContextBuilder::new().predecessor_account_id(accounts(0)).build());
    atlas.clear_all_deposits();
    atlas.clear_all_verifications();
    
    // Re-mint the same deposit
    simulate_deposit_mint_process(atlas);
}

Executing the PoC produces the following result:

It is recommended to remove the potentially dangerous functions from the Atlas contract prior to deployment.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

The atlas contract defines specific roles for administrative actions, including an owner and an admin. However, the owner has the ability to change the admin and designate their own account ID as the admin. This violates the principle of separation of duties, introduces centralization risks, and grants the owner absolute control over the entire contract.

Below is the code of the change_atlas_admin_id function:

pub fn change_atlas_admin_id(&mut self, new_admin_id: AccountId) {
    self.assert_owner();

    assert!(!new_admin_id.to_string().is_empty(), "New admin ID cannot be blank");
    assert_ne!(new_admin_id, self.admin_id, "New admin ID must be different from the current admin ID");

    // Log the change for transparency
    env::log_str(&format!(
        "Changing Atlas admin from {} to {}",
        self.admin_id, new_admin_id
    ));

    self.admin_id = new_admin_id;
}

It is recommended to prevent the Atlas protocol owner from designating themselves as the admin to uphold the principle of separation of duties.

SOLVED: The Atlas Protocol team partially solved the issue in the following commit id:

• 3c09435

• d031a4c

The current implementation of the Atlas protocol lacks proper verification against the appropriate mempool in both the deposit and redemption processes. Specifically:

• During the deposit process, regarding the minting of atBTC tokens, while the protocol requires a minted_tx_hash representing the transaction where atBTC tokens are minted and sent to the user, there is no mechanism in place to verify this transaction in the NEAR/EVM mempool.

• Similarly, in the redemption process, after the status is set to RED_BTC_PENDING_MEMPOOL_CONFIRMATION, there is no verification to ensure that the btc_txn_hash corresponds to a valid transaction in the Bitcoin mempool before marking the redemption as completed with RED_BTC_REDEEMED_BACK_TO_USER.

In both the deposit and redemption processes, the lack of proper mempool verification could be exploited by a malicious admin, who has the necessary privileges to provide fake or arbitrary transaction hashes.

For deposits, using a fake or arbitrary minted_txn_hash when calling the update_deposit_minted function would allow the admin to set the deposit status to DEP_BTC_MINTED_INTO_ABTC without actually minting any atBTC tokens, marking the deposit as completed even though no tokens have been issued to the user in reality.

For redemptions, an admin could use an arbitrary btc_txn_hash and timestamp when calling the update_redemption_redeemed function, which would set the redemption status to RED_BTC_REDEEMED_BACK_TO_USER without ensuring that the BTC has actually been sent back to the user.

Below is the implementation of the update_deposit_minted function from contract/src/modules/deposits.rs file:

    pub fn update_deposit_minted(&mut self, btc_txn_hash: String, minted_txn_hash: String) {
        self.assert_admin();

        // Validate input parameters
        assert!(!btc_txn_hash.is_empty(), "BTC transaction hash cannot be empty");
        assert!(!minted_txn_hash.is_empty(), "Minted transaction hash cannot be empty");
        
        // Check if the deposit exists for the given btc_txn_hash
        if let Some(mut deposit) = self.deposits.get(&btc_txn_hash).cloned() {
            // Fetch chain configuration for the deposit's receiving_chain_id
            if let Some(chain_config) = self
                .chain_configs
                .get_chain_config(deposit.receiving_chain_id.clone())
            {
                // Check all specified conditions
                if (deposit.status == DEP_BTC_PENDING_MINTED_INTO_ABTC || 
                    deposit.status == DEP_BTC_DEPOSITED_INTO_ATLAS) 
                    && deposit.verified_count >= chain_config.validators_threshold
                    && deposit.remarks.is_empty()
                    && deposit.minted_txn_hash.is_empty()
                {
                    // All conditions are met, proceed to update the deposit status and minted transaction hash
                    deposit.status = DEP_BTC_MINTED_INTO_ABTC;
                    deposit.minted_txn_hash = minted_txn_hash.clone();
                    self.deposits.insert(btc_txn_hash.clone(), deposit);
                    log!("Deposit status updated to DEP_BTC_MINTED_INTO_ABTC for btc_txn_hash: {}", btc_txn_hash);
                } else {
                    // Log a message if conditions are not met
                    log!(
                        "Conditions not met for updating deposit minted status for btc_txn_hash: {}. 
                         Status: {}, Verified count: {}, Remarks: {}, Minted txn hash: {}",
                        btc_txn_hash,
                        deposit.status,
                        deposit.verified_count,
                        deposit.remarks,
                        deposit.minted_txn_hash
                    );
                }
            } else {
                env::panic_str("Chain configuration not found for receiving chain ID");
            }
        } else {
            env::panic_str("Deposit record not found");
        }
    }

Below is the implementation of update_redemption_redeemed function from contract/src/modules/redemptions.rs file:

    pub fn update_redemption_redeemed(&mut self, txn_hash: String, btc_txn_hash: String, timestamp: u64) {
        self.assert_admin();

        // Validate input parameters
        assert!(!txn_hash.is_empty(), "Transaction hash cannot be empty");
        assert!(!btc_txn_hash.is_empty(), "BTC transaction hash cannot be empty");
        assert!(timestamp != 0, "Timestamp cannot be zero");

        // Retrieve the redemption record based on txn_hash
        if let Some(mut redemption) = self.redemptions.get(&txn_hash).cloned() {
            // Fetch chain configuration for the redemption's chain ID
            if let Some(chain_config) = self
                .chain_configs
                .get_chain_config(redemption.abtc_redemption_chain_id.clone())
            {
                // Check all specified conditions
                if (redemption.status == RED_BTC_PENDING_REDEMPTION_FROM_ATLAS_TO_USER
                    || redemption.status == RED_BTC_PENDING_MEMPOOL_CONFIRMATION)
                    && redemption.verified_count >= chain_config.validators_threshold
                    && redemption.remarks.is_empty()
                {
                    // All conditions are met, proceed to update the redemption status
                    redemption.status = RED_BTC_REDEEMED_BACK_TO_USER;
                    redemption.btc_txn_hash = btc_txn_hash;
                    redemption.timestamp = timestamp;
                    self.redemptions.insert(txn_hash.clone(), redemption);
                    log!("Redemption status updated to RED_BTC_REDEEMED_BACK_TO_USER for txn_hash: {}", txn_hash);
                } else {
                    // Panic with the expected message if conditions are not met
                    env::panic_str("Conditions not met for updating redemption status");
                }
            } else {
                env::panic_str("Chain configuration not found for redemption chain ID");
            }
        } else {
            env::panic_str("Redemption record not found");
        }
    }

It is recommended to perform thorough checks against the relevant mempool to confirm that the provided transaction hash corresponds to a successful transaction. This ensures that the user has received their tokens before marking the deposit or redemption as complete.

SOLVED: The Atlas Protocol team solved the issue in the following commit id:

• 8f16fcf

• 2b88c2

• 72e266

The team chose the following solution:

Deposit: A new minted_txn_hash_verified_count counter has been added to the Deposit record to ensure it meets the required validator threshold before marking the deposit as minted. This counter is incremented each time an authorized validator from the receiving chain of the deposit calls the increment_deposit_minted_txn_hash_verified_count function.

Redemption: A new btc_txn_hash_verified_count counter has been added to the Redemption record to ensure it meets the required validator threshold before marking the redemption as redeemed. This counter is incremented each time an authorized validator from the Bitcoin chain of the redemption record calls the increment_redemption_btc_txn_hash_verified_count function.

In the Atlas contract, specifically within the deposits module, a deposit can have the following statuses:

1. A newly created deposit will initially have the status of DEP_BTC_PENDING_MEMPOOL.

2. The contract admin can then update the deposit status to DEP_BTC_DEPOSITED_INTO_ATLAS using the update_deposit_btc_deposited function.

3. Once the required verifications by validators are obtained, the contract admin sets the deposit status to DEP_BTC_PENDING_MINTED_INTO_ABTC through the create_mint_abtc_signed_tx function. However, at this stage, it's unclear whether the atBTC has been minted for the user, as the status remains unchanged regardless of the execution outcome.

4. Based on the content of the deposit's remarks field, the following can occur: If the remarks field is empty, the contract admin can update the deposit status to DEP_BTC_MINTED_INTO_ABTC by executing the update_deposit_minted function as soon as the required verifications threshold is met. Conversely, if the create_mint_abtc_signed_tx execution fails and the remarks field is no longer empty, anyone can roll back the status of the deposit.

However, a flaw in the logic of the update_deposit_minted function allows a malicious admin to change the deposit status from DEP_BTC_DEPOSITED_INTO_ATLASto DEP_BTC_MINTED_INTO_ABTC, as demonstrated in the implementation of the function below:

// Check all specified conditions
if (deposit.status == DEP_BTC_PENDING_MINTED_INTO_ABTC || 
    deposit.status == DEP_BTC_DEPOSITED_INTO_ATLAS) 
    && deposit.verified_count >= chain_config.validators_threshold
    && deposit.remarks.is_empty()
    && deposit.minted_txn_hash.is_empty()
{
   // All conditions are met, proceed to update the deposit status and minted transaction hash
   deposit.status = DEP_BTC_MINTED_INTO_ABTC;
   deposit.minted_txn_hash = minted_txn_hash.clone();
   self.deposits.insert(btc_txn_hash.clone(), deposit);
   log!("Deposit status updated to DEP_BTC_MINTED_INTO_ABTC for btc_txn_hash: {}", btc_txn_hash);
}

PoC Explanation: The PoC performs the following steps:

1. Setup the Atlas contract

2. Insert a new deposit with dummy data

3. Update the deposit status to "Deposited"

4. Collect two verifications from two validators to meet the required deposit validations threshold

5. Update the deposit status to "Minted" without successfully executing the create_mint_abtc_signed_tx function prior to it

6. Validate that the deposit status is not set to "Minted" and the minted transaction hash is successfully updated

The following test was added to the tests/update_deposit_tests.rs file:

#[tokio::test]
async fn test_update_deposit_status_to_minted_without_minting_atlas_btc() {
    let mut atlas = setup_atlas();
    let btc_txn_hash = "btc_txn_hash".to_string();
    atlas.insert_deposit_btc(
        btc_txn_hash.clone(),
        "btc_sender_address".to_string(),
        SIGNET.to_string(),
        "receiving_address".to_string(),
        1000,
        "".to_string(),
        1234567890,
        "".to_string(),
        1234567890,
    );
    atlas.update_deposit_btc_deposited(btc_txn_hash.clone(), 1234567891);
    
    // Simulate verification by two validators
    let deposit = atlas.get_deposit_by_btc_txn_hash(btc_txn_hash.clone()).unwrap();
    testing_env!(VMContextBuilder::new().predecessor_account_id(accounts(1)).build());
    atlas.increment_deposit_verified_count(deposit.clone());
    testing_env!(VMContextBuilder::new().predecessor_account_id(accounts(2)).build());
    atlas.increment_deposit_verified_count(deposit);

    testing_env!(VMContextBuilder::new().predecessor_account_id(accounts(1)).build());
    let minted_txn_hash = "minted_txn_hash".to_string();
    atlas.update_deposit_minted(btc_txn_hash.clone(), minted_txn_hash.clone());

    let updated_deposit = atlas.get_deposit_by_btc_txn_hash(btc_txn_hash.clone()).unwrap();
    assert_eq!(updated_deposit.status, DEP_BTC_MINTED_INTO_ABTC);
    assert_eq!(updated_deposit.minted_txn_hash, minted_txn_hash);
}

Executing the PoC with the following command yields the results shown below:

• Command:

cargo test test_update_deposit_status_to_minted_without_minting_atlas_btc --test update_deposit_tests -- --nocapture -- --exact

• Result:

To resolve this issue, the following actions are required:

1. The update_deposit_minted function must ensure that the deposit status is exclusively DEP_BTC_PENDING_MINTED_INTO_ABTC. This confirms that the create_mint_abtc_signed_tx function has been executed.

2. To verify the successful execution of create_mint_abtc_signed_tx, the deposit remarks field should be updated with any errors encountered during its execution. Currently, this is only done when the deposit path is EVM and the receiving address is invalid, which is insufficient.

3. When updating a deposit status to minted in case of an EVM path, the update_deposit_minted function should validate that the signed EVM transaction was executed successfully.

SOLVED: The Atlas Protocol team solved the issue in the following commit ids:

• b246ac2

• 5833b83

With the solution implemented during the HAL-06 issue fix, the update_deposit_minted function can only be executed once the minted transaction hash has been verified by a sufficient number of authorized validators from the deposit's receiving chain, meeting the required validator threshold.

In the deposit::insert_deposit_btc function of the Atlas contract, the minted_txn_hash field is not validated to be an empty string:

pub fn insert_deposit_btc(
    &mut self,
    btc_txn_hash: String,
    btc_sender_address: String,
    receiving_chain_id: String,
    receiving_address: String,
    btc_amount: u64,
    minted_txn_hash: String,
    timestamp: u64,
    remarks: String,
    date_created: u64,
) {
    self.assert_admin();

    // Validate mandatory input fields
    assert!(!btc_txn_hash.is_empty(), "BTC transaction hash cannot be empty");
    assert!(!btc_sender_address.is_empty(), "Sender address cannot be empty");
    assert!(!receiving_chain_id.is_empty(), "Receiving chain ID cannot be empty");
    assert!(!receiving_address.is_empty(), "Receiving address cannot be empty");
    assert!(btc_amount > 0, "BTC amount must be greater than zero");
    assert!(timestamp > 0, "Timestamp must be greater than zero");
    assert!(date_created > 0, "Date created must be greater than zero");

    // Check for duplicate transaction hash
    if self.deposits.contains_key(&btc_txn_hash) {
        env::panic_str("Deposit with this transaction hash already exists");
    }

    let record = DepositRecord {
        btc_txn_hash: btc_txn_hash.clone(),
        btc_sender_address,
        receiving_chain_id,
        receiving_address,
        btc_amount,
        minted_txn_hash,
        timestamp,
        status: DEP_BTC_PENDING_MEMPOOL,
        remarks,
        date_created,
        verified_count: 0,
    };

    self.deposits.insert(btc_txn_hash, record);
}

As a result, providing a non-empty minted_txn_hash will make it impossible to change the deposit status to DEP_BTC_DEPOSITED_INTO_ATLAS later on through the update_deposit_btc_deposited function and it will stay permanently in the DEP_BTC_PENDING_MEMPOOL status without a way to revert:

pub fn update_deposit_btc_deposited(&mut self, btc_txn_hash: String, timestamp: u64) {
        
    self.assert_admin();

    // Validate input parameters
    assert!(!btc_txn_hash.is_empty(), "BTC transaction hash cannot be empty");
    assert!(timestamp > 0, "Timestamp must be greater than zero");

    // Check if the deposit exists for the given btc_txn_hash
    if let Some(mut deposit) = self.deposits.get(&btc_txn_hash).cloned() {
        // Check all specified conditions
        if deposit.status == DEP_BTC_PENDING_MEMPOOL
            && deposit.remarks.is_empty()
            && deposit.minted_txn_hash.is_empty()
        {
            // All conditions are met, proceed to update the deposit status
            deposit.status = DEP_BTC_DEPOSITED_INTO_ATLAS;
            deposit.timestamp = timestamp;
            self.deposits.insert(btc_txn_hash.clone(), deposit);
            log!("Deposit status updated to DEP_BTC_DEPOSITED_INTO_ATLAS for btc_txn_hash: {}", btc_txn_hash);
        } else {
            // Log a message if conditions are not met
            log!("Conditions not met for updating deposit status for btc_txn_hash: {}. 
                  Status: {}, Remarks: {}, Minted txn hash: {}",
                  btc_txn_hash,
                  deposit.status,
                  deposit.remarks,
                  deposit.minted_txn_hash);
        }
    } else {
        env::panic_str("Deposit record not found");
    }
}

This will invalidate the provided BTC transaction hash, preventing the user who has potentially paid with native BTC from receiving atBTC and making it impossible for them to reclaim their BTC during the redemption phase.

Although the entire transaction sequence is likely automated from the server side, a malicious admin could frontrun the execution and call the smart contract directly with a non-empty minted_txn_hash, preventing users from recovering their funds.

PoC Explanation: The PoC performs the following steps:

1. Setup an instance of the Atlas contract

2. Insert a new deposit with a non-empty minted_txn_hash field

3. Validate the initial deposit status

4. Attempt to update the deposit status to deposited (DEP_BTC_DEPOSITED_INTO_ATLAS)

5. Validate that the deposit status did not change and is still set to DEP_BTC_PENDING_MEMPOOL

The following test was added to the contract/tests/update_deposit_tests.rs file:

#[tokio::test]
async fn test_create_corrupted_deposit() {
    // Setup atlas contract
    let mut atlas = setup_atlas();
    
    // Insert a new deposit with non-empty minted_txn_hash
    let btc_txn_hash = "btc_txn_hash".to_string();
    atlas.insert_deposit_btc(
        btc_txn_hash.clone(),
        "btc_sender_address".to_string(),
        SIGNET.to_string(),
        "receiving_address".to_string(),
        1000,
        "non-empty minted_txn_hash".to_string(),
        1234567890,
        "".to_string(),
        1234567890,
    );

    // Validate that the initial deposit status is set to DEP_BTC_PENDING_MEMPOOL
    let mut deposit = atlas.get_deposit_by_btc_txn_hash(btc_txn_hash.clone()).unwrap();
    assert_eq!(deposit.status, DEP_BTC_PENDING_MEMPOOL);

    // Update the deposit status to "Deposited"
    atlas.update_deposit_btc_deposited(btc_txn_hash.clone(), 1234567891);

    // Validate that the deposit status did not change
    deposit = atlas.get_deposit_by_btc_txn_hash(btc_txn_hash.clone()).unwrap();
    assert_eq!(deposit.status, DEP_BTC_PENDING_MEMPOOL);
}

Executing the PoC produces the following result:

It is recommended to ensure that the minted_txn_hash field is an empty string when creating a new deposit.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

The atBTC (NEAR) and atlas contracts currently lack a pausability mechanism. This feature is crucial for smart contracts, especially those handling significant assets or performing critical operations. A pause functionality allows the contract owner or designated parties to temporarily halt contract operations in case of emergencies, such as discovered vulnerabilities, unexpected behavior, or during upgrades.

Consider implementing a pausability mechanism for the aforementioned contracts.

SOLVED: The Atlas Protocol team solved the issue in the following commit id:

• af3f6fa

• 1ac752

The Atlas contract features four types of fees: deposit fees, redemption fees, bridging fees, and Babylon rewards fees.

Each fee type has a maximum limit of 10000 basis points (bps), which equates to 100%.

The update_fee_deposit_bps function enables setting the fee_deposit_bps field to a value of up to 10000 basis points:

pub fn update_fee_deposit_bps(&mut self, fee_deposit_bps: u16) {
    self.assert_owner();
    assert!(fee_deposit_bps <= 10000, "Invalid fee: must be between 0 and 10000 basis points");
    self.fee_deposit_bps = fee_deposit_bps;
}

The update_fee_redemption_bps function enables setting the fee_redemption_bps field to a value of up to 10000 basis points:

pub fn update_fee_redemption_bps(&mut self, fee_redemption_bps: u16) {
    self.assert_owner();
    assert!(fee_redemption_bps <= 10000, "Invalid fee: must be between 0 and 10000 basis points");
    self.fee_redemption_bps = fee_redemption_bps;
}

The update_fee_bridging_bps function enables setting the fee_bridging_bps field to a value of up to 10000 basis points:

pub fn update_fee_bridging_bps(&mut self, fee_bridging_bps: u16) {
    self.assert_owner();
    assert!(fee_bridging_bps <= 10000, "Invalid fee: must be between 0 and 10000 basis points");
    self.fee_bridging_bps = fee_bridging_bps;
}

The update_fee_babylon_rewards_bps function enables setting the fee_babylon_rewards_bps field to a value of up to 10000 basis points:

pub fn update_fee_babylon_rewards_bps(&mut self, fee_babylon_rewards_bps: u16) {
    self.assert_owner();
    assert!(fee_babylon_rewards_bps <= 10000, "Invalid fee: must be between 0 and 10000 basis points");
    self.fee_babylon_rewards_bps = fee_babylon_rewards_bps;
}

Consequently, setting excessively high fees could result in users' tokens being completely depleted by fees, which is not ideal.

Below is an example from the fees::get_deposit_tax_amount function, illustrating that if fee_deposit_bps equals 10000, the tax amount on the deposit will equal the initial deposit amount:

pub fn get_deposit_tax_amount(&self, amount: u64) -> u64 {
    assert!(amount > 0, "Amount must be greater than zero");
    let deposit_percentage = self.global_params.get_fee_deposit_bps() as u128;
    let fee = (amount as u128 * deposit_percentage / 10000) as u64;
    fee
}

It is recommended to establish a reasonable upper limit for the various protocol fees within the Atlas contract.

SOLVED: The Atlas Protocol team solved the issue in the specified commit by adjusting the upper bounds for the protocol fees as follows:

• Deposit Fee: 3%

• Redemption Fee: 3%

• Bridging Fee: 3%

• Babylon Rewards Fee: 10%

The NEAR blockchain allows for redeploying a contract to the same Account ID to modify its code. However, basic redeployment can cause issues if the contract's state requires changes, such as alterations in variable types or the addition or removal of storage variables. In such cases, upgrading via code redeployment can prevent the successful deserialization of the state, rendering the contract inoperable.

Notably, the Atlas and atBTC contracts do not include a migration feature, making it impossible to upgrade their states as needed.

It is recommended to implement a state-updating upgrade functionality to mitigate the potential limitations that may arise from a code upgrade using full access keys.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

The atlas protocol enables users to stake their native BTC within Bitcoin staking platforms and get a corresponding amount of atBTC tokens on their preferred wallet, whether it's EVM or NEAR, which can be used in DeFi protocols while the original BTC collateral earns yield passively.

For this to function correctly, atBTC must share the same decimal representation as BTC, which is 8.

While the EVM variant of atBTC ensures this by overriding the decimals() function to return 8, the NEAR variant lacks this constraint, as the new function does not verify that the decimals specified in the fungible token metadata equal 8:

#[init]
pub fn new(owner_id: AccountId, metadata: FungibleTokenMetadata) -> Self {
    assert!(!env::state_exists(), "Already initialized");
    metadata.assert_valid();
    let mut this = Self {
        token: FungibleToken::new(b"a".to_vec()),
        metadata: LazyOption::new(b"m".to_vec(), Some(&metadata)),
        owner_id: owner_id.clone(), // Set the owner's account ID
    };
    this.token.internal_register_account(&owner_id);
    this
}

Consequently, if the deployer mistakenly sets a different decimals value, it may confuse users, as they will see an amount in their NEAR wallet that does not match what they deposited. Additionally, this discrepancy would result in atBTC having different decimal representations across two chains, causing the same token to represent different values on each chain.

It is recommended to implement a check that enforces the decimals to be set to 8, preventing any potential issues.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

The smart contract includes ownership transfer functions in both global_params.rs and chain_config.rs files—specifically change_global_param_owner_id and change_chain_configs_owner_id—intended to allow the transfer of ownership of their respective modules to new accounts.

However, due to the absence of the #[near_bindgen] macro in each trait where the functions are defined, they are not callable directly from outside the contract, making them inaccessible for their intended purpose of ownership transfer.

Snippet of code of GlobalParams trait implementation from contract/src/global_params.rs file:

use near_sdk::borsh::{BorshDeserialize, BorshSerialize};
use near_sdk::{env, AccountId, PanicOnDefault};
use serde::{Deserialize, Serialize};

#[derive(BorshDeserialize, BorshSerialize, Deserialize, Serialize, PanicOnDefault, Clone)]
pub struct GlobalParams {
    mpc_contract: AccountId,
    fee_deposit_bps: u16,
    fee_redemption_bps: u16,
    fee_bridging_bps: u16,
    fee_babylon_rewards_bps: u16,
    btc_staking_cap: u64,
    btc_max_staking_amount: u64,
    btc_min_staking_amount: u64,
    treasury_address: String,    
    owner_id: AccountId,
}

impl GlobalParams {

    pub fn init_global_params(owner_id: AccountId, treasury_address: String) -> Self {        
        // Validate owner_id
        assert!(!owner_id.to_string().is_empty(), "Owner ID cannot be empty");

Snippet of code of ChainConfig trait implementation from contract/src/chain_config.rs file:

use near_sdk::borsh::{BorshDeserialize, BorshSerialize};
use near_sdk::serde::{Deserialize, Serialize};
use near_sdk::store::IterableMap;
use near_sdk::{env, AccountId, PanicOnDefault};
use serde_json;

#[derive(BorshDeserialize, BorshSerialize, Deserialize, Serialize, Clone, Debug)]
#[borsh(crate = "near_sdk::borsh")]
pub struct ChainConfigRecord {
    pub chain_id: String,
    pub network_type: String,
    pub network_name: String,
    pub chain_rpc_url: String,
    pub explorer_url: String,
    pub abtc_address: String,
    pub native_currency_name: String,
    pub native_currency_decimals: u8,
    pub native_currency_symbol: String,
    pub first_block: u64,
    pub batch_size: u64,
    pub gas_limit: u64,
    pub abi_path: String,
    pub validators_threshold: u8,
}

#[derive(BorshDeserialize, BorshSerialize, PanicOnDefault)]
#[borsh(crate = "near_sdk::borsh")]
pub struct ChainConfigs {
    chain_configs: IterableMap<String, ChainConfigRecord>,
    owner_id: AccountId,
}

impl ChainConfigs {
    pub fn init_chain_configs(owner_id: AccountId) -> Self {
        // Validate owner_id
        assert!(!owner_id.to_string().is_empty(), "Owner ID cannot be empty");

As a result, the ownership of the GlobalParams and ChainConfigs modules could become permanently locked to specific accounts, preventing necessary administrative actions, such as updates or parameter changes, in the future.

Although the easiest option would be to add the #[near_bindgen] macro to the trait, doing so would expose other sensitive functions, such as initialization ones. Therefore, it is recommended to create an auxiliary function in a separate module (e.g., Utils) that is accessible from outside and can execute these ownership transfer functions securely.

SOLVED: The Atlas Protocol team solved this issue by adding to the Utils file the corresponding auxiliary functions mentioned in the recommendation.

The Atlas smart contract defines two roles, admin and owner, and in both cases, the transfer of ownership is performed in a single step, which introduces a security risk.

This issue is present in multiple instances throughout the contract, where the change of ownership lacks a verification step before finalization. As a result, there is a risk that the owner or admin could make a mistake when executing the ownership transfer, such as setting an incorrect address, which could lead to the contract being locked or inaccessible.

The following functions in the Atlas contract are affected:

• change_atlas_owner_id function from contract/src/modules/admin.rs.

• change_atlas_admin_id from contract/src/modules/admin.rs.

• change_global_param_owner_id from contract/src/global_params.

• change_chain_configs_owner_id from contract/src/chain_configs.rs.

As example, this is the code of change_atlas_owner_id function from contract/src/modules/admin.rs file:

pub fn change_atlas_owner_id(&mut self, new_owner_id: AccountId) {
    self.assert_owner();

    assert!(
        !new_owner_id.to_string().is_empty(),
        "New owner ID cannot be blank"
    );
    assert_ne!(
        new_owner_id, self.owner_id,
        "New owner ID must be different from the current owner ID"
    );

    // Log the change for transparency
    env::log_str(&format!(
        "Changing Atlas owner from {} to {}",
        self.owner_id, new_owner_id
    ));

    self.owner_id = new_owner_id;
}

Additionally, the atBTC Solidity contract is also vulnerable to this issue, as it inherits from the Ownable class provided by OpenZeppelin, which does not include a two-step ownership transfer process:

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Pausable.sol";
import "@openzeppelin/contracts/access/Ownable.sol";

contract atBTC is ERC20, ERC20Burnable, ERC20Pausable, Ownable {
    constructor() ERC20("Atlas Bitcoin", "atBTC") Ownable(msg.sender) {}    

For the Atlas contract, it is recommended to implement a two-step ownership transfer process: one function to establish the new owner's address (propose_new_owner) and another to accept the transfer (accept_ownership). The latter function should only be executable by the new owner.

For the atBTC Solidity contract, it is recommended to switch from using Ownable to Ownable2Step from OpenZeppelin. This library offers a safer ownership transfer process by requiring the new owner to accept the transfer before it is finalized.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

The Atlas contract includes four fee types: deposit fees, redemption fees, bridging fees, and Babylon rewards fees.

While the necessary getter and setter functions for each fee type have been implemented, these fees have not been integrated into any calculations within the contract's logic.

The following getter functions are implemented in the src/modules/fees module and are publicly exposed through the near_bindgen macro:

#[near_bindgen]
impl Atlas {
    pub fn get_redemption_tax_amount(&self, amount: u64) -> u64 {
        assert!(amount > 0, "Amount must be greater than zero");
        let redemption_percentage = self.global_params.get_fee_redemption_bps() as u128;
        let fee = (amount as u128 * redemption_percentage / 10000) as u64;
        fee
    }

    pub fn get_deposit_tax_amount(&self, amount: u64) -> u64 {
        assert!(amount > 0, "Amount must be greater than zero");
        let deposit_percentage = self.global_params.get_fee_deposit_bps() as u128;
        let fee = (amount as u128 * deposit_percentage / 10000) as u64;
        fee
    }

    pub fn get_bridging_tax_amount(&self, amount: u64) -> u64 {
        assert!(amount > 0, "Amount must be greater than zero");
        let bridging_percentage = self.global_params.get_fee_bridging_bps() as u128;
        let fee = (amount as u128 * bridging_percentage / 10000) as u64;
        fee
    }
}

The following setter functions are implemented in the src/modules/utils module and are publicly exposed through the near_bindgen macro:

pub fn update_fee_deposit_bps(&mut self, fee_deposit_bps: u16) {
    self.global_params.update_fee_deposit_bps(fee_deposit_bps);
}

pub fn update_fee_redemption_bps(&mut self, fee_redemption_bps: u16) {
    self.global_params.update_fee_redemption_bps(fee_redemption_bps);
}

pub fn update_fee_bridging_bps(&mut self, fee_bridging_bps: u16) {
    self.global_params.update_fee_bridging_bps(fee_bridging_bps);
}

pub fn update_fee_babylon_rewards_bps(&mut self, fee_babylon_rewards_bps: u16) {
    self.global_params.update_fee_babylon_rewards_bps(fee_babylon_rewards_bps);
}

For example, we can examine where the fee_deposit_bps parameter in the global config is used by executing the following command with the ripgrep CLI tool: rg self.fee_deposit_bps from the contract folder:

src/global_params.rs
83:        self.fee_deposit_bps
117:        self.fee_deposit_bps = fee_deposit_bps;

This means that it is only utilized within the get_fee_deposit_bps and update_fee_deposit_bps functions in the GlobalParams struct.

In this context, we are particularly focused on the global_params.get_fee_deposit_bps() getter function which is used exclusively within the fees::get_deposit_tax_amount function and the utils module integration tests:

src/modules/fees.rs
17:        let deposit_percentage = self.global_params.get_fee_deposit_bps() as u128;

tests/utils_tests.rs
35:        assert_eq!(atlas.global_params.get_fee_deposit_bps(), new_fee);
336:        assert!(atlas.global_params.get_fee_deposit_bps() > 0);

The get_deposit_tax_amount() function is used solely within the integration tests of the fees module.

src/modules/fees.rs
15:    pub fn get_deposit_tax_amount(&self, amount: u64) -> u64 {

tests/fees_tests.rs
22:    let fee = atlas.get_deposit_tax_amount(amount);
84:        atlas.get_deposit_tax_amount(amount);
106:    let fee = atlas.get_deposit_tax_amount(amount);

The same approach can be applied to demonstrate that the other fee types are not utilized in the contract's calculations.

To avoid unnecessary contract binary size bloat, it is recommended to either incorporate the implemented fee mechanism into the contract logic or remove it entirely.

SOLVED: The Atlas Protocol team solved the issue by removing the fees module, which contained the getters for the protocol fees. Although the various fee fields remain in the GlobalParams struct, along with their setters, the team indicated that these fields are incorporated on the frontend instead of within the contract, in order to reduce gas costs.

Every Solidity file specifies in the header a version number of the format pragma solidity (^)0.8.* .
The caret ( ^ ) before the version number implies an unlocked pragma, meaning that the compiler will use the specified version and above, hence the term "unlocked".

Contracts should be deployed using the same compiler version and flags that were used during their development and testing phases. Locking the pragma ensures consistency and prevents unintended deployment with a different pragma version. Using an outdated pragma version can introduce bugs that may adversely affect the contract system.

The atBTC.sol contract has unlocked pragma:

pragma solidity ^0.8.0;

Consider locking the pragma version in the atBTC smart contract. It is not recommended to use a floating pragma in production.
For example: pragma solidity 0.8.21;

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

Several functions in the atBTC contract are marked as public visibility but are never internally called by any other function. To optimize gas usage when these functions are invoked, consider marking them with external visibility.

The affected functions are:

• pause

• unpause

• mintDeposit

• mintBridge

• burnRedeem

• burnBridge

Consider changing the visibility of the mentioned functions to external instead of public.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

During the creation of the atBTC contract, a manual state existence check was implemented in the new function. However, the new function is also marked with #[init] macro which implements this behavior by default, making the manual assertion redundant.

#[init]
pub fn new(owner_id: AccountId, metadata: FungibleTokenMetadata) -> Self {
    assert!(!env::state_exists(), "Already initialized");
    ...

It is recommended to remove the redundant state existence check.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

In Solidity smart contract development, replacing hard-coded revert message strings with the Error() syntax is an optimization strategy that can significantly reduce gas costs. Hard-coded strings, stored on the blockchain, increase the size and cost of deploying and executing contracts.

The Error() syntax allows for the definition of reusable, parameterized custom errors, leading to a more efficient use of storage and reduced gas consumption. This approach not only optimizes gas usage during deployment and interaction with the contract but also enhances code maintainability and readability by providing clearer, context-specific error information.

It is recommended to replace hard-coded revert strings in require statements with custom errors, as outlined in the steps below:

1. Standard require statement (to be replaced):

   require(condition, "Condition not met");

   
   

2. Declare the custom error:

   error ConditionNotMet();

   
   

3. Use the custom error in require or if statements.

   Using custom errors in require statements is supported only in Solidity versions 0.8.26 and above. For older compiler versions, consider using if statements followed by a revert with the custom error.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

In the codebase, several instances of inefficient code were identified.

The admin::new function validates the global_params_owner_id and the treasury_address alongside the other provided arguments:

#[init]
pub fn new(atlas_owner_id: AccountId, atlas_admin_id: AccountId, global_params_owner_id: AccountId, chain_configs_owner_id: AccountId, treasury_address: String) -> Self {
    env::log_str("Initializing Atlas");

    // Validate input parameters
    assert!(!atlas_owner_id.to_string().is_empty(), "Atlas owner ID cannot be empty");
    assert!(!atlas_admin_id.to_string().is_empty(), "Atlas admin ID cannot be empty");
    assert!(!global_params_owner_id.to_string().is_empty(), "Global params owner ID cannot be empty");
    assert!(!chain_configs_owner_id.to_string().is_empty(), "Chain configs owner ID cannot be empty");
    assert!(!treasury_address.is_empty(), "Treasury address cannot be empty");

Then, the admin::new function calls the GlobalParams::init_global_params function:

global_params: GlobalParams::init_global_params(global_params_owner_id, treasury_address),

The GlobalParams::init_global_params function re-checks the global_params_owner_id and the treasury_address parameters:

pub fn init_global_params(owner_id: AccountId, treasury_address: String) -> Self {        
// Validate owner_id
assert!(!owner_id.to_string().is_empty(), "Owner ID cannot be empty");
        
// Validate treasury_address
assert!(!treasury_address.is_empty(), "Treasury address cannot be empty");

In the deposits::increment_deposit_verified_count function, the updated deposit is being unnecessarily cloned before being inserted back, as we can rely on the btc_txn_hash that comes from the mempool_deposit input:

// Clone deposit before inserting it to avoid moving it
let cloned_deposit = deposit.clone();

// Update the deposit record in the map
self.deposits.insert(deposit.btc_txn_hash.clone(), cloned_deposit);
        
// Add the caller to the list of validators for this btc_txn_hash
validators_list.push(caller);
self.verifications.insert(deposit.btc_txn_hash.clone(), validators_list);

In the redemptions::increment_redemption_verified_count function, the same logic applies for the updated redemption record being unnecessarily cloned:

// Clone redemption before inserting it to avoid moving it
let cloned_redemption = redemption.clone();

// Update the redemption record in the map
self.redemptions.insert(redemption.txn_hash.clone(), cloned_redemption);

// Add the caller to the list of validators for this txn_hash
validators_list.push(caller);
self.verifications.insert(redemption.txn_hash.clone(), validators_list);

It is recommended to:

1. Remove Redundant Checks: Eliminate unnecessary checks from the admin::new function, as these validations are already handled within the GlobalParams::init_global_params function.

2. Avoid Unnecessary Record Cloning:

   • Use mempool_deposit.btc_txn_hash instead of deposit.btc_txn_hash.

   • Use mempool_redemption.txn_hash instead of redemption.txn_hash.

   • Remove the clone() call in the final usage of the transaction hash when inserting into the verifications map.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

Incorporating documentation and comments into the codebase is essential for elucidating key aspects and facilitating comprehension of the developer's intentions by others. However, the codebase contains misleading documentation and comments.

In the src/modules/deposit.rs file, the increment_deposit_verified_count function is documented as follows: // Caller of this function has to be an authorised validator for the particular chain_id of the redemption record. However, the caller must be an authorized validator for the Bitcoin chain ID, even though the chain ID is currently set to SIGNET (the Bitcoin testnet).

// Increments deposit record's verified_count by 1 based on the mempool_deposit record passed in
// Caller of this function has to be an authorised validator for the particular chain_id of the redemption record
// Caller of this function has to be a new validator of this btc_txn_hash
// Checks all fields of mempool_record equal to deposit record
// Returns true if verified_count incremented successfully and returns false if not incremented
pub fn increment_deposit_verified_count(&mut self, mempool_deposit: DepositRecord) -> bool {
    ...
    let chain_id = SIGNET.to_string();
            
    // Use the is_validator function to check if the caller is authorized for the bitcoin deposit
    if self.is_validator(&caller, &chain_id) {
    ...
}

It is recommended to correct the misleading comment for accuracy.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id. The Atlas contract now includes a production_mode state variable, which determines whether the caller must be an authorized validator for Bitcoin or Signet, depending on the production mode.

Within the contracts in scope, the following typographical error was identified:

"Receiving adress is not a valid EVM address".to_string();

It is recommended to fix the identified typographical error as follows:

adress -> address

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

Contract size directly corresponds to the costs associated with its operation, mainly - the deployment. Although many of the strategies aimed at reducing the compiled binary size achieve this goal at the expense of code readability, there are some measures that could be implemented without such sacrifices.

It was observed that Cargo.toml files of atlas and atBTC contracts specified the crate-type as both cdylib and rlib, however usually only cdylib is necessary. By specifying the crate type as cdylib for both contracts and configuring the release profile with an optimization level of z, a codegen units setting of 1, and enabling LTO (link time optimizations) for the atBTC contract, we achieved a reduction in the WASM binary size of approximately 22.1% and 37.15%, respectively.

It is recommended to implement the specified profile release optimizations in the atBTC contract's manifest (Cargo.toml). Additionally, for both contracts, ensure that the crate type does not include rlib unless it is necessary.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

The atlas contract implements a series of utility functions that are responsible for verifying certain aspects of execution, for example, who called the function. Those functions are intended to be used internally. However, they are marked with a pub keyword, which instructs the near_bindgen macro to make them callable on the blockchain.

The src/modules/admin.rs module exposes unnecessarily the following utility functions:

pub fn assert_owner(&self) {
    assert_eq!(self.owner_id, env::predecessor_account_id(), "Only the owner can call this method");
}
    
pub fn assert_admin(&self) {
    assert_eq!(self.admin_id, env::predecessor_account_id(), "Only the admin can call this method");
}

It is recommended to make the mentioned functions private by decorating them with the #[private] macro. This ensures that the functions can only be called by the atlas contract itself.

SOLVED: The Atlas Protocol team solved the issue in the specified commit id.

Unit tests are designed to verify the correctness of individual components in a smart contract. They heavily rely on assertions to validate the execution of the contract against its inputs and outputs.

In integration testing, individual components of the smart contract are tested collectively to verify that complex dependencies, interactions, and cross-contract calls function as intended.

The following issues were identified in the current tests:

Outdated tests: the atBTC contract lacks adequate unit tests, as they are outdated and cause execution failures when running cargo test for the following reasons:

• When calling the minting feature, the mint function name was used, which is not one of the contract's exposed functions.

• When setting up an instance of the atBTC contract, the current implementation requires two arguments. However, there is an extra argument at the end, specifically near_sdk::json_types::U128(10000000) indicating the total_supply argument most likely.

  
  

Below is an example of the test_mint unit test that illustrates both of the aforementioned errors:

#[test]
fn test_mint() {
    let mut context = get_context(accounts(1));
    testing_env!(context.build());
    let mut contract = Contract::new(
        accounts(1).into(),
        FungibleTokenMetadata {
            spec: FT_METADATA_SPEC.to_string(),
            name: "Example NEAR fungible token".to_string(),
            symbol: "EXAMPLE".to_string(),
            icon: Some(DATA_IMAGE_SVG_NEAR_ICON.to_string()),
            reference: None,
            reference_hash: None,
            decimals: 24,
        },
        near_sdk::json_types::U128(10000000),
    );

    // Mint some tokens with a dummy BTC transaction hash
    let btc_txn_hash = "dummy_btc_txn_hash".to_string();
    contract.mint(accounts(1).into(), U128(1000), btc_txn_hash);
    testing_env!(context.is_view(true).build());
    assert_eq!(contract.ft_total_supply().0, 1000);
    assert_eq!(contract.ft_balance_of(accounts(1)).0, 1000);
}

Missing integration tests: Although the Atlas contract interacts with the atBTC contract—specifically within the create_mint_abtc_signed_tx function—the lack of testing for this critical function poses a risk of unexpected behavior.

To resolve the mentioned issues, it is recommended to:

• Replace mint with mint_deposit and remove the last argument previously provided to Contract::new. Below is the corrected implementation of the test_mint unit test:

#[test]
fn test_mint() {
    let mut context = get_context(accounts(1));
    testing_env!(context.build());
    let mut contract = Contract::new(
        accounts(1).into(),
        FungibleTokenMetadata {
            spec: FT_METADATA_SPEC.to_string(),
            name: "Example NEAR fungible token".to_string(),
            symbol: "EXAMPLE".to_string(),
            icon: Some(DATA_IMAGE_SVG_NEAR_ICON.to_string()),
            reference: None,
            reference_hash: None,
            decimals: 24,
        },
    );

    // Mint some tokens with a dummy BTC transaction hash
    let btc_txn_hash = "dummy_btc_txn_hash".to_string();
    contract.mint_deposit(accounts(1).into(), U128(1000), btc_txn_hash);
    testing_env!(context.is_view(true).build());
    assert_eq!(contract.ft_total_supply().0, 1000);
    assert_eq!(contract.ft_balance_of(accounts(1)).0, 1000);
    }

• Implement comprehensive integration tests to ensure the correctness of the Atlas core functions, with particular emphasis on the create_mint_abtc_signed_tx function.

SOLVED: The Atlas Protocol team solved the issue in the following commit ids:

• f209860

• 0672b48