Skip to content

Commit 7106992

Browse files
dmokadmoka
committed
release deposit all amount instead of specified one so we save one query on the usage size
1 parent cd44460 commit 7106992

6 files changed

Lines changed: 33 additions & 136 deletions

File tree

‎integration-tests/src/deposit_limiter.rs‎

Lines changed: 10 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ fn release_deposit_should_fail_when_in_lockdown() {
205205

206206
//Act
207207
assert_noop!(
208-
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI, UNITS),
208+
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI),
209209
pallet_circuit_breaker::Error::<hydradx_runtime::Runtime>::AssetInLockdown,
210210
);
211211
});
@@ -225,7 +225,7 @@ fn release_deposit_should_payable_when_fails() {
225225
assert_reserved_balance!(&ALICE.into(), DAI, UNITS);
226226

227227
//Act
228-
let err = CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI, UNITS)
228+
let err = CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI)
229229
.expect_err("Expected the call to fail");
230230
assert_eq!(err.post_info.pays_fee, frame_support::dispatch::Pays::Yes);
231231
});
@@ -249,7 +249,7 @@ fn release_deposit_should_fail_when_in_the_last_block_of_lockdown() {
249249

250250
//Act
251251
assert_noop!(
252-
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI, UNITS),
252+
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI),
253253
pallet_circuit_breaker::Error::<hydradx_runtime::Runtime>::AssetInLockdown
254254
);
255255
});
@@ -273,7 +273,7 @@ fn release_deposit_should_release_asset_when_lockdown_expires() {
273273

274274
//Act
275275
assert_ok!(
276-
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI, UNITS),
276+
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI),
277277
Pays::No.into()
278278
);
279279

@@ -306,7 +306,7 @@ fn release_deposit_should_not_work_when_lockedown_triggered_2nd_time() {
306306

307307
//Act and assert
308308
assert_noop!(
309-
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI, UNITS),
309+
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI),
310310
pallet_circuit_breaker::Error::<hydradx_runtime::Runtime>::AssetInLockdown
311311
);
312312

@@ -339,7 +339,6 @@ fn release_deposit_should_work_when_asset_unclocked() {
339339
RuntimeOrigin::signed(ALICE.into()),
340340
ALICE.into(),
341341
DAI,
342-
UNITS
343342
));
344343

345344
//Assert
@@ -382,7 +381,6 @@ fn release_deposit_should_work_when_accumulated_through_multiple_periods() {
382381
RuntimeOrigin::signed(ALICE.into()),
383382
ALICE.into(),
384383
DAI,
385-
6 * UNITS
386384
));
387385

388386
//Assert
@@ -394,75 +392,6 @@ fn release_deposit_should_work_when_accumulated_through_multiple_periods() {
394392
});
395393
}
396394

397-
#[test]
398-
fn release_deposit_should_fail_when_amount_is_more_than_reserved() {
399-
Hydra::execute_with(|| {
400-
//Arrange
401-
crate::circuit_breaker::init_omnipool();
402-
set_relaychain_block_number(4);
403-
404-
assert_eq!(Currencies::free_balance(DAI, &ALICE.into()), ALICE_INITIAL_DAI_BALANCE);
405-
let deposit_limit = 100_000_000_000_000_000;
406-
update_deposit_limit(DAI, deposit_limit).unwrap();
407-
408-
assert_ok!(Currencies::deposit(DAI, &ALICE.into(), deposit_limit + UNITS));
409-
assert_reserved_balance!(&ALICE.into(), DAI, UNITS);
410-
411-
set_relaychain_block_number(DAYS + 5);
412-
413-
//Act and assert
414-
assert_noop!(
415-
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI, UNITS * 99),
416-
pallet_circuit_breaker::Error::<hydradx_runtime::Runtime>::InvalidAmount
417-
);
418-
});
419-
}
420-
421-
#[test]
422-
fn release_deposit_should_fail_when_amount_is_less_than_reserved() {
423-
Hydra::execute_with(|| {
424-
//Arrange
425-
crate::circuit_breaker::init_omnipool();
426-
set_relaychain_block_number(4);
427-
428-
assert_eq!(Currencies::free_balance(DAI, &ALICE.into()), ALICE_INITIAL_DAI_BALANCE);
429-
let deposit_limit = 100_000_000_000_000_000;
430-
update_deposit_limit(DAI, deposit_limit).unwrap();
431-
432-
assert_ok!(Currencies::deposit(DAI, &ALICE.into(), deposit_limit + UNITS));
433-
assert_reserved_balance!(&ALICE.into(), DAI, UNITS);
434-
435-
set_relaychain_block_number(DAYS + 5);
436-
437-
//Act and assert
438-
assert_noop!(
439-
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI, UNITS / 4),
440-
pallet_circuit_breaker::Error::<hydradx_runtime::Runtime>::InvalidAmount
441-
);
442-
});
443-
}
444-
445-
#[test]
446-
fn release_deposit_should_fail_when_amount_is_zero() {
447-
Hydra::execute_with(|| {
448-
//Arrange
449-
crate::circuit_breaker::init_omnipool();
450-
set_relaychain_block_number(4);
451-
452-
assert_eq!(Currencies::free_balance(DAI, &ALICE.into()), ALICE_INITIAL_DAI_BALANCE);
453-
let deposit_limit = 100_000_000_000_000_000;
454-
update_deposit_limit(DAI, deposit_limit).unwrap();
455-
456-
set_relaychain_block_number(DAYS + 5);
457-
458-
//Act and assert
459-
assert_noop!(
460-
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI, 0),
461-
pallet_circuit_breaker::Error::<hydradx_runtime::Runtime>::InvalidAmount
462-
);
463-
});
464-
}
465-
466395
#[test]
467396
fn release_deposit_should_fail_when_nothing_is_reserved() {
468397
Hydra::execute_with(|| {
@@ -478,7 +407,7 @@ fn release_deposit_should_fail_when_nothing_is_reserved() {
478407

479408
//Act and assert
480409
assert_noop!(
481-
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI, 17 * UNITS),
410+
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI),
482411
pallet_circuit_breaker::Error::<hydradx_runtime::Runtime>::InvalidAmount
483412
);
484413
});
@@ -499,7 +428,7 @@ fn release_deposit_should_fail_when_no_reserved_asset_for_user() {
499428

500429
//Act and assert
501430
assert_noop!(
502-
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI, UNITS),
431+
CircuitBreaker::release_deposit(RuntimeOrigin::signed(ALICE.into()), ALICE.into(), DAI),
503432
pallet_circuit_breaker::Error::<hydradx_runtime::Runtime>::InvalidAmount
504433
);
505434
});
@@ -527,8 +456,7 @@ fn release_deposit_should_work_when_other_user_claims_it() {
527456
assert_ok!(CircuitBreaker::release_deposit(
528457
RuntimeOrigin::signed(BOB.into()),
529458
ALICE.into(),
530-
DAI,
531-
UNITS
459+
DAI
532460
));
533461

534462
//Assert
@@ -562,12 +490,11 @@ fn release_deposit_should_fail_when_called_2nd_time() {
562490
assert_ok!(CircuitBreaker::release_deposit(
563491
RuntimeOrigin::signed(BOB.into()),
564492
ALICE.into(),
565-
DAI,
566-
UNITS
493+
DAI
567494
));
568495

569496
assert_noop!(
570-
CircuitBreaker::release_deposit(RuntimeOrigin::signed(BOB.into()), ALICE.into(), DAI, UNITS),
497+
CircuitBreaker::release_deposit(RuntimeOrigin::signed(BOB.into()), ALICE.into(), DAI),
571498
pallet_circuit_breaker::Error::<hydradx_runtime::Runtime>::InvalidAmount
572499
);
573500
});

‎pallets/circuit-breaker/src/lib.rs‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -335,12 +335,8 @@ pub mod pallet {
335335
/// Asset lockdown was removed
336336
AssetLockdownRemoved { asset_id: T::AssetId },
337337

338-
/// Reserved amount of deposit was saved
339-
DepositSaved {
340-
who: T::AccountId,
341-
asset_id: T::AssetId,
342-
amount: T::Balance,
343-
},
338+
/// All reserved amount of deposit was released
339+
DepositReleased { who: T::AccountId, asset_id: T::AssetId },
344340
}
345341

346342
#[pallet::error]
@@ -511,9 +507,9 @@ pub mod pallet {
511507
Self::do_lift_lockdown(asset_id, T::Balance::default())
512508
}
513509

514-
/// Save deposit of an asset.
510+
/// Release deposit of an asset.
515511
///
516-
/// The amount must be equal to the total reserved amount of the asset in the account.
512+
/// It releases all the pallet reserved balance of the asset for the given account
517513
///
518514
/// Can be called by any origin, but only if the asset is not in active lockdown.
519515
///
@@ -523,20 +519,17 @@ pub mod pallet {
523519
/// - `origin`: The dispatch origin for this call. Can be signed or root.
524520
/// - `who`: The account that is saving the deposit.
525521
/// - `asset_id`: The identifier of the asset.
526-
/// - `amount`: The amount of the asset to save as a deposit
527522
///
528-
/// Emits `DepositSaved` event when successful.
523+
/// Emits `DepositReleased` event when successful.
529524
#[pallet::call_index(5)]
530525
#[pallet::weight(<T as Config>::WeightInfo::release_deposit())]
531526
pub fn release_deposit(
532527
origin: OriginFor<T>,
533528
who: T::AccountId,
534529
asset_id: T::AssetId,
535-
amount: T::Balance,
536530
) -> DispatchResultWithPostInfo {
537531
ensure_signed_or_root(origin)?;
538532

539-
ensure!(amount > T::Balance::zero(), Error::<T>::InvalidAmount);
540533
let current_block = <frame_system::Pallet<T>>::block_number();
541534
let last_state = AssetLockdownState::<T>::get(asset_id);
542535

@@ -547,14 +540,10 @@ pub mod pallet {
547540
}
548541

549542
<T::DepositLimiter as AssetDepositLimiter<T::AccountId, T::AssetId, T::Balance>>::OnDepositRelease::handle(
550-
&(asset_id, who.clone(), amount),
543+
&(asset_id, who.clone()),
551544
)?;
552545

553-
Self::deposit_event(Event::DepositSaved {
554-
who: who,
555-
asset_id,
556-
amount,
557-
});
546+
Self::deposit_event(Event::DepositReleased { who, asset_id });
558547

559548
Ok(Pays::No.into())
560549
}

‎pallets/circuit-breaker/src/tests/mock.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -820,10 +820,11 @@ impl Handler<(AssetId, AccountId, Balance)> for OnLockdownDepositHandler {
820820
}
821821

822822
pub struct OnReleaseDepositHandler;
823-
impl Handler<(AssetId, AccountId, Balance)> for OnReleaseDepositHandler {
824-
fn handle(t: &(AssetId, AccountId, Balance)) -> DispatchResult {
825-
//TODO: we can check this feature in unit test better
826-
Currencies::unreserve_named(&NamedReserveId::get(), t.0, &t.1, t.2);
823+
impl Handler<(AssetId, AccountId)> for OnReleaseDepositHandler {
824+
fn handle(t: &(AssetId, AccountId)) -> DispatchResult {
825+
let reserved_balance = Currencies::reserved_balance_named(&NamedReserveId::get(), t.0, &t.1);
826+
827+
Currencies::unreserve_named(&NamedReserveId::get(), t.0, &t.1, reserved_balance);
827828

828829
Ok(())
829830
}

‎pallets/circuit-breaker/src/tests/release_deposit.rs‎

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,14 @@ fn release_deposit_should_release_amount() {
3333
RawOrigin::Signed(ALICE).into(),
3434
ALICE,
3535
ASSET_ID,
36-
10
3736
));
3837

3938
//Assert
4039
let balance = Tokens::free_balance(10000, &ALICE);
4140
assert_eq!(balance, 110);
42-
expect_events(vec![crate::pallet::Event::DepositSaved {
41+
expect_events(vec![crate::pallet::Event::DepositReleased {
4342
who: ALICE,
4443
asset_id: crate::tests::deposit_limit::ASSET_ID,
45-
amount: 10,
4644
}
4745
.into()]);
4846
});
@@ -73,7 +71,6 @@ fn release_deposit_should_be_callable_by_other_origin() {
7371
RawOrigin::Signed(BOB).into(),
7472
ALICE,
7573
ASSET_ID,
76-
10
7774
));
7875

7976
//Assert
@@ -103,12 +100,7 @@ fn release_deposit_should_be_callable_by_root() {
103100
System::set_block_number(13);
104101

105102
//Act
106-
assert_ok!(CircuitBreaker::release_deposit(
107-
RawOrigin::Root.into(),
108-
ALICE,
109-
ASSET_ID,
110-
10
111-
));
103+
assert_ok!(CircuitBreaker::release_deposit(RawOrigin::Root.into(), ALICE, ASSET_ID,));
112104

113105
//Assert
114106
let balance = Tokens::free_balance(10000, &ALICE);
@@ -136,7 +128,7 @@ fn release_deposit_should_not_work_when_asset_in_lockdown() {
136128

137129
//Act and assert
138130
assert_noop!(
139-
CircuitBreaker::release_deposit(RawOrigin::Root.into(), ALICE, ASSET_ID, 10),
131+
CircuitBreaker::release_deposit(RawOrigin::Root.into(), ALICE, ASSET_ID),
140132
Error::<Test>::AssetInLockdown
141133
);
142134

@@ -167,12 +159,7 @@ fn release_deposit_should_work_when_asset_in_lockdown_but_expired() {
167159
System::set_block_number(13);
168160

169161
//Act and assert
170-
assert_ok!(CircuitBreaker::release_deposit(
171-
RawOrigin::Root.into(),
172-
ALICE,
173-
ASSET_ID,
174-
10
175-
),);
162+
assert_ok!(CircuitBreaker::release_deposit(RawOrigin::Root.into(), ALICE, ASSET_ID,),);
176163

177164
//Assert
178165
let balance = Tokens::free_balance(10000, &ALICE);
@@ -202,12 +189,7 @@ fn release_deposit_should_work_when_asset_in_unlocked_state() {
202189
assert_ok!(Tokens::deposit(ASSET_ID, &ALICE, 20)); //This sets the asset to unlocked state
203190

204191
//Act and assert
205-
assert_ok!(CircuitBreaker::release_deposit(
206-
RawOrigin::Root.into(),
207-
ALICE,
208-
ASSET_ID,
209-
10
210-
),);
192+
assert_ok!(CircuitBreaker::release_deposit(RawOrigin::Root.into(), ALICE, ASSET_ID,),);
211193

212194
//Assert
213195
let balance = Tokens::free_balance(10000, &ALICE);
@@ -237,7 +219,7 @@ fn release_deposit_should_fail_when_amount_is_zero() {
237219

238220
//Act and assert
239221
assert_noop!(
240-
CircuitBreaker::release_deposit(RawOrigin::Root.into(), ALICE, ASSET_ID, 0),
222+
CircuitBreaker::release_deposit(RawOrigin::Root.into(), ALICE, ASSET_ID),
241223
Error::<Test>::InvalidAmount
242224
);
243225
});

‎pallets/circuit-breaker/src/traits.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ pub trait AssetDepositLimiter<AccountId, AssetId, Balance> {
1111
type Issuance: GetByKey<AssetId, Balance>;
1212
type OnLimitReached: Happened<AssetId>;
1313
type OnLockdownDeposit: Handler<(AssetId, AccountId, Balance)>;
14-
type OnDepositRelease: Handler<(AssetId, AccountId, Balance)>;
14+
type OnDepositRelease: Handler<(AssetId, AccountId)>;
1515
}
1616

1717
pub struct NoDepositLimit<T>(PhantomData<T>);

‎runtime/hydradx/src/assets.rs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,19 +149,17 @@ impl Handler<(AssetId, AccountId, Balance)> for OnLockdownDepositHandler {
149149
}
150150

151151
pub struct OnDepositReleaseHandler;
152-
impl Handler<(AssetId, AccountId, Balance)> for OnDepositReleaseHandler {
153-
fn handle(t: &(AssetId, AccountId, Balance)) -> DispatchResult {
152+
impl Handler<(AssetId, AccountId)> for OnDepositReleaseHandler {
153+
fn handle(t: &(AssetId, AccountId)) -> DispatchResult {
154154
let named_reserve_id = DepositCircuitBreakerNamedReserveId::get();
155155

156-
// The exact amount should be reserved because otherwise it can be DDoS attacked with small amounts
157-
// as CircuitBreaker::release_deposit is a free extrinsic.
158156
let reserved_balance = Currencies::reserved_balance_named(&named_reserve_id, t.0, &t.1);
159157
ensure!(
160-
reserved_balance == t.2,
158+
reserved_balance != Balance::zero(),
161159
pallet_circuit_breaker::Error::<Runtime>::InvalidAmount
162160
);
163161

164-
let remaining_reserved = Currencies::unreserve_named(&named_reserve_id, t.0, &t.1, t.2);
162+
let remaining_reserved = Currencies::unreserve_named(&named_reserve_id, t.0, &t.1, reserved_balance);
165163

166164
//We should not have any remaining reserved balance after unreserving, otherwise open to DDos attack
167165
ensure!(

0 commit comments

Comments
 (0)