refactor of financialrecords (paritytech#54)

refactor financialrecords to support  parallel deposit/withdrawal and  parallel symbol withdrawal

cxrml/financialrecords/src/lib.rs

@@ -222,6 +222,27 @@ impl<T: Trait> StorageDoubleMap for RecordsOf<T> {
     const PREFIX: &'static [u8] = b"FinancialRecords RecordsOf";
 }
 
+/// Last deposit index of a account and related symbol
+pub(crate) struct LastDepositIndexOf<T>(::rstd::marker::PhantomData<T>);
+
+impl<T: Trait> StorageDoubleMap for LastDepositIndexOf<T> {
+    type Key1 = T::AccountId;
+    type Key2 = T::Symbol;
+    type Value = u32;
+    const PREFIX: &'static [u8] = b"FinancialRecords LastDepositIndexOf";
+}
+
+/// Last withdrawal index of a account and related symbol
+pub(crate) struct LastWithdrawalIndexOf<T>(::rstd::marker::PhantomData<T>);
+
+impl<T: Trait> StorageDoubleMap for LastWithdrawalIndexOf<T> {
+    type Key1 = T::AccountId;
+    type Key2 = T::Symbol;
+    type Value = u32;
+    const PREFIX: &'static [u8] = b"FinancialRecords LastWithdrawalIndexOf";
+}
+
+
 impl<T: Trait> Module<T> {
     /// Deposit one of this module's events.
     fn deposit_event(event: Event<T>) {
@@ -262,6 +283,51 @@ impl<T: Trait> Module<T> {
             <RecordsOf<T>>::get(who.clone(), index).map(|r| (index, r))
         }
     }
+
+    pub fn last_deposit_index_of(who: &T::AccountId, sym: &T::Symbol) -> Option<u32> {
+        <LastDepositIndexOf<T>>::get(who.clone(), sym.clone())
+    }
+
+    pub fn last_withdrawal_index_of(who: &T::AccountId, sym: &T::Symbol) -> Option<u32> {
+        <LastWithdrawalIndexOf<T>>::get(who.clone(), sym.clone())
+    }
+
+    pub fn last_deposit_of(who: &T::AccountId, sym: &T::Symbol) -> Option<(u32, RecordT<T>)> {
+        Self::last_deposit_index_of(who, sym).and_then(|index| {
+            <RecordsOf<T>>::get(who.clone(), index).map(|r| (index, r))
+        })
+    }
+
+    pub fn last_withdrawal_of(who: &T::AccountId, sym: &T::Symbol) -> Option<(u32, RecordT<T>)> {
+        Self::last_withdrawal_index_of(who, sym).and_then(|index| {
+            <RecordsOf<T>>::get(who.clone(), index).map(|r| (index, r))
+        })
+    }
+
+    /// deposit/withdrawal pre-process
+    fn before(who: &T::AccountId, sym: &T::Symbol, is_withdrawal: bool) -> Result<(), &'static str> {
+        let r = if is_withdrawal {
+            match Self::last_deposit_of(who, sym) {
+                None => return Err("the account has no deposit record for this token yet"),
+                Some((_, record)) =>
+                    {
+                        if !record.is_finish() {
+                            return Err("the account has no deposit record for this token yet");
+                        }
+                    }
+            }
+
+            Self::last_withdrawal_of(who, sym)
+        } else { Self::last_deposit_of(who, sym) };
+
+        if let Some((_, record)) = r {
+            if !record.is_finish() {
+                return Err("the last action have not finished yet! only if the last deposit/withdrawal have finished you can do a new action.");
+            }
+        }
+        Ok(())
+    }
+
     /// insert a new record for a account, notice the record state must be init state
     fn new_record(who: &T::AccountId, record: &RecordT<T>) -> Result<u32, &'static str> {
         if !record.is_init() {
@@ -270,63 +336,65 @@ impl<T: Trait> Module<T> {
         let len: u32 = Self::records_len_of(who);
         <RecordsOf<T>>::insert(who.clone(), len, *record);
         <RecordsLenOf<T>>::insert(who, len + 1);  // len is more than 1 to max index
+        match record.action() {
+            Action::Deposit(_) => <LastDepositIndexOf<T>>::insert(who.clone(), record.symbol(), len),
+            Action::Withdrawal(_) => <LastWithdrawalIndexOf<T>>::insert(who.clone(), record.symbol(), len),
+        }
         Ok(len)
     }
 }
 
 impl<T: Trait> Module<T> {
-    /// deposit/withdrawal pre-process
-    fn before(who: &T::AccountId, is_withdrawal: bool) -> Result<(), &'static str> {
-        match Self::last_record_of(who) {
-            Some((_, record)) => {
-                if !record.is_finish() {
-                    return Err("the last action have not finished yet! only if the last deposit/withdrawal have finished you can do a new action.");
-                }
-            }
-            None => {
-                if is_withdrawal {
-                    return Err("the account has not deposit record yet")
-                }
-            }
-        }
-        Ok(())
-    }
     /// deposit, notice this func has include deposit_init and deposit_finish (not wait for block confirm process)
     pub fn deposit(who: &T::AccountId, sym: &T::Symbol, balance: T::TokenBalance) -> DispatchResult {
         let index = Self::deposit_with_index(who, sym, balance)?;
         Self::deposit_finish_with_index(who, index, true).map(|_| ())
     }
+
     /// withdrawal, notice this func has include withdrawal_init and withdrawal_locking
     pub fn withdrawal(who: &T::AccountId, sym: &T::Symbol, balance: T::TokenBalance) -> DispatchResult {
         let index = Self::withdrawal_with_index(who, sym, balance)?;
         Self::withdrawal_locking_with_index(who, index).map(|_| ())
     }
+
     /// withdrawal finish, let the locking token destroy
-    pub fn withdrawal_finish(who: &T::AccountId, success: bool) -> DispatchResult {
-        let last_index = Self::records_len_of(who) - 1;
-        Self::withdrawal_finish_with_index(who, last_index, success).map(|_| ())
+    pub fn withdrawal_finish(who: &T::AccountId, sym: &T::Symbol, success: bool) -> DispatchResult {
+        let r = Self::last_withdrawal_index_of(who, sym);
+        if r.is_none() {
+            return Err("have not executed withdrawal() or withdrawal_init() yet for this record");
+        }
+        Self::withdrawal_finish_with_index(who, r.unwrap(), success).map(|_| ())
     }
+
     /// deposit init, use for record a deposit process begin, usually for start block confirm process
     pub fn deposit_init(who: &T::AccountId, sym: &T::Symbol, balance: T::TokenBalance) -> DispatchResult {
         Self::deposit_with_index(who, sym, balance).map(|_| ())
     }
     /// deposit finish, use for change the deposit record to final, success mark the deposit if success
-    pub fn deposit_finish(who: &T::AccountId, success: bool) -> DispatchResult {
-        let last_index = Self::records_len_of(who) - 1;
-        Self::deposit_finish_with_index(who, last_index, success).map(|_| ())
+    pub fn deposit_finish(who: &T::AccountId, sym: &T::Symbol, success: bool) -> DispatchResult {
+        let r = Self::last_deposit_index_of(who, sym);
+        if r.is_none() {
+            return Err("have not executed deposit_init() yet for this record");
+        }
+        Self::deposit_finish_with_index(who, r.unwrap(), success).map(|_| ())
     }
+
     /// withdrawal init, use for record a withdrawal start, should call withdrawal_locking after it
     pub fn withdrawal_init(who: &T::AccountId, sym: &T::Symbol, balance: T::TokenBalance) -> DispatchResult {
         Self::withdrawal_with_index(who, sym, balance).map(|_| ())
     }
     /// change the free token to locking state
-    pub fn withdrawal_locking(who: &T::AccountId) -> DispatchResult {
-        let last_index = Self::records_len_of(who) - 1;
-        Self::withdrawal_locking_with_index(who, last_index).map(|_| ())
+    pub fn withdrawal_locking(who: &T::AccountId, sym: &T::Symbol) -> DispatchResult {
+        let r = Self::last_withdrawal_index_of(who, sym);
+        if r.is_none() {
+            return Err("have not executed withdrawal() or withdrawal_init() yet for this record");
+        }
+        Self::withdrawal_locking_with_index(who, r.unwrap()).map(|_| ())
     }
+
     /// deposit init, notice this func return index to show the index of records for this account
     pub fn deposit_with_index(who: &T::AccountId, sym: &T::Symbol, balance: T::TokenBalance) -> Result<u32, &'static str> {
-        Self::before(who, false)?;
+        Self::before(who, sym, false)?;
 
         <tokenbalances::Module<T>>::is_valid_token(sym)?;
 
@@ -374,7 +442,7 @@ impl<T: Trait> Module<T> {
     }
     /// withdrawal init, notice this func return index to show the index of records for this account
     pub fn withdrawal_with_index(who: &T::AccountId, sym: &T::Symbol, balance: T::TokenBalance) -> Result<u32, &'static str> {
-        Self::before(who, true)?;
+        Self::before(who, sym, true)?;
 
         <tokenbalances::Module<T>>::is_valid_token_for(who, sym)?;
         // check token balance

cxrml/financialrecords/src/tests.rs

@@ -118,16 +118,34 @@ fn test_normal2() {
     with_externalities(&mut new_test_ext(), || {
         let a: u64 = 1; // accountid
         let btc_symbol = slice_to_u8_8(b"x-btc");
+        let eth_symbol = slice_to_u8_8(b"x-eth");
 
         // deposit
         FinancialRecords::deposit_init(&a, &btc_symbol, 100).unwrap();
-        assert_ok!(FinancialRecords::deposit_finish(&a, true));
+        assert_ok!(FinancialRecords::deposit_finish(&a, &btc_symbol, true));
+        assert_ok!(FinancialRecords::deposit(&a, &eth_symbol, 100));
+
+        assert_eq!(TokenBalances::total_token_of(&a, &btc_symbol), 100);
+        assert_eq!(TokenBalances::total_token_of(&a, &eth_symbol), 100);
+
         // withdraw
         FinancialRecords::withdrawal(&a, &btc_symbol, 50).unwrap();
+        FinancialRecords::withdrawal(&a, &eth_symbol, 50).unwrap();
 
-        assert_ok!(FinancialRecords::withdrawal_finish(&a, true));
+        assert_ok!(FinancialRecords::withdrawal_finish(&a, &btc_symbol, true));
+        assert_ok!(FinancialRecords::withdrawal_finish(&a, &eth_symbol, false));
 
         assert_eq!(TokenBalances::total_token_of(&a, &btc_symbol), 50);
+        assert_eq!(TokenBalances::total_token_of(&a, &eth_symbol), 100);
+
+        assert_eq!(FinancialRecords::records_len_of(&a), 4);
+
+        assert_eq!(FinancialRecords::last_deposit_index_of(&a, &btc_symbol).unwrap(), 0);
+        assert_eq!(FinancialRecords::last_withdrawal_index_of(&a, &btc_symbol).unwrap(), 2);
+        assert_eq!(FinancialRecords::last_deposit_index_of(&a, &eth_symbol).unwrap(), 1);
+        assert_eq!(FinancialRecords::last_withdrawal_index_of(&a, &eth_symbol).unwrap(), 3);
+
+        assert_eq!(Balances::free_balance(&a), 960);
     })
 }
 
@@ -138,34 +156,37 @@ fn test_last_not_finish() {
         let btc_symbol = slice_to_u8_8(b"x-btc");
         FinancialRecords::deposit_init(&a, &btc_symbol, 100).unwrap();
         assert_err!(FinancialRecords::withdrawal(&a, &btc_symbol, 50),
-            "the last action have not finished yet! only if the last deposit/withdrawal have finished you can do a new action.");
+            "the account has no deposit record for this token yet");
         // let deposit fail
-        assert_ok!(FinancialRecords::deposit_finish(&a, false));
+        assert_ok!(FinancialRecords::deposit_finish(&a, &btc_symbol, false)); // 1. deposit failed
         assert_eq!(TokenBalances::total_token_of(&a, &btc_symbol), 0);
+
         assert_err!(FinancialRecords::withdrawal(&a, &btc_symbol, 50), "not a existed token in this account token list");
         assert_eq!(FinancialRecords::records_len_of(&a), 1);
 
         FinancialRecords::deposit_init(&a, &btc_symbol, 100).unwrap();
-        assert_ok!(FinancialRecords::deposit_finish(&a, true));
+        assert_ok!(FinancialRecords::deposit_finish(&a, &btc_symbol, true));  // 2. deposit success
+
         assert_ok!(FinancialRecords::withdrawal(&a, &btc_symbol, 50));
-        assert_err!(FinancialRecords::deposit(&a, &btc_symbol, 50),
-            "the last action have not finished yet! only if the last deposit/withdrawal have finished you can do a new action.");
-        assert_eq!(TokenBalances::total_token_of(&a, &btc_symbol), 100);
-        assert_eq!(TokenBalances::free_token_of(&a, &btc_symbol), 50);
 
-        assert_ok!(FinancialRecords::withdrawal_finish(&a, false));
+        assert_ok!(FinancialRecords::deposit(&a, &btc_symbol, 50));  // 3. deposit success
+
+        assert_eq!(TokenBalances::total_token_of(&a, &btc_symbol), 150);
         assert_eq!(TokenBalances::free_token_of(&a, &btc_symbol), 100);
 
+        assert_ok!(FinancialRecords::withdrawal_finish(&a, &btc_symbol, false)); // 4. withdrawal failed
+        assert_eq!(TokenBalances::free_token_of(&a, &btc_symbol), 150);
+
         assert_ok!(FinancialRecords::withdrawal(&a, &btc_symbol, 25));
-        assert_ok!(FinancialRecords::withdrawal_finish(&a, true));  // destroy token here
-        assert_eq!(FinancialRecords::records_len_of(&a), 4);
-        assert_eq!(TokenBalances::free_token_of(&a, &btc_symbol), 75);
+        assert_ok!(FinancialRecords::withdrawal_finish(&a, &btc_symbol, true));  // destroy token here 5. withdrawal success
+        assert_eq!(FinancialRecords::records_len_of(&a), 5);
+        assert_eq!(TokenBalances::free_token_of(&a, &btc_symbol), 125);
 
-        assert_eq!(TokenBalances::total_token(&btc_symbol), 175);
+        assert_eq!(TokenBalances::total_token(&btc_symbol), 225);
 
-        // 1. deposit failed 2. deposit success 3. withdrawal failed 4. withdrawal success
-        // 10 + 10 + 10 + 10 = 40
-        assert_eq!(Balances::free_balance(&a), 960);
+        // 1. deposit failed 2. deposit success 3. deposit success 4. withdrawal failed 5. withdrawal success
+        // 10 + 10 + 10 + 10 = 50
+        assert_eq!(Balances::free_balance(&a), 950);
     })
 }
 
@@ -198,6 +219,58 @@ fn test_withdrawal_first() {
     with_externalities(&mut new_test_ext(), || {
         let a: u64 = 1; // accountid
         let btc_symbol = slice_to_u8_8(b"x-btc");
-        assert_err!(FinancialRecords::withdrawal(&a, &btc_symbol, 50), "the account has not deposit record yet");
+        assert_err!(FinancialRecords::withdrawal(&a, &btc_symbol, 50), "the account has no deposit record for this token yet");
+    })
+}
+
+#[test]
+fn test_multi_sym() {
+    with_externalities(&mut new_test_ext(), || {
+        let a: u64 = 1; // accountid
+        let btc_symbol = slice_to_u8_8(b"x-btc");
+        let eth_symbol = slice_to_u8_8(b"x-eth");
+
+
+        assert_err!(FinancialRecords::withdrawal_finish(&a, &btc_symbol, true), "have not executed withdrawal() or withdrawal_init() yet for this record");
+        assert_err!(FinancialRecords::withdrawal(&a, &btc_symbol, 50), "the account has no deposit record for this token yet");
+
+        assert_ok!(FinancialRecords::deposit(&a, &btc_symbol, 100));  // index = 0
+        assert_ok!(FinancialRecords::deposit(&a, &eth_symbol, 100));  // eth 100 index = 1
+        assert_ok!(FinancialRecords::deposit(&a, &btc_symbol, 100));  // btc 200 index = 2
+
+        assert_eq!(FinancialRecords::last_deposit_index_of(&a, &btc_symbol), Some(2));
+        assert_eq!(FinancialRecords::last_deposit_index_of(&a, &eth_symbol), Some(1));
+        assert_eq!(FinancialRecords::last_withdrawal_index_of(&a, &eth_symbol), None);
+
+        assert_eq!(TokenBalances::total_token_of(&a, &btc_symbol), 200);
+        assert_eq!(TokenBalances::total_token_of(&a, &eth_symbol), 100);
+
+        // withdraw
+        assert_ok!(FinancialRecords::withdrawal(&a, &btc_symbol, 50));  // index = 3
+        assert_err!(FinancialRecords::withdrawal(&a, &btc_symbol, 25), "the last action have not finished yet! only if the last deposit/withdrawal have finished you can do a new action.");
+
+        assert_ok!(FinancialRecords::withdrawal(&a, &eth_symbol, 50));  // parallel withdraw  index = 4
+
+        assert_eq!(TokenBalances::free_token_of(&a, &btc_symbol), 150);
+        assert_eq!(TokenBalances::free_token_of(&a, &eth_symbol), 50);
+
+        assert_ok!(FinancialRecords::deposit(&a, &eth_symbol, 50));  // deposit while withdraw  index = 5
+
+        assert_eq!(TokenBalances::free_token_of(&a, &eth_symbol), 100);
+
+        assert_ok!(FinancialRecords::withdrawal_finish(&a, &btc_symbol, true));
+        assert_ok!(FinancialRecords::withdrawal_finish(&a, &eth_symbol, false));
+
+        assert_eq!(TokenBalances::total_token_of(&a, &btc_symbol), 150);
+        assert_eq!(TokenBalances::total_token_of(&a, &eth_symbol), 150);
+
+        assert_eq!(FinancialRecords::last_deposit_index_of(&a, &btc_symbol), Some(2));
+        assert_eq!(FinancialRecords::last_deposit_index_of(&a, &eth_symbol), Some(5));
+
+        assert_eq!(FinancialRecords::last_withdrawal_index_of(&a, &btc_symbol), Some(3));
+        assert_eq!(FinancialRecords::last_withdrawal_index_of(&a, &eth_symbol), Some(4));
+
+
+        assert_eq!(FinancialRecords::records_len_of(&a), 6);
     })
 }