Market deal metadata methods to return EX_DEAL_EXPIRED if the deal ID is used

actors/market/src/lib.rs

@@ -482,6 +482,7 @@ impl Actor {
             validate_and_return_deal_space(
                 &proposals,
                 &sector.deal_ids,
+                st.next_id,
                 &miner_addr,
                 sector.sector_expiry,
                 curr_epoch,
@@ -516,6 +517,7 @@ impl Actor {
             validate_and_return_deal_space(
                 &proposals,
                 &params.deal_ids,
+                st.next_id,
                 &miner_addr,
                 params.sector_expiry,
                 curr_epoch,
@@ -542,9 +544,7 @@ impl Actor {
                     ));
                 }
 
-                let proposal = st
-                    .find_proposal(rt.store(), deal_id)?
-                    .ok_or_else(|| actor_error!(not_found, "no such deal_id: {}", deal_id))?;
+                let proposal = st.get_proposal(rt.store(), deal_id)?;
 
                 let propc = rt_deal_cid(rt, &proposal)?;
 
@@ -696,9 +696,7 @@ impl Actor {
                 let deal_ids = st.get_deals_for_epoch(rt.store(), i)?;
 
                 for deal_id in deal_ids {
-                    let deal = st.find_proposal(rt.store(), deal_id)?.ok_or_else(|| {
-                        actor_error!(not_found, "proposal doesn't exist ({})", deal_id)
-                    })?;
+                    let deal = st.get_proposal(rt.store(), deal_id)?;
 
                     let dcid = rt_deal_cid(rt, &deal)?;
 
@@ -974,7 +972,7 @@ impl Actor {
                 terminated: state.slash_epoch,
             }),
             None => {
-                // State::get_proposal will fail with USR_NOT_FOUND in either case.
+                // State::find_proposal will fail with USR_NOT_FOUND or EX_DEAL_EXPIRED in either case.
                 let maybe_proposal = st.find_proposal(rt.store(), params.id)?;
                 match maybe_proposal {
                     Some(_) => Ok(GetDealActivationReturn {
@@ -1030,6 +1028,7 @@ fn compute_data_commitment<BS: Blockstore>(
 pub fn validate_and_return_deal_space<BS: Blockstore>(
     proposals: &DealArray<BS>,
     deal_ids: &[DealID],
+    next_id: DealID,
     miner_addr: &Address,
     sector_expiry: ChainEpoch,
     sector_activation: ChainEpoch,
@@ -1051,7 +1050,13 @@ pub fn validate_and_return_deal_space<BS: Blockstore>(
         let proposal = proposals
             .get(*deal_id)
             .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load deal")?
-            .ok_or_else(|| actor_error!(not_found, "no such deal {}", deal_id))?;
+            .ok_or_else(|| {
+                if deal_id < &next_id {
+                    ActorError::unchecked(EX_DEAL_EXPIRED, format!("deal {} expired", deal_id))
+                } else {
+                    actor_error!(not_found, "no such deal {}", deal_id)
+                }
+            })?;
 
         validate_deal_can_activate(proposal, miner_addr, sector_expiry, sector_activation)
             .with_context(|| format!("cannot activate deal {}", deal_id))?;

actors/market/src/state.rs

@@ -23,7 +23,7 @@ use std::collections::BTreeMap;
 
 use super::policy::*;
 use super::types::*;
-use super::{DealProposal, DealState};
+use super::{DealProposal, DealState, EX_DEAL_EXPIRED};
 
 pub enum Reason {
     ClientCollateral,
@@ -213,9 +213,14 @@ impl State {
         store: &BS,
         id: DealID,
     ) -> Result<DealProposal, ActorError> {
-        let found = self
-            .find_proposal(store, id)?
-            .with_context_code(ExitCode::USR_NOT_FOUND, || format!("no such deal {}", id))?;
+        let found = self.find_proposal(store, id)?.ok_or_else(|| {
+            if id < self.next_id {
+                // If the deal ID has been used, it must have been cleaned up.
+                ActorError::unchecked(EX_DEAL_EXPIRED, format!("deal {} expired", id))
+            } else {
+                ActorError::not_found(format!("no such deal {}", id))
+            }
+        })?;
         Ok(found)
     }
 

actors/market/tests/activate_deal_failures.rs

@@ -1,14 +1,16 @@
 // Copyright 2019-2022 ChainSafe Systems
 // SPDX-License-Identifier: Apache-2.0, MIT
 
-use fil_actor_market::{ActivateDealsParams, Actor as MarketActor, Method};
+use fil_actor_market::{ActivateDealsParams, Actor as MarketActor, Method, State, EX_DEAL_EXPIRED};
 use fil_actors_runtime::network::EPOCHS_IN_DAY;
 use fil_actors_runtime::runtime::builtins::Type;
 use fil_actors_runtime::test_utils::*;
+use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR;
 use fvm_ipld_encoding::ipld_block::IpldBlock;
 use fvm_shared::address::Address;
 use fvm_shared::deal::DealID;
 use fvm_shared::error::ExitCode;
+use fvm_shared::METHOD_SEND;
 
 mod harness;
 
@@ -109,3 +111,45 @@ fn fail_when_deal_has_already_been_activated() {
     rt.verify();
     check_state(&rt);
 }
+
+#[test]
+fn fail_when_deal_has_already_been_expired() {
+    let start_epoch = 10;
+    let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY;
+    let sector_expiry = end_epoch + 100;
+
+    let mut rt = setup();
+    let deal_id = generate_and_publish_deal(
+        &mut rt,
+        CLIENT_ADDR,
+        &MinerAddresses::default(),
+        start_epoch,
+        end_epoch,
+    );
+
+    let deal_proposal = get_deal_proposal(&mut rt, deal_id);
+
+    let current = end_epoch + 25;
+    rt.set_epoch(current);
+    rt.expect_send(
+        BURNT_FUNDS_ACTOR_ADDR,
+        METHOD_SEND,
+        None,
+        deal_proposal.provider_collateral.clone(),
+        None,
+        ExitCode::OK,
+    );
+
+    cron_tick(&mut rt);
+
+    assert_deal_deleted(&mut rt, deal_id, deal_proposal);
+
+    let mut st: State = rt.get_state::<State>();
+    st.next_id = deal_id + 1;
+
+    expect_abort_contains_message(
+        EX_DEAL_EXPIRED,
+        "expired",
+        activate_deals_raw(&mut rt, sector_expiry, PROVIDER_ADDR, 0, &[deal_id]),
+    );
+}

actors/market/tests/market_actor_test.rs

@@ -6,8 +6,8 @@ use fil_actor_market::policy::detail::DEAL_MAX_LABEL_SIZE;
 use fil_actor_market::{
     deal_id_key, ext, ActivateDealsParams, Actor as MarketActor, ClientDealProposal, DealArray,
     DealMetaArray, Label, MarketNotifyDealParams, Method, PublishStorageDealsParams,
-    PublishStorageDealsReturn, State, WithdrawBalanceParams, MARKET_NOTIFY_DEAL_METHOD,
-    NO_ALLOCATION_ID, PROPOSALS_AMT_BITWIDTH, STATES_AMT_BITWIDTH,
+    PublishStorageDealsReturn, State, WithdrawBalanceParams, EX_DEAL_EXPIRED,
+    MARKET_NOTIFY_DEAL_METHOD, NO_ALLOCATION_ID, PROPOSALS_AMT_BITWIDTH, STATES_AMT_BITWIDTH,
 };
 use fil_actors_runtime::cbor::{deserialize, serialize};
 use fil_actors_runtime::network::EPOCHS_IN_DAY;
@@ -1362,7 +1362,7 @@ fn fail_when_deal_is_activated_but_proposal_is_not_found() {
     delete_deal_proposal(&mut rt, deal_id);
 
     rt.set_epoch(process_epoch(start_epoch, deal_id));
-    expect_abort(ExitCode::USR_NOT_FOUND, cron_tick_raw(&mut rt));
+    expect_abort(EX_DEAL_EXPIRED, cron_tick_raw(&mut rt));
 
     check_state_with_expected(
         &rt,