From fe531cbe580ffa5894a3cb849a291d4b8ad8dc9c Mon Sep 17 00:00:00 2001
From: Geoff Stuart <geoff.vball@gmail.com>
Date: Tue, 13 Sep 2022 22:27:00 -0400
Subject: [PATCH] WithdrawBalance fails when beneficiary is expired or out of
 quota (#602)

* WithdrawBalance fail when beneficiary is expired or out of quota
---
 actors/miner/src/lib.rs                       | 9 +++++++++
 actors/miner/tests/change_beneficiary_test.rs | 8 ++++----
 actors/miner/tests/withdraw_balance.rs        | 7 ++++---
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs
index d1a6e968f..d00878757 100644
--- a/actors/miner/src/lib.rs
+++ b/actors/miner/src/lib.rs
@@ -3300,6 +3300,15 @@ impl Actor {
                 if info.beneficiary != info.owner {
                     // remaining_quota always zero and positive
                     let remaining_quota = info.beneficiary_term.available(rt.curr_epoch());
+                    if remaining_quota.is_zero() {
+                        return Err(actor_error!(
+                            forbidden,
+                            "beneficiary expiration of epoch {} passed or quota of {} depleted with {} used",
+                            info.beneficiary_term.expiration,
+                            info.beneficiary_term.quota,
+                            info.beneficiary_term.used_quota
+                        ));
+                    }
                     amount_withdrawn = std::cmp::min(amount_withdrawn, &remaining_quota);
                     if amount_withdrawn.is_positive() {
                         info.beneficiary_term.used_quota += amount_withdrawn;
diff --git a/actors/miner/tests/change_beneficiary_test.rs b/actors/miner/tests/change_beneficiary_test.rs
index 6207b60ff..db93608da 100644
--- a/actors/miner/tests/change_beneficiary_test.rs
+++ b/actors/miner/tests/change_beneficiary_test.rs
@@ -1,5 +1,5 @@
 use fil_actor_miner::BeneficiaryTerm;
-use fil_actors_runtime::test_utils::{expect_abort, MockRuntime};
+use fil_actors_runtime::test_utils::{expect_abort, expect_abort_contains_message, MockRuntime};
 use fvm_shared::clock::ChainEpoch;
 use fvm_shared::{address::Address, econ::TokenAmount, error::ExitCode};
 use num_traits::Zero;
@@ -274,14 +274,14 @@ fn successfully_change_quota_and_test_withdraw() {
 
     //withdraw 0 zero
     let withdraw_left = TokenAmount::from_atto(20);
-    h.withdraw_funds(
+    let ret = h.withdraw_funds(
         &mut rt,
         h.beneficiary,
         &withdraw_left,
         &TokenAmount::zero(),
         &TokenAmount::zero(),
-    )
-    .unwrap();
+    );
+    expect_abort_contains_message(ExitCode::USR_FORBIDDEN, "beneficiary expiration of epoch", ret);
 
     let increase_quota = TokenAmount::from_atto(120);
     let increase_beneficiary_change =
diff --git a/actors/miner/tests/withdraw_balance.rs b/actors/miner/tests/withdraw_balance.rs
index b0d59e3c2..7d1a71b3c 100644
--- a/actors/miner/tests/withdraw_balance.rs
+++ b/actors/miner/tests/withdraw_balance.rs
@@ -142,7 +142,7 @@ fn successfully_withdraw_limited_to_quota() {
 }
 
 #[test]
-fn allow_withdraw_but_no_send_when_beneficiary_not_efficient() {
+fn withdraw_fail_when_beneficiary_expired() {
     let mut h = ActorHarness::new(PERIOD_OFFSET);
     let mut rt = h.new_runtime();
     rt.set_balance(BIG_BALANCE.clone());
@@ -159,8 +159,9 @@ fn allow_withdraw_but_no_send_when_beneficiary_not_efficient() {
     let info = h.get_info(&rt);
     assert_eq!(PERIOD_OFFSET - 10, info.beneficiary_term.expiration);
     rt.set_epoch(100);
-    h.withdraw_funds(&mut rt, h.beneficiary, quota, &TokenAmount::zero(), &TokenAmount::zero())
-        .unwrap();
+    let ret =
+        h.withdraw_funds(&mut rt, h.beneficiary, quota, &TokenAmount::zero(), &TokenAmount::zero());
+    expect_abort_contains_message(ExitCode::USR_FORBIDDEN, "beneficiary expiration of epoch", ret);
     h.check_state(&rt);
 }