Miscellaneous code improvements for Orchestrator

[rnbguy]

Surfaced from @informalsystems audit of Althea Gravity Bridge at commit 19a4cfe

severity: Informational
type: Restructuring proposal
difficulty: Easy

Involved artifacts

• Orchestrator

Description

• orchestrator/cosmos_gravity/src/send.rs#L264

  Consider using BTreeMap to sort the msg.

  The current code uses HashMap then sorts the keys in a Vec then collects the values from HashMap.
  Using BTreeMap will remove the need of sorting the keys separately in Vec.

• Consider simplifying error handling using thiserror.

  To effectively handle all errors globally (probably from hereorchestrator/gravity_utils/src/error.rs), we recommend to add global struct deriving thiserror::Error for it.
  Propagate all errors to fn main() and use an error handler to execute final steps in case of error.

  #[tokio::main]
  async fn main() {
      if let Err(e) = app.run().await {
          e.error_exit()
      }
  }

• One can avoid unnecessary Vec::insert calls at orchestrator/ethereum_gravity/src/utils.rs#L13-L21 and orchestrator/ethereum_gravity/src/utils.rs#L29-L37

  let mut val = input.to_bytes_be();
  // pad to 8 bytes
  while val.len() < 8 {
      val.insert(0, 0);
  }
  let mut lower_bytes: [u8; 8] = [0; 8];
  // get the 'lowest' 8 bytes from a 256 bit integer
  lower_bytes.copy_from_slice(&val[0..val.len()]);
  Some(u64::from_be_bytes(lower_bytes))

  may be transformed to,

  let mut val = input.to_bytes_be();
  let mut lower_bytes: [u8; 8] = [0; 8];
  // get the start index after the trailing zeros
  let start_index = 8 - val.len();
  // get the 'lowest' 8 bytes from a 256 bit integer
  lower_bytes[start_index..].copy_from_slice(val.as_slice());
  Some(u64::from_be_bytes(lower_bytes))

• The exit should be avoided and should push the error through the function call stack.

  • orchestrator/gbt/src/orchestrator.rs#L104-L110

  • orchestrator/gbt/src/orchestrator.rs#L113-L114

    Because this makes the caller function not be aware of unexpected error scenarios.

• Misleading naming conventions.

  Example case: orchestrator/cosmos_gravity/src/query.rs#L150-L155

  let request = client
          .batch_confirms(QueryBatchConfirmsRequest {
              nonce,
              contract_address: contract_address.to_string(),
          })
          .await?;

  Here, request variable is actually a Response struct.