Skip to content

Internal Audit Comments #339

@eigmax

Description

@eigmax
  • Remove let network = get_network(); if there is already a btc_client available to use: Get the network by calling btc_client.network().
  • Miss error handler, one of the examples below:
  // it will happened when handle history events
            warn!("Instance {instance_id} is finished but not find in db. we will create it");
            if let Some(extra) = tx_record.extra.as_ref()
                && let Ok(bridge_event) = serde_json::from_str::<BridgeInRequestEvent>(extra)
                && let Ok((mut instance, _)) = generate_instance_from_bridge_in_request_event(
                    btc_client.as_ref(),
                    goat_client.as_ref(),
                    &bridge_event,
                    false,
                )
                .await
            {
                info!("Instance {instance_id} is created and set status to RelayerL2Minted");
                instance.status = InstanceBridgeInStatus::RelayerL2Minted.to_string();
                storage_processor.upsert_instance(&instance).await?;
            }

In this way we miss lots of errors. A better way to implement can be either checking all the function call and printing the error if applied, or create a single function to handle the logic in this condition like:

if let Some(extra) = tx_record.extra.as_ref() {
     try_insert_instance(....)
}
  • Remove strip_hex_prefix_owned , and use crate::util::hex_parse to parse the hex string, i.e. handle_swap_claim_events.

  • Invalid value.

            let mut claim_amount =
                U256::from_str(&bridge_out_global_stats.claim_amount).unwrap_or_default();
            claim_amount
                .add_assign(&U256::from_str(&instance.bridge_out_amount).unwrap_or_default());

We should do rigorous check about the fund values.

  • Code style. For example,
for input in &inputs {
            if !outpoint_available(btc_client, &input.outpoint.txid, input.outpoint.vout.into())
                .await?
            {
                input_utxo_available = false;
                break;
            }
        }

can be changed to

        // Check all inputs concurrently; short-circuit on the first error.
        let avail_vec = try_join_all(inputs.iter().map(|input| {
            outpoint_available(btc_client, &input.outpoint.txid, input.outpoint.vout.into())
        })).await?;
        input_utxo_available = avail_vec.into_iter().all(|v| v);
  • Key rotation not compatible for all roles.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions