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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions