From 5ac736d6d08e7295c6afeecc566aeadd44086a51 Mon Sep 17 00:00:00 2001
From: maltekliemann <mail@maltekliemann.com>
Date: Mon, 28 Mar 2022 19:11:27 +0200
Subject: [PATCH] Check that all pool join/exit's raise on zero

`util::pool` raises if either `pool_amount` or `asset_amount` is zero.
This is an unfortunate result of the abstraction of `pool_join` and
`pool_exit` into `util::pool`, but it does protect the user from paying
for something they get nothing for.

The other four functions only raise an error if what the joining/exiting
user pays is zero. So this protects the LPs, but not the interacting
user.
---
 zrml/swaps/src/lib.rs   |  3 +++
 zrml/swaps/src/tests.rs | 22 +++++++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/zrml/swaps/src/lib.rs b/zrml/swaps/src/lib.rs
index 99c24987c..b485bb742 100644
--- a/zrml/swaps/src/lib.rs
+++ b/zrml/swaps/src/lib.rs
@@ -292,6 +292,7 @@ mod pallet {
             pool_amount: BalanceOf<T>,
             min_asset_amount: BalanceOf<T>,
         ) -> DispatchResult {
+            ensure!(pool_amount != Zero::zero(), Error::<T>::MathApproximation);
             let pool = Self::pool_by_id(pool_id)?;
             let pool_ref = &pool;
             let who = ensure_signed(origin)?;
@@ -313,6 +314,7 @@ mod pallet {
                         pool.swap_fee.ok_or(Error::<T>::PoolMissingFee)?.saturated_into(),
                     )?
                     .saturated_into();
+                    ensure!(asset_amount != Zero::zero(), Error::<T>::MathApproximation);
                     ensure!(asset_amount >= min_asset_amount, Error::<T>::LimitOut);
                     ensure!(
                         asset_amount
@@ -1554,6 +1556,7 @@ mod pallet {
             asset_amount: BalanceOf<T>,
             min_pool_amount: BalanceOf<T>,
         ) -> Result<Weight, DispatchError> {
+            ensure!(asset_amount != Zero::zero(), Error::<T>::MathApproximation);
             let pool = Pallet::<T>::pool_by_id(pool_id)?;
             let pool_ref = &pool;
             let pool_account_id = Pallet::<T>::pool_account_id(pool_id);
diff --git a/zrml/swaps/src/tests.rs b/zrml/swaps/src/tests.rs
index 37788b790..88890c8bf 100644
--- a/zrml/swaps/src/tests.rs
+++ b/zrml/swaps/src/tests.rs
@@ -555,7 +555,7 @@ fn out_amount_must_be_equal_or_less_than_max_out_ratio() {
 }
 
 #[test]
-fn pool_amount_must_not_be_zero() {
+fn pool_join_or_exit_raises_on_zero_value() {
     ExtBuilder::default().build().execute_with(|| {
         create_initial_pool_with_funds_for_alice(ScoringRule::CPMM, true);
 
@@ -568,6 +568,26 @@ fn pool_amount_must_not_be_zero() {
             Swaps::pool_exit(alice_signed(), 0, 0, vec!(_1, _1, _1, _1)),
             crate::Error::<Runtime>::MathApproximation
         );
+
+        assert_noop!(
+            Swaps::pool_join_with_exact_pool_amount(alice_signed(), 0, ASSET_A, 0, 0),
+            crate::Error::<Runtime>::MathApproximation
+        );
+
+        assert_noop!(
+            Swaps::pool_join_with_exact_asset_amount(alice_signed(), 0, ASSET_A, 0, 0),
+            crate::Error::<Runtime>::MathApproximation
+        );
+
+        assert_noop!(
+            Swaps::pool_exit_with_exact_pool_amount(alice_signed(), 0, ASSET_A, 0, 0),
+            crate::Error::<Runtime>::MathApproximation
+        );
+
+        assert_noop!(
+            Swaps::pool_exit_with_exact_asset_amount(alice_signed(), 0, ASSET_A, 0, 0),
+            crate::Error::<Runtime>::MathApproximation
+        );
     });
 }