Skip to content

Commit 2de7cee

Browse files
thashimoto1998thashimoto1998
authored
Merge pull request #70 from thashimoto1998/master
fix bug and refactoring
2 parents efb802a + af7b3e3 commit 2de7cee

File tree

10 files changed

+463
-967
lines changed

10 files changed

+463
-967
lines changed

‎pallets/celer-pay/src/celer_wallet.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ mod tests {
8585

8686
#[test]
8787
fn test_pass_deposit_native_token() {
88-
ExtBuilder::build().execute_with(|| {
88+
ExtBuilder::build().execute_with(|| {
8989
let alice_pair = account_pair("Alice");
9090
let bob_pair = account_pair("Bob");
9191
let (channel_peers, peers_pair) = get_sorted_peer(alice_pair.clone(), bob_pair.clone());
@@ -109,7 +109,7 @@ mod tests {
109109

110110
#[test]
111111
fn test_fail_deposit_native_token_because_peer_does_not_have_enough_balance() {
112-
ExtBuilder::build().execute_with(|| {
112+
ExtBuilder::build().execute_with(|| {
113113
let alice_pair = account_pair("Alice");
114114
let bob_pair = account_pair("Bob");
115115
let (channel_peers, peers_pair) = get_sorted_peer(alice_pair.clone(), bob_pair.clone());

‎pallets/celer-pay/src/ledger_operation.rs‎

Lines changed: 243 additions & 767 deletions
Large diffs are not rendered by default.

‎pallets/celer-pay/src/lib.rs‎

Lines changed: 29 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ mod weight_for {
108108
signed_simplex_states_len: u64,
109109
signed_simplex_states_len_weight: u64,
110110
) -> Weight {
111-
T::DbWeight::get().reads_writes(signed_simplex_states_len + 2, 2)
111+
T::DbWeight::get().reads_writes(signed_simplex_states_len, 2)
112112
.saturating_add(50_000_000)
113113
.saturating_add(signed_simplex_states_len_weight.saturating_mul(100_000_000))
114114
}
@@ -399,11 +399,11 @@ decl_module! {
399399
/// - Complexity: `O(1)`
400400
/// - DB:
401401
/// - 2 storage reads `ChannelMap`
402-
/// - 2 storage mutation `ChannelMap`
402+
/// - 1 storage mutation `ChannelMap`
403403
/// - 2 storage reads `Wallets`
404404
/// - 2 storage mutation `Wallets`
405405
/// # </weight>
406-
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(4, 4)]
406+
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(4, 3)]
407407
fn confirm_withdraw(
408408
origin,
409409
channel_id: T::Hash
@@ -447,11 +447,11 @@ decl_module! {
447447
/// - Complexity: `O(1)`
448448
/// - DB:
449449
/// - 2 storage reads `ChannelMap`
450-
/// - 2 storage mutation `ChannelMap`
450+
/// - 1 storage mutation `ChannelMap`
451451
/// - 2 storage reads `Wallets`
452452
/// - 2 storage mutation `Wallets`
453453
/// # </weight>
454-
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(4, 4)]
454+
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(4, 3)]
455455
fn cooperative_withdraw(
456456
origin,
457457
cooperative_withdraw_request: CooperativeWithdrawRequestOf<T>
@@ -479,7 +479,7 @@ decl_module! {
479479
/// - `M` pay_hashes-len
480480
/// - DB:
481481
/// - N storage reads `ChannelMap`
482-
/// - 2 * N storage mutation `ChannelMap`
482+
/// - N storage mutation `ChannelMap`
483483
/// - 2 * M storage reads `PayInfoMap`
484484
/// # </weight>
485485
#[weight = (
@@ -513,10 +513,10 @@ decl_module! {
513513
/// - Complexity: `O(N)`
514514
/// - `N` pay_ids-len
515515
/// - DB:
516-
/// - 2 storage reads `ChannelMap`
516+
/// - 1 storage reads `ChannelMap`
517517
/// - 1 storage mutation `ChannelMap`
518518
/// # </weight>
519-
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(2, 1)]
519+
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(1, 1)]
520520
fn clear_pays(
521521
origin,
522522
channel_id: T::Hash,
@@ -540,13 +540,13 @@ decl_module! {
540540
/// - Complexity: `O(1)`
541541
/// - DB:
542542
/// - 1 storage reads `ChannelMap`
543-
/// - 2 storage mutation `ChannelMap`
544-
/// - 1 storage reads `ChannelStatusNums`
545-
/// - 1 storage mutation `ChannelStatusNums`
546-
/// - 1 storage reads `Wallets`
547-
/// - 1 storage mutation `Wallets`
543+
/// - 1 storage mutation `ChannelMap`
544+
/// - 2 storage reads `ChannelStatusNums`
545+
/// - 2 storage mutation `ChannelStatusNums`
546+
/// - 2 storage reads `Wallets`
547+
/// - 2 storage mutation `Wallets`
548548
/// # </weight>
549-
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(3, 4)]
549+
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(5, 5)]
550550
fn confirm_settle(
551551
origin,
552552
channel_id: T::Hash
@@ -567,12 +567,12 @@ decl_module! {
567567
/// - DB:
568568
/// - 2 storage reads `ChannelMap`
569569
/// - 1 storage mutation `ChannelMap`
570-
/// - 1 storage reads `ChannelStatusNums`
571-
/// - 1 storage mutation `ChannelStatusNums`
572-
/// - 1 storage reads `Wallets`
573-
/// - 1 storage mutation `Wallets`
570+
/// - 2 storage reads `ChannelStatusNums`
571+
/// - 2 storage mutation `ChannelStatusNums`
572+
/// - 2 storage reads `Wallets`
573+
/// - 2 storage mutation `Wallets`
574574
/// # </weight>
575-
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(4, 3)]
575+
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(6, 5)]
576576
fn cooperative_settle(
577577
origin,
578578
settle_request: CooperativeSettleRequestOf<T>
@@ -593,8 +593,8 @@ decl_module! {
593593
/// ## Weight
594594
/// - Complexity: `O(1)`
595595
/// - DB:
596-
/// - 2 storage reads `Balances`
597-
/// - 1 storage mutation `Balances`
596+
/// - 2 storage reads `PoolBalances`
597+
/// - 1 storage mutation `PoolBalances`
598598
/// #</weight>
599599
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(2, 1)]
600600
fn deposit_pool(
@@ -615,10 +615,10 @@ decl_module! {
615615
/// ## Weight
616616
/// - Complexity: `O(1)`
617617
/// - DB:
618-
/// - 2 storage reads `Balances`
619-
/// - 1 storage mutation `Balances`
618+
/// - 1 storage reads `PoolBalances`
619+
/// - 1 storage mutation `PoolBalances`
620620
/// # </weight>
621-
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(2, 1)]
621+
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(1, 1)]
622622
fn withdraw_from_pool(
623623
origin,
624624
value: BalanceOf<T>
@@ -660,12 +660,12 @@ decl_module! {
660660
/// ## Weight
661661
/// - Complexity: `O(1)`
662662
/// - DB:
663-
/// - 2 storage reads `Allowed`
663+
/// - 1 storage reads `Allowed`
664664
/// - 1 storage mutation `Allowed`
665-
/// - 3 storage reads `Balances`
666-
/// - 1 storage mutation `Balances`
665+
/// - 2 storage reads `PoolBalances`
666+
/// - 1 storage mutation `PoolBalances`
667667
/// # </weight>
668-
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(5, 2)]
668+
#[weight = 100_000_000 + T::DbWeight::get().reads_writes(3, 2)]
669669
fn transfer_from(
670670
origin,
671671
from: T::AccountId,
@@ -1165,8 +1165,6 @@ decl_event! (
11651165
DepositToPool(AccountId, Balance),
11661166
/// WithdrawFromPool(receiver, amount)
11671167
WithdrawFromPool(AccountId, Balance),
1168-
/// Transfer(from, receiver, amount)
1169-
Transfer(AccountId, AccountId, Balance),
11701168
/// Approval(owner, spender, amount)
11711169
Approval(AccountId, AccountId, Balance),
11721170

@@ -1259,7 +1257,7 @@ decl_error! {
12591257
// confrom_settle fail
12601258
ConfirmSettleFail,
12611259
// Balances is not exist
1262-
BalancesNotExist,
1260+
PoolBalancesNotExist,
12631261
// Wallet is not exist
12641262
WalletNotExist,
12651263
// Allowed is not exist

‎pallets/celer-pay/src/migration.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ pub mod test {
138138

139139
#[test]
140140
fn test_pass_migrate_operable_channel() {
141-
ExtBuilder::build().execute_with(|| {
141+
ExtBuilder::build().execute_with(|| {
142142
let celer_ledger_account = CelerPayModule::get_celer_ledger_id();
143143
let alice_pair = account_pair("Alice");
144144
let bob_pair = account_pair("Bob");
@@ -166,7 +166,8 @@ pub mod test {
166166

167167
#[test]
168168
fn test_pass_migrate_settling_channel() {
169-
ExtBuilder::build().execute_with(|| {
169+
ExtBuilder::build().execute_with(|| {
170+
System::set_block_number(1);
170171
let celer_ledger_account = CelerPayModule::get_celer_ledger_id();
171172
let alice_pair = account_pair("Alice");
172173
let bob_pair = account_pair("Bob");

‎pallets/celer-pay/src/pay_registry.rs‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -247,11 +247,22 @@ impl<T: Trait> PayRegistry<T> {
247247
for i in 0..pay_id_len {
248248
if PayInfoMap::<T>::contains_key(&pay_ids[i]) {
249249
pay_info = PayInfoMap::<T>::get(&pay_ids[i]).unwrap();
250-
ensure!(
251-
frame_system::Module::<T>::block_number() > pay_info.resolve_deadline.unwrap(),
252-
"Payment is not finalized"
253-
);
254-
amounts.push(pay_info.amount.unwrap());
250+
251+
if pay_info.resolve_deadline.unwrap_or(Zero::zero()) == Zero::zero() {
252+
// should pass last pay resolve deadline if never resolved
253+
ensure!(
254+
frame_system::Module::<T>::block_number() > last_pay_resolve_deadline,
255+
"Payment is not finalized"
256+
);
257+
258+
} else {
259+
// should pass resolve deadline if resolved
260+
ensure!(
261+
frame_system::Module::<T>::block_number() > pay_info.resolve_deadline.unwrap(),
262+
"Payment is not finalized"
263+
);
264+
}
265+
amounts.push(pay_info.amount.unwrap_or(Zero::zero()));
255266
} else {
256267
// should pass last pay resolve deadline if never resolved
257268
ensure!(
@@ -275,7 +286,7 @@ impl<T: Trait> PayRegistry<T> {
275286
resolve_deadline: Some(Zero::zero()),
276287
};
277288
PayInfoMap::<T>::insert(&pay_id, &pay_info);
278-
return Ok((pay_info.amount.unwrap(), pay_info.resolve_deadline.unwrap()));
289+
return Ok((Zero::zero(), Zero::zero()));
279290
}
280291
}
281292
}

‎pallets/celer-pay/src/pay_resolver.rs‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ fn resolve_payment<T: Trait>(
214214

215215
// Should never resolve a pay before or not rearching on-chain resolve deadline.
216216
ensure!(
217-
current_deadline == Zero::zero() || block_number <= current_deadline,
217+
current_deadline.is_zero() || block_number <= current_deadline,
218218
"Passed onchain resolve pay deadline"
219219
);
220220

@@ -279,17 +279,14 @@ fn calculate_boolean_and_payment<T: Trait>(
279279
preimages: Vec<T::Hash>,
280280
) -> Result<BalanceOf<T>, DispatchError> {
281281
let mut j: usize = 0;
282-
283-
let pay_conditions_len = pay.conditions.len();
284282
let mut has_false_contract_cond: bool = false;
285-
for i in 0..pay_conditions_len {
283+
for i in 0..pay.conditions.len() {
286284
let cond = pay.conditions[i].clone();
287285
if cond.condition_type == ConditionType::HashLock {
288286
let hash_lock = match cond.hash_lock {
289287
Some(lock) => lock,
290288
None => Err(Error::<T>::HashLockNotExist)?,
291289
};
292-
293290
ensure!(preimages[j] == hash_lock, "Wrong preimage");
294291
j = j + 1;
295292
} else if cond.condition_type == ConditionType::BooleanRuntimeModule {
@@ -327,12 +324,10 @@ fn calculate_boolean_or_payment<T: Trait>(
327324
preimages: Vec<T::Hash>,
328325
) -> Result<BalanceOf<T>, DispatchError> {
329326
let mut j: usize = 0;
330-
let condition_len = pay.conditions.len();
331-
332327
// Whether there are any contract based conditions, i.e. DEPLOYED_CONTRACT or VIRTUAL_CONTRACT
333328
let mut has_contract_cond = false;
334329
let mut has_true_contract_cond = false;
335-
for i in 0..condition_len {
330+
for i in 0..pay.conditions.len() {
336331
let cond = pay.conditions[i].clone();
337332
if cond.condition_type == ConditionType::HashLock {
338333
let hash_lock = match cond.hash_lock {
@@ -378,11 +373,9 @@ fn calculate_numeric_logic_payment<T: Trait>(
378373
func_type: TransferFunctionType,
379374
) -> Result<BalanceOf<T>, DispatchError> {
380375
let mut amount: BalanceOf<T> = <BalanceOf<T>>::zero();
381-
382376
let mut j: usize = 0;
383-
let pay_conditions_len = pay.conditions.len();
384377
let mut has_contract_cond: bool = false;
385-
for i in 0..pay_conditions_len {
378+
for i in 0..pay.conditions.len() {
386379
let cond = pay.conditions[i].clone();
387380
if cond.condition_type == ConditionType::HashLock {
388381
let hash_lock = match cond.hash_lock {

0 commit comments

Comments
 (0)