Resolve TODOs and Clippy lints

.github/workflows/test.yml

@@ -32,7 +32,7 @@ jobs:
           key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
       - name: Run cargo clippy
         run: |
-          cargo clippy --all-features --workspace --tests -- --warn clippy::all --warn clippy::nursery
+          cargo clippy --all-features --workspace --tests -- -D warnings
 
   tests:
     name: Tests

Cargo.toml

@@ -26,6 +26,9 @@ redundant_closure_for_method_calls = "allow"
 module_name_repetitions = "allow"
 unwrap_used = "warn"
 expect_used = "warn"
+large_digit_groups = "warn"
+missing_panics_doc = "allow"
+wildcard_imports = "warn"
 
 [profile.release]
 codegen-units = 1

common/src/asset.rs

@@ -17,7 +17,6 @@ pub struct FungibleAsset<T: AssetClass> {
 
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[near(serializers = [json, borsh])]
-#[non_exhaustive]
 enum FungibleAssetKind {
     Nep141(AccountId),
 }
@@ -39,29 +38,19 @@ impl<T: AssetClass> FungibleAsset<T> {
     }
 
     pub fn is_nep141(&self, account_id: &AccountId) -> bool {
-        #[allow(irrefutable_let_patterns)]
-        if let FungibleAssetKind::Nep141(ref contract_id) = self.kind {
-            contract_id == account_id
-        } else {
-            false
-        }
+        let FungibleAssetKind::Nep141(ref contract_id) = self.kind;
+        contract_id == account_id
     }
 
     pub fn into_nep141(self) -> Option<AccountId> {
-        #[allow(clippy::match_wildcard_for_single_variants, unreachable_patterns)]
-        match self.kind {
-            FungibleAssetKind::Nep141(contract_id) => Some(contract_id),
-            _ => None,
-        }
+        let FungibleAssetKind::Nep141(contract_id) = self.kind;
+        Some(contract_id)
     }
 
     pub fn current_account_balance(&self) -> Promise {
         let current_account_id = env::current_account_id();
-        match self.kind {
-            FungibleAssetKind::Nep141(ref account_id) => {
-                ext_ft_core::ext(account_id.clone()).ft_balance_of(current_account_id.clone())
-            }
-        }
+        let FungibleAssetKind::Nep141(ref account_id) = self.kind;
+        ext_ft_core::ext(account_id.clone()).ft_balance_of(current_account_id.clone())
     }
 }
 

common/src/borrow.rs

@@ -214,7 +214,7 @@ impl<M: Borrow<Market>> LinkedBorrowPosition<M> {
     }
 
     pub(crate) fn calculate_last_snapshot_interest(&self) -> BorrowAssetAmount {
-        let market = self.market.borrow();
+        let market: &Market = self.market.borrow();
         let last_snapshot = market.get_last_snapshot();
         let interest_rate = market.get_interest_rate_for_snapshot(last_snapshot);
         let duration_ms = Decimal::from(env::block_timestamp_ms() - last_snapshot.timestamp_ms.0);
@@ -223,6 +223,10 @@ impl<M: Borrow<Market>> LinkedBorrowPosition<M> {
         let interest =
             interest_rate_part * Decimal::from(self.position.get_borrow_asset_principal());
 
+        #[allow(
+            clippy::unwrap_used,
+            reason = "Assume interest will never exceed u128::MAX"
+        )]
         interest.to_u128_ceil().unwrap().into()
     }
 
@@ -232,8 +236,10 @@ impl<M: Borrow<Market>> LinkedBorrowPosition<M> {
 
         let mut accumulated = Decimal::ZERO;
 
-        // Assume # of snapshots will never be > u32::MAX.
-        #[allow(clippy::cast_possible_truncation)]
+        #[allow(
+            clippy::cast_possible_truncation,
+            reason = "Assume # of snapshots will never be > u32::MAX"
+        )]
         let mut it = self
             .market
             .borrow()
@@ -280,6 +286,10 @@ impl<M: Borrow<Market>> LinkedBorrowPosition<M> {
         }
 
         AccumulationRecord {
+            #[allow(
+                clippy::unwrap_used,
+                reason = "Assume accumulated interest will never exceed u128::MAX"
+            )]
             amount: accumulated.to_u128_ceil().unwrap().into(),
             next_snapshot_index,
         }
@@ -310,7 +320,7 @@ impl<M: Borrow<Market>> LinkedBorrowPosition<M> {
     pub fn minimum_acceptable_liquidation_amount(
         &self,
         price_pair: &PricePair,
-    ) -> BorrowAssetAmount {
+    ) -> Option<BorrowAssetAmount> {
         self.market
             .borrow()
             .configuration

common/src/chunked_append_only_list.rs

@@ -184,7 +184,7 @@ mod tests {
         for i in 0..10_000u32 {
             list.push(i);
             list.replace_last(i * 2);
-            assert_eq!(list.len() as u32, i + 1);
+            assert_eq!(list.len(), i + 1);
             assert!(!list.is_empty());
         }
 

common/src/interest_rate_strategy.rs

@@ -175,7 +175,8 @@ pub struct Exponential2 {
 }
 
 impl Exponential2 {
-    #[allow(clippy::missing_panics_doc)]
+    /// # Panics
+    /// - If 2^eccentricity overflows `Decimal`.
     pub fn new(base: Decimal, top: Decimal, eccentricity: Decimal) -> Option<Self> {
         if base > top {
             return None;

common/src/market/balance_oracle_configuration.rs

@@ -77,9 +77,15 @@ mod error {
         NegativePrice,
         #[error("Confidence interval too large")]
         ConfidenceIntervalTooLarge,
+        #[error("Exponent too large")]
+        ExponentTooLarge,
     }
 }
 
+// Maximum number of fully-representable whole digits in 384 bits: floor(log_10(2^384)) = 115
+// Maximum number of digits in a 64-bit integer: floor(log_10(2^64)) + 1 = 20
+const MAXIMUM_POSITIVE_EXPONENT: i32 = 115 - 20;
+
 fn from_pyth_price<T: AssetClass>(
     pyth_price: &pyth::Price,
     decimals: i32,
@@ -92,6 +98,13 @@ fn from_pyth_price<T: AssetClass>(
         return Err(error::PriceDataError::ConfidenceIntervalTooLarge);
     }
 
+    if pyth_price.expo > MAXIMUM_POSITIVE_EXPONENT {
+        return Err(error::PriceDataError::ExponentTooLarge);
+    }
+
+    // TODO: If price falls below minimum representation, it will get truncated to zero.
+    // Is this okay?
+
     Ok(Price {
         _asset: PhantomData,
         price: u128::from(price),
@@ -101,11 +114,11 @@ fn from_pyth_price<T: AssetClass>(
 }
 
 impl<T: AssetClass> Price<T> {
-    fn upper_bound(&self) -> Decimal {
+    pub fn upper_bound(&self) -> Decimal {
         (self.price + self.confidence) * self.power_of_10
     }
 
-    fn lower_bound(&self) -> Decimal {
+    pub fn lower_bound(&self) -> Decimal {
         (self.price - self.confidence) * self.power_of_10
     }
 
@@ -127,7 +140,7 @@ pub struct PricePair {
 impl PricePair {
     /// # Errors
     ///
-    /// If the price data are invalid.
+    /// - If the price data are invalid.
     pub fn new(
         collateral_price: &pyth::Price,
         collateral_decimals: i32,
@@ -140,56 +153,3 @@ impl PricePair {
         })
     }
 }
-
-pub trait AssetConversion<F: AssetClass, T: AssetClass> {
-    fn convert_optimistic(&self, amount: FungibleAssetAmount<F>) -> FungibleAssetAmount<T>;
-    fn convert_pessimistic(&self, amount: FungibleAssetAmount<F>) -> FungibleAssetAmount<T>;
-}
-
-impl AssetConversion<CollateralAsset, BorrowAsset> for PricePair {
-    fn convert_optimistic(
-        &self,
-        amount: FungibleAssetAmount<CollateralAsset>,
-    ) -> FungibleAssetAmount<BorrowAsset> {
-        (Decimal::from(amount) * self.collateral_asset_price.upper_bound()
-            / self.borrow_asset_price.lower_bound())
-        .to_u128_ceil()
-        .unwrap()
-        .into()
-    }
-
-    fn convert_pessimistic(
-        &self,
-        amount: FungibleAssetAmount<CollateralAsset>,
-    ) -> FungibleAssetAmount<BorrowAsset> {
-        (Decimal::from(amount) * self.collateral_asset_price.lower_bound()
-            / self.borrow_asset_price.upper_bound())
-        .to_u128_floor()
-        .unwrap()
-        .into()
-    }
-}
-
-impl AssetConversion<BorrowAsset, CollateralAsset> for PricePair {
-    fn convert_optimistic(
-        &self,
-        amount: FungibleAssetAmount<BorrowAsset>,
-    ) -> FungibleAssetAmount<CollateralAsset> {
-        (Decimal::from(amount) * self.borrow_asset_price.upper_bound()
-            / self.collateral_asset_price.lower_bound())
-        .to_u128_ceil()
-        .unwrap()
-        .into()
-    }
-
-    fn convert_pessimistic(
-        &self,
-        amount: FungibleAssetAmount<BorrowAsset>,
-    ) -> FungibleAssetAmount<CollateralAsset> {
-        (Decimal::from(amount) * self.borrow_asset_price.lower_bound()
-            / self.collateral_asset_price.upper_bound())
-        .to_u128_floor()
-        .unwrap()
-        .into()
-    }
-}

common/src/market/configuration.rs

@@ -11,7 +11,7 @@ use crate::{
     time_chunk::TimeChunkConfiguration,
 };
 
-use super::{AssetConversion, BalanceOracleConfiguration, PricePair, YieldWeights};
+use super::{BalanceOracleConfiguration, PricePair, YieldWeights};
 
 #[derive(Clone, Debug)]
 #[near(serializers = [json, borsh])]
@@ -161,21 +161,16 @@ impl MarketConfiguration {
         is_within_mcr(&self.borrow_mcr, borrow_position, oracle_price_proof)
     }
 
-    #[allow(clippy::missing_panics_doc)]
     pub fn minimum_acceptable_liquidation_amount(
         &self,
         amount: CollateralAssetAmount,
         price_pair: &PricePair,
-    ) -> BorrowAssetAmount {
-        BorrowAssetAmount::new(
-            // Safe because the factor is guaranteed to be <=1, so the result
-            // must still fit in u128.
-            #[allow(clippy::unwrap_used)]
-            ((1u32 - self.liquidation_maximum_spread)
-                * u128::from(price_pair.convert_pessimistic(amount)))
-            .to_u128_ceil()
-            .unwrap(),
-        )
+    ) -> Option<BorrowAssetAmount> {
+        ((1u32 - self.liquidation_maximum_spread)
+            * price_pair.collateral_asset_price.value_pessimistic(amount)
+            / price_pair.borrow_asset_price.upper_bound())
+        .to_u128_ceil()
+        .map(BorrowAssetAmount::new)
     }
 }
 

common/src/market/external.rs

@@ -23,12 +23,6 @@ pub trait MarketExternalInterface {
     fn get_snapshots(&self, offset: Option<u32>, count: Option<u32>) -> Vec<&Snapshot>;
     fn get_borrow_asset_metrics(&self) -> BorrowAssetMetrics;
 
-    // TODO: Decide how to work with remote balances:
-    // Option 1:
-    // Balance oracle calls a function directly.
-    // Option 2: Balance oracle creates/maintains separate NEP-141-ish contracts that track remote
-    // balances.
-
     fn list_borrows(&self, offset: Option<u32>, count: Option<u32>) -> Vec<AccountId>;
     fn list_supplys(&self, offset: Option<u32>, count: Option<u32>) -> Vec<AccountId>;
 

common/src/market/impl.rs

@@ -79,7 +79,7 @@ impl Market {
         self_
     }
 
-    #[allow(clippy::unwrap_used, clippy::missing_panics_doc)]
+    #[allow(clippy::unwrap_used, reason = "Snapshots are never empty")]
     pub fn get_last_snapshot(&self) -> &Snapshot {
         self.snapshots.get(self.snapshots.len() - 1).unwrap()
     }
@@ -135,10 +135,11 @@ impl Market {
         }
     }
 
-    #[allow(clippy::missing_panics_doc)]
     pub fn get_borrow_asset_available_to_borrow(&self) -> BorrowAssetAmount {
-        // Safe because factor is guaranteed to be <=1, so value must still fit in u128.
-        #[allow(clippy::unwrap_used)]
+        #[allow(
+            clippy::unwrap_used,
+            reason = "Factor is guaranteed to be <=1, so value must still fit in u128"
+        )]
         let must_retain = ((1u32 - self.configuration.borrow_asset_maximum_usage_ratio)
             * Decimal::from(self.borrow_asset_deposited))
         .to_u128_ceil()
@@ -288,42 +289,41 @@ impl Market {
 
         // First, static yield.
 
-        let total_weight = u128::from(u16::from(self.configuration.yield_weights.total_weight()));
-        let total_amount = u128::from(amount);
-        if total_weight != 0 {
-            for (account_id, share) in &self.configuration.yield_weights.r#static {
-                #[allow(clippy::unwrap_used)]
-                let portion = amount
-                    .split(
-                        // Safety:
-                        // total_weight is guaranteed >0 and <=u16::MAX
-                        // share is guaranteed <=u16::MAX
-                        // Therefore, as long as total_amount <= u128::MAX / u16::MAX, this will never overflow.
-                        // u128::MAX / u16::MAX == 5192376087906286159508272029171713 (0x10001000100010001000100010001)
-                        // With 24 decimals, that's about 5,192,376,087 tokens.
-                        // TODO: Fix.
-                        total_amount
-                            .checked_mul(u128::from(*share))
-                            .unwrap() // TODO: This one might panic.
-                        / total_weight, // This will never panic: is never div0
-                    )
+        let total_weight =
+            Decimal::from(u16::from(self.configuration.yield_weights.total_weight()));
+        let total_amount = Decimal::from(u128::from(amount));
+        let amount_per_weight = total_amount / total_weight;
+        if !total_weight.is_zero() {
+            for (account_id, share_weight) in &self.configuration.yield_weights.r#static {
+                #[allow(clippy::unwrap_used, reason = "share_weight / total_weight <= 1")]
+                let share = amount
+                    .split((*share_weight * amount_per_weight).to_u128_floor().unwrap())
                     // Safety:
-                    // Guaranteed share <= total_weight
-                    // Guaranteed sum(shares) == total_weight
-                    // Guaranteed sum(floor(total_amount * share / total_weight) for each share in shares) <= total_amount
+                    // Guaranteed share_weight <= total_weight
+                    // Guaranteed sum(share_weights) == total_weight
+                    // Guaranteed sum(floor(total_amount * share_weight / total_weight) for each share_weight in share_weights) <= total_amount
                     // Therefore this should never panic.
                     .unwrap();
 
                 let mut yield_record = self.static_yield.get(account_id).unwrap_or_default();
                 // Assuming borrow_asset is implemented correctly:
                 // this only panics if the circulating supply is somehow >u128::MAX
                 // and we have somehow obtained >u128::MAX amount.
-                // TODO: Include warning somewhere about tokens with >u128::MAX supply.
+                //
+                // NOTE: This is not necessary when working with NEP-141
+                // tokens, which are required by standard to use 128-bit balances.
                 //
                 // Otherwise, borrow_asset is implemented incorrectly.
                 // TODO: If that is the case, how to deal?
-                #[allow(clippy::unwrap_used)]
-                yield_record.borrow_asset.join(portion).unwrap();
+                //
+                // Probably, it is okay to ignore this case. We can assume
+                // that the configuration will only specify
+                // correctly-implemented token contracts.
+                #[allow(
+                    clippy::unwrap_used,
+                    reason = "Assume borrow asset is implemented correctly"
+                )]
+                yield_record.borrow_asset.join(share).unwrap();
                 self.static_yield.insert(account_id, &yield_record);
             }
         }

common/src/market/mod.rs

@@ -32,7 +32,7 @@ pub struct YieldWeights {
 impl YieldWeights {
     /// # Panics
     /// - If `supply` is zero.
-    #[allow(clippy::unwrap_used)]
+    #[allow(clippy::unwrap_used, reason = "Only used during initial construction")]
     pub fn new_with_supply_weight(supply: u16) -> Self {
         Self {
             supply: supply.try_into().unwrap(),