diff --git a/.github/workflows/lints.yaml b/.github/workflows/lints.yaml new file mode 100644 index 00000000..9479a283 --- /dev/null +++ b/.github/workflows/lints.yaml @@ -0,0 +1,28 @@ +name: Rust testing + +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] + +jobs: + clippy: + runs-on: ubuntu-latest + strategy: + matrix: + component: [near, omni-relayer] + steps: + - name: Checkout code + uses: actions/checkout@v3 + + - name: Set up Rust + uses: actions-rs/toolchain@v1 + with: + toolchain: 1.79.0 + components: clippy + + - name: Run Clippy + run: | + make rust-lint-${{ matrix.component }} + diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..ed9fc568 --- /dev/null +++ b/Makefile @@ -0,0 +1,11 @@ +.PHONY: rust-lint rust-lint-near rust-lint-omni-relayer + +OPTIONS = -D warnings -D clippy::pedantic -A clippy::missing_errors_doc -A clippy::must_use_candidate -A clippy::module_name_repetitions + +rust-lint: rust-lint-near rust-lint-relayer + +rust-lint-near: + cargo clippy --manifest-path ./near/Cargo.toml -- $(OPTIONS) + +rust-lint-omni-relayer: + cargo clippy --manifest-path ./omni-relayer/Cargo.toml -- $(OPTIONS) diff --git a/near/mock/mock-prover/src/lib.rs b/near/mock/mock-prover/src/lib.rs index 8a630cec..5a13c4e0 100644 --- a/near/mock/mock-prover/src/lib.rs +++ b/near/mock/mock-prover/src/lib.rs @@ -24,20 +24,23 @@ impl Default for OmniProver { #[near] impl OmniProver { - pub fn add_prover(&mut self, prover_id: ProverId, account_id: AccountId) { - self.provers.insert(&prover_id, &account_id); + pub fn add_prover(&mut self, prover_id: &ProverId, account_id: &AccountId) { + self.provers.insert(prover_id, account_id); } - pub fn remove_prover(&mut self, prover_id: ProverId) { - self.provers.remove(&prover_id); + pub fn remove_prover(&mut self, prover_id: &ProverId) { + self.provers.remove(prover_id); } pub fn get_provers(&self) -> Vec<(ProverId, AccountId)> { self.provers.iter().collect::>() } + /// # Panics + /// + /// This function will panic if the prover args are not valid. #[result_serializer(borsh)] - pub fn verify_proof(&self, #[serializer(borsh)] args: VerifyProofArgs) -> ProverResult { + pub fn verify_proof(&self, #[serializer(borsh)] args: &VerifyProofArgs) -> ProverResult { ProverResult::try_from_slice(&args.prover_args).unwrap() } } diff --git a/near/mock/mock-token/src/lib.rs b/near/mock/mock-token/src/lib.rs index bf0919a6..9d24a28e 100644 --- a/near/mock/mock-token/src/lib.rs +++ b/near/mock/mock-token/src/lib.rs @@ -35,11 +35,11 @@ impl Contract { /// Initializes the contract with the given total supply owned by the given `owner_id` with /// default metadata (for example purposes only). #[init] - pub fn new_default_meta(owner_id: AccountId, total_supply: U128) -> Self { + pub fn new_default_meta(owner_id: &AccountId, total_supply: U128) -> Self { Self::new( owner_id, total_supply, - FungibleTokenMetadata { + &FungibleTokenMetadata { spec: FT_METADATA_SPEC.to_string(), name: "Example NEAR fungible token".to_string(), symbol: "EXAMPLE".to_string(), @@ -54,18 +54,18 @@ impl Contract { /// Initializes the contract with the given total supply owned by the given `owner_id` with /// the given fungible token metadata. #[init] - pub fn new(owner_id: AccountId, total_supply: U128, metadata: FungibleTokenMetadata) -> Self { + pub fn new(owner_id: &AccountId, total_supply: U128, metadata: &FungibleTokenMetadata) -> Self { require!(!env::state_exists(), "Already initialized"); metadata.assert_valid(); let mut this = Self { token: FungibleToken::new(StorageKey::FungibleToken), - metadata: LazyOption::new(StorageKey::Metadata, Some(&metadata)), + metadata: LazyOption::new(StorageKey::Metadata, Some(metadata)), }; - this.token.internal_register_account(&owner_id); - this.token.internal_deposit(&owner_id, total_supply.into()); + this.token.internal_register_account(owner_id); + this.token.internal_deposit(owner_id, total_supply.into()); near_contract_standards::fungible_token::events::FtMint { - owner_id: &owner_id, + owner_id, amount: total_supply, memo: Some("new tokens are minted"), } @@ -79,7 +79,7 @@ impl Contract { impl FungibleTokenCore for Contract { #[payable] fn ft_transfer(&mut self, receiver_id: AccountId, amount: U128, memo: Option) { - self.token.ft_transfer(receiver_id, amount, memo) + self.token.ft_transfer(receiver_id, amount, memo); } #[payable] diff --git a/near/nep141-locker/src/lib.rs b/near/nep141-locker/src/lib.rs index c68a3e0a..43fa9ea5 100644 --- a/near/nep141-locker/src/lib.rs +++ b/near/nep141-locker/src/lib.rs @@ -45,7 +45,7 @@ const STORAGE_BALANCE_OF_GAS: Gas = Gas::from_tgas(3); const STORAGE_DEPOSIT_GAS: Gas = Gas::from_tgas(3); const NO_DEPOSIT: NearToken = NearToken::from_near(0); const ONE_YOCTO: NearToken = NearToken::from_yoctonear(1); -const NEP141_DEPOSIT: NearToken = NearToken::from_yoctonear(1250000000000000000000); +const NEP141_DEPOSIT: NearToken = NearToken::from_yoctonear(1_250_000_000_000_000_000_000); const SIGN_PATH: &str = "bridge-1"; @@ -196,7 +196,7 @@ impl Contract { contract } - pub fn log_metadata(&self, token_id: AccountId) -> Promise { + pub fn log_metadata(&self, token_id: &AccountId) -> Promise { ext_token::ext(token_id.clone()) .with_static_gas(LOG_METADATA_GAS) .ft_metadata() @@ -213,7 +213,7 @@ impl Contract { pub fn log_metadata_callbcak( &self, #[callback] metadata: FungibleTokenMetadata, - token_id: AccountId, + token_id: &AccountId, ) -> Promise { let metadata_payload = MetadataPayload { token: token_id.to_string(), @@ -357,12 +357,18 @@ impl Contract { } } + /// # Panics + /// + /// This function will panic under the following conditions: + /// + /// - If the `borsh::to_vec` serialization of the `TransferMessagePayload` fails. + /// - If a `fee` is provided and it doesn't match the fee in the stored transfer message. #[payable] pub fn sign_transfer( &mut self, nonce: U128, fee_recipient: Option, - fee: Option, + fee: &Option, ) -> Promise { let transfer_message = self.get_transfer_message(nonce); if let Some(fee) = &fee { @@ -390,7 +396,7 @@ impl Contract { .then( Self::ext(env::current_account_id()) .with_static_gas(SIGN_TRANSFER_CALLBACK_GAS) - .sign_transfer_callback(transfer_payload, transfer_message.fee), + .sign_transfer_callback(transfer_payload, &transfer_message.fee), ) } @@ -399,7 +405,7 @@ impl Contract { &mut self, #[callback_result] call_result: Result, #[serializer(borsh)] message_payload: TransferMessagePayload, - #[serializer(borsh)] fee: Fee, + #[serializer(borsh)] fee: &Fee, ) { if let Ok(signature) = call_result { let nonce = message_payload.nonce; @@ -442,7 +448,7 @@ impl Contract { .with_attached_deposit(attached_deposit) .with_static_gas(CLAIM_FEE_CALLBACK_GAS) .fin_transfer_callback( - args.storage_deposit_args, + &args.storage_deposit_args, env::predecessor_account_id(), args.native_fee_recipient, ), @@ -453,7 +459,7 @@ impl Contract { #[payable] pub fn fin_transfer_callback( &mut self, - #[serializer(borsh)] storage_deposit_args: StorageDepositArgs, + #[serializer(borsh)] storage_deposit_args: &StorageDepositArgs, #[serializer(borsh)] predecessor_account_id: AccountId, #[serializer(borsh)] native_fee_recipient: OmniAddress, ) -> PromiseOrValue { @@ -684,14 +690,15 @@ impl Contract { pub fn get_transfer_message(&self, nonce: U128) -> TransferMessage { self.pending_transfers .get(&nonce.0) - .map(|m| m.into_main().message) + .map(storage::TransferMessageStorage::into_main) + .map(|m| m.message) .sdk_expect("The transfer does not exist") } pub fn get_transfer_message_storage(&self, nonce: U128) -> TransferMessageStorageValue { self.pending_transfers .get(&nonce.0) - .map(|m| m.into_main()) + .map(storage::TransferMessageStorage::into_main) .sdk_expect("The transfer does not exist") } @@ -740,7 +747,7 @@ impl Contract { .flatten() .is_some() } - _ => false, + PromiseResult::Failed => false, } } @@ -786,7 +793,7 @@ impl Contract { let transfer = self .pending_transfers .remove(&nonce) - .map(|m| m.into_main()) + .map(storage::TransferMessageStorage::into_main) .sdk_expect("ERR_TRANSFER_NOT_EXIST"); let refund = diff --git a/near/nep141-locker/src/storage.rs b/near/nep141-locker/src/storage.rs index 6415be5d..337f3fe9 100644 --- a/near/nep141-locker/src/storage.rs +++ b/near/nep141-locker/src/storage.rs @@ -2,7 +2,10 @@ use near_contract_standards::storage_management::{StorageBalance, StorageBalance use near_sdk::{assert_one_yocto, borsh}; use near_sdk::{env, near_bindgen, AccountId, NearToken}; -use crate::*; +use crate::{ + require, BorshDeserialize, BorshSerialize, ChainKind, Contract, ContractExt, Deserialize, Fee, + OmniAddress, Promise, SdkExpect, Serialize, TransferMessage, U128, +}; #[derive(BorshDeserialize, BorshSerialize, Serialize, Deserialize, Debug, Clone)] pub struct TransferMessageStorageValue { @@ -111,7 +114,7 @@ impl Contract { } pub fn storage_balance_of(&self, account_id: &AccountId) -> Option { - self.accounts_balances.get(&account_id) + self.accounts_balances.get(account_id) } pub fn required_balance_for_account(&self) -> NearToken { diff --git a/near/omni-prover/evm-prover/src/lib.rs b/near/omni-prover/evm-prover/src/lib.rs index 9087a882..b8d4a2a2 100644 --- a/near/omni-prover/evm-prover/src/lib.rs +++ b/near/omni-prover/evm-prover/src/lib.rs @@ -42,9 +42,13 @@ impl EvmProver { } } + /// # Panics + /// + /// This function will panic in the following situations: + /// - If the log entry at the specified index doesn't match the decoded log entry. #[handle_result] - pub fn verify_proof(&self, #[serializer(borsh)] input: Vec) -> Result { - let args = EvmVerifyProofArgs::try_from_slice(&input).map_err(|_| "ERR_PARSE_ARGS")?; + pub fn verify_proof(&self, #[serializer(borsh)] input: &[u8]) -> Result { + let args = EvmVerifyProofArgs::try_from_slice(input).map_err(|_| "ERR_PARSE_ARGS")?; let evm_proof = args.proof; let header: BlockHeader = rlp::decode(&evm_proof.header_data).map_err(|e| e.to_string())?; @@ -76,7 +80,7 @@ impl EvmProver { .with_static_gas(VERIFY_PROOF_CALLBACK_GAS) .verify_proof_callback( args.proof_kind, - evm_proof.log_entry_data, + &evm_proof.log_entry_data, header.hash.ok_or("ERR_HASH_NOT_SET")?.0, ), )) @@ -87,7 +91,7 @@ impl EvmProver { pub fn verify_proof_callback( &mut self, #[serializer(borsh)] kind: ProofKind, - #[serializer(borsh)] log_entry_data: Vec, + #[serializer(borsh)] log_entry_data: &[u8], #[serializer(borsh)] expected_block_hash: H256, #[callback] #[serializer(borsh)] @@ -133,11 +137,11 @@ impl EvmProver { actual_key.push(el / 16); actual_key.push(el % 16); } - Self::_verify_trie_proof(expected_root.to_vec(), &actual_key, proof, 0, 0) + Self::_verify_trie_proof(&expected_root, &actual_key, proof, 0, 0) } fn _verify_trie_proof( - expected_root: Vec, + expected_root: &[u8], key: &Vec, proof: &Vec>, key_index: usize, @@ -147,15 +151,15 @@ impl EvmProver { if key_index == 0 { // trie root is always a hash - assert_eq!(keccak256(node), expected_root.as_slice()); + assert_eq!(keccak256(node), expected_root); } else if node.len() < 32 { // if rlp < 32 bytes, then it is not hashed assert_eq!(node.as_slice(), expected_root); } else { - assert_eq!(keccak256(node), expected_root.as_slice()); + assert_eq!(keccak256(node), expected_root); } - let node = Rlp::new(&node.as_slice()); + let node = Rlp::new(node.as_slice()); if node.iter().count() == 17 { // Branch node @@ -164,17 +168,17 @@ impl EvmProver { get_vec(&node, 16) } else { let new_expected_root = get_vec(&node, key[key_index] as usize); - if !new_expected_root.is_empty() { + if new_expected_root.is_empty() { + // not included in proof + vec![] + } else { Self::_verify_trie_proof( - new_expected_root, + &new_expected_root, key, proof, key_index + 1, proof_index + 1, ) - } else { - // not included in proof - vec![] } } } else { @@ -210,7 +214,7 @@ impl EvmProver { assert_eq!(path.as_slice(), &key[key_index..key_index + path.len()]); let new_expected_root = get_vec(&node, 1); Self::_verify_trie_proof( - new_expected_root, + &new_expected_root, key, proof, key_index + path.len(), diff --git a/near/omni-prover/wormhole-omni-prover-proxy/src/lib.rs b/near/omni-prover/wormhole-omni-prover-proxy/src/lib.rs index 3e9647ae..0760605e 100644 --- a/near/omni-prover/wormhole-omni-prover-proxy/src/lib.rs +++ b/near/omni-prover/wormhole-omni-prover-proxy/src/lib.rs @@ -8,12 +8,12 @@ use omni_types::prover_result::{ProofKind, ProverResult}; mod byte_utils; mod parsed_vaa; -/// Gas to call verify_log_entry on prover. +/// Gas to call `verify_log_entry` on prover. pub const VERIFY_LOG_ENTRY_GAS: Gas = Gas::from_tgas(50); #[ext_contract(ext_prover)] pub trait Prover { - fn verify_vaa(&self, vaa: &String) -> u32; + fn verify_vaa(&self, vaa: &str) -> u32; } #[near_bindgen] @@ -31,11 +31,11 @@ impl WormholeOmniProverProxy { Self { prover_account } } - pub fn verify_proof(&self, #[serializer(borsh)] input: Vec) -> Promise { - let args = WormholeVerifyProofArgs::try_from_slice(&input) + pub fn verify_proof(&self, #[serializer(borsh)] input: &[u8]) -> Promise { + let args = WormholeVerifyProofArgs::try_from_slice(input) .unwrap_or_else(|_| env::panic_str("ErrorOnArgsParsing")); - env::log_str(&format!("{}", args.vaa)); + env::log_str(&args.vaa.to_string()); ext_prover::ext(self.prover_account.clone()) .with_static_gas(VERIFY_LOG_ENTRY_GAS) @@ -47,13 +47,19 @@ impl WormholeOmniProverProxy { ) } + /// # Panics + /// + /// This function will panic in the following situations: + /// - If the `vaa` string cannot be decoded as a valid hexadecimal string. + /// - If the `ParsedVAA::parse` function fails to parse the decoded VAA data. + /// - If the `proof_kind` doesn't match the first byte of the VAA payload. #[private] #[handle_result] pub fn verify_vaa_callback( &mut self, proof_kind: ProofKind, vaa: String, - #[callback_result] gov_idx: Result, + #[callback_result] gov_idx: &Result, ) -> Result { if gov_idx.is_err() { return Err("Proof is not valid!".to_owned()); diff --git a/near/omni-prover/wormhole-omni-prover-proxy/src/parsed_vaa.rs b/near/omni-prover/wormhole-omni-prover-proxy/src/parsed_vaa.rs index 75b8159e..a5d8e8ac 100644 --- a/near/omni-prover/wormhole-omni-prover-proxy/src/parsed_vaa.rs +++ b/near/omni-prover/wormhole-omni-prover-proxy/src/parsed_vaa.rs @@ -11,7 +11,7 @@ use { }; // Validator Action Approval(VAA) data - +#[allow(dead_code)] pub struct ParsedVAA { pub version: u8, pub guardian_set_index: u32, @@ -69,7 +69,7 @@ impl ParsedVAA { // Load 4 bytes starting from index 1 let guardian_set_index: u32 = data.get_u32(Self::GUARDIAN_SET_INDEX_POS); let len_signers = data.get_u8(Self::LEN_SIGNER_POS) as usize; - let body_offset: usize = Self::HEADER_LEN + Self::SIGNATURE_LEN * len_signers as usize; + let body_offset: usize = Self::HEADER_LEN + Self::SIGNATURE_LEN * len_signers; // Hash the body if body_offset >= data.len() { @@ -99,7 +99,7 @@ impl ParsedVAA { guardian_set_index, timestamp, nonce, - len_signers: len_signers as usize, + len_signers, emitter_chain, emitter_address, sequence, diff --git a/near/omni-tests/src/lib.rs b/near/omni-tests/src/lib.rs index f9fe2017..f18fb1c3 100644 --- a/near/omni-tests/src/lib.rs +++ b/near/omni-tests/src/lib.rs @@ -5,7 +5,7 @@ mod tests { use omni_types::{ locker_args::{FinTransferArgs, StorageDepositArgs}, prover_result::{InitTransferMessage, ProverResult}, - OmniAddress, TransferMessage, + Fee, OmniAddress, TransferMessage, }; const MOCK_TOKEN_PATH: &str = "./../target/wasm32-unknown-unknown/release/mock_token.wasm"; @@ -209,6 +209,7 @@ mod tests { .call(locker_contract.id(), "fin_transfer") .args_borsh(FinTransferArgs { chain_kind: omni_types::ChainKind::Eth, + native_fee_recipient: eth_eoa_address(), storage_deposit_args: StorageDepositArgs { token: token_contract.id().clone(), accounts: storage_deposit_accounts, @@ -220,7 +221,10 @@ mod tests { token: token_contract.id().clone(), recipient: OmniAddress::Near(account_1().to_string()), amount: U128(amount), - fee: U128(fee), + fee: Fee { + fee: U128(fee), + native_fee: U128(0), + }, sender: eth_eoa_address(), }, })) diff --git a/near/omni-types/src/evm/events.rs b/near/omni-types/src/evm/events.rs index dc39b7e9..6e74ed83 100644 --- a/near/omni-types/src/evm/events.rs +++ b/near/omni-types/src/evm/events.rs @@ -40,12 +40,12 @@ sol! { pub fn parse_evm_event>>( chain_kind: ChainKind, - log_rlp: Vec, + log_rlp: &[u8], ) -> Result where >>::Error: std::fmt::Display, { - let rlp_decoded = Log::decode(&mut log_rlp.as_slice()).map_err(stringify)?; + let rlp_decoded = Log::decode(&mut log_rlp.to_vec().as_slice()).map_err(stringify)?; V::try_from_log( chain_kind, T::decode_log(&rlp_decoded, true).map_err(stringify)?, diff --git a/near/omni-types/src/lib.rs b/near/omni-types/src/lib.rs index 6382fb85..f03a0e81 100644 --- a/near/omni-types/src/lib.rs +++ b/near/omni-types/src/lib.rs @@ -198,7 +198,7 @@ impl fmt::Display for OmniAddress { OmniAddress::Arb(recipient) => ("arb", recipient.to_string()), OmniAddress::Base(recipient) => ("base", recipient.to_string()), }; - write!(f, "{}:{}", chain_str, recipient) + write!(f, "{chain_str}:{recipient}") } } diff --git a/near/omni-types/src/mpc_types.rs b/near/omni-types/src/mpc_types.rs index ed5a59d8..fb3320d9 100644 --- a/near/omni-types/src/mpc_types.rs +++ b/near/omni-types/src/mpc_types.rs @@ -19,9 +19,19 @@ pub struct SignatureResponse { } impl SignatureResponse { + /// # Panics + /// + /// This function will panic in the following cases: + /// - If `self.big_r.affine_point` is not a valid hexadecimal string. + /// - If `self.s.scalar` is not a valid hexadecimal string. + /// - If the decoded `self.big_r.affine_point` is shorter than 1 byte. + /// + /// The error message "Incorrect Signature" will be displayed in case of a panic. pub fn to_bytes(&self) -> Vec { let mut bytes = Vec::new(); - bytes.extend_from_slice(&hex::decode(&self.big_r.affine_point).expect("Incorrect Signature")[1..]); + bytes.extend_from_slice( + &hex::decode(&self.big_r.affine_point).expect("Incorrect Signature")[1..], + ); bytes.extend_from_slice(&hex::decode(&self.s.scalar).expect("Incorrect Signature")); bytes.push(self.recovery_id + 27); diff --git a/omni-relayer/src/startup/eth.rs b/omni-relayer/src/startup/eth.rs index 985406d7..e87b5935 100644 --- a/omni-relayer/src/startup/eth.rs +++ b/omni-relayer/src/startup/eth.rs @@ -86,7 +86,7 @@ pub async fn start_indexer( .event_signature([FinTransfer::SIGNATURE_HASH, InitTransfer::SIGNATURE_HASH].to_vec()); for current_block in - (from_block..latest_block).step_by(config.eth.block_processing_batch_size as usize) + (from_block..latest_block).step_by(usize::try_from(config.eth.block_processing_batch_size)?) { let logs = http_provider .get_logs(&filter.clone().from_block(current_block).to_block( @@ -157,6 +157,8 @@ pub async fn start_indexer( Ok(()) } +// TODO: try to split this function +#[allow(clippy::too_many_lines)] async fn process_log( config: &config::Config, redis_connection: &mut redis::aio::MultiplexedConnection, diff --git a/omni-relayer/src/utils/fee.rs b/omni-relayer/src/utils/fee.rs index 791d265f..ae3cf13c 100644 --- a/omni-relayer/src/utils/fee.rs +++ b/omni-relayer/src/utils/fee.rs @@ -60,6 +60,7 @@ pub async fn is_fee_sufficient(jsonrpc_client: &JsonRpcClient, sender: &OmniAddr let token_price = get_price_by_contract_address("near-protocol", token.as_ref()).await?; let token_decimals = get_token_decimals(jsonrpc_client, token).await?; + #[allow(clippy::cast_precision_loss)] let given_fee = fee as f64 / 10u128.pow(token_decimals) as f64 * token_price; // TODO: Right now I chose a random fee (around 0.10 USD), but it should be calculated based on the chain in the future