Add GetDealSector exported API method to market actor (#1443)

Author: anorth

actors/market/src/lib.rs

@@ -62,8 +62,10 @@ fil_actors_runtime::wasm_trampoline!(Actor);
 
 pub const NO_ALLOCATION_ID: u64 = 0;
 
-// An exit code indicating that information about a past deal is no longer available.
+// Indicates that information about a past deal is no longer available.
 pub const EX_DEAL_EXPIRED: ExitCode = ExitCode::new(FIRST_ACTOR_SPECIFIC_EXIT_CODE);
+// Indicates that information about a deal's activation is not yet available.
+pub const EX_DEAL_NOT_ACTIVATED: ExitCode = ExitCode::new(FIRST_ACTOR_SPECIFIC_EXIT_CODE + 1);
 
 /// Market actor methods available
 #[derive(FromPrimitive)]
@@ -93,6 +95,7 @@ pub enum Method {
     GetDealProviderCollateralExported = frc42_dispatch::method_hash!("GetDealProviderCollateral"),
     GetDealVerifiedExported = frc42_dispatch::method_hash!("GetDealVerified"),
     GetDealActivationExported = frc42_dispatch::method_hash!("GetDealActivation"),
+    GetDealSectorExported = frc42_dispatch::method_hash!("GetDealSector"),
     SectorContentChangedExported = ext::miner::SECTOR_CONTENT_CHANGED,
 }
 
@@ -1116,28 +1119,56 @@ impl Actor {
                 terminated: state.slash_epoch,
             }),
             None => {
-                let maybe_proposal = st.find_proposal(rt.store(), params.id)?;
-                match maybe_proposal {
-                    Some(_) => Ok(GetDealActivationReturn {
-                        // The proposal has been published, but not activated.
-                        activated: EPOCH_UNDEFINED,
-                        terminated: EPOCH_UNDEFINED,
-                    }),
-                    None => {
-                        if params.id < st.next_id {
-                            // If the deal ID has been used, it must have been cleaned up.
-                            Err(ActorError::unchecked(
-                                EX_DEAL_EXPIRED,
-                                format!("deal {} expired", params.id),
-                            ))
-                        } else {
-                            // We can't distinguish between failing to activate, or having been
-                            // cleaned up after completion/termination.
-                            Err(ActorError::not_found(format!("no such deal {}", params.id)))
-                        }
-                    }
+                // Pass through exit codes if proposal doesn't exist.
+                let _ = st.get_proposal(rt.store(), params.id)?;
+                // Proposal was published but never activated.
+                Ok(GetDealActivationReturn {
+                    activated: EPOCH_UNDEFINED,
+                    terminated: EPOCH_UNDEFINED,
+                })
+            }
+        }
+    }
+
+    /// Fetches the sector in which a deal is stored.
+    /// This is available from after a deal is activated until it is finally settled
+    /// (either normally or by termination).
+    /// Fails with USR_NOT_FOUND if the deal doesn't exist (yet),
+    /// EX_DEAL_NOT_ACTIVATED if the deal is published but has not been activated,
+    /// or EX_DEAL_EXPIRED if the deal has been removed from state.
+    fn get_deal_sector(
+        rt: &impl Runtime,
+        params: GetDealSectorParams,
+    ) -> Result<GetDealSectorReturn, ActorError> {
+        rt.validate_immediate_caller_accept_any()?;
+        let st = rt.state::<State>()?;
+        let found = st.find_deal_state(rt.store(), params.id)?;
+        match found {
+            Some(state) => {
+                // The deal has been activated and not yet finally settled.
+                if state.slash_epoch != EPOCH_UNDEFINED {
+                    // The deal has been terminated but not cleaned up.
+                    // Hide this internal state from caller and fail as if it had been cleaned up.
+                    // This will become an impossible state when deal termination is
+                    // processed immediately.
+                    // Remove with https://github.com/filecoin-project/builtin-actors/issues/1388.
+                    Err(ActorError::unchecked(
+                        EX_DEAL_EXPIRED,
+                        format!("deal {} expired", params.id),
+                    ))
+                } else {
+                    Ok(GetDealSectorReturn { sector: state.sector_number })
                 }
             }
+            None => {
+                // Pass through exit codes if proposal doesn't exist.
+                let _ = st.get_proposal(rt.store(), params.id)?;
+                // Proposal was published but never activated.
+                Err(ActorError::unchecked(
+                    EX_DEAL_NOT_ACTIVATED,
+                    format!("deal {} not yet activated", params.id),
+                ))
+            }
         }
     }
 }
@@ -1603,6 +1634,7 @@ impl ActorCode for Actor {
         GetDealProviderCollateralExported => get_deal_provider_collateral,
         GetDealVerifiedExported => get_deal_verified,
         GetDealActivationExported => get_deal_activation,
+        GetDealSectorExported => get_deal_sector,
         SectorContentChangedExported => sector_content_changed,
     }
 }

actors/market/src/state.rs

@@ -1086,6 +1086,7 @@ pub fn get_proposal<BS: Blockstore>(
             // If the deal ID has been used, it must have been cleaned up.
             ActorError::unchecked(EX_DEAL_EXPIRED, format!("deal {} expired", id))
         } else {
+            // Never been published.
             ActorError::not_found(format!("no such deal {}", id))
         }
     })?;

actors/market/src/types.rs

@@ -242,6 +242,15 @@ pub struct GetDealActivationReturn {
     pub terminated: ChainEpoch,
 }
 
+pub type GetDealSectorParams = DealQueryParams;
+
+#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)]
+#[serde(transparent)]
+pub struct GetDealSectorReturn {
+    /// Sector number with the provider that has committed the deal.
+    pub sector: SectorNumber,
+}
+
 // Interface market clients can implement to receive notifications from builtin market
 pub const MARKET_NOTIFY_DEAL_METHOD: u64 = frc42_dispatch::method_hash!("MarketNotifyDeal");
 

actors/market/tests/deal_api_test.rs

@@ -9,14 +9,12 @@ use serde::de::DeserializeOwned;
 use fil_actor_market::{
     Actor as MarketActor, DealQueryParams, GetDealActivationReturn, GetDealClientCollateralReturn,
     GetDealClientReturn, GetDealDataCommitmentReturn, GetDealLabelReturn,
-    GetDealProviderCollateralReturn, GetDealProviderReturn, GetDealTermReturn,
-    GetDealTotalPriceReturn, GetDealVerifiedReturn, Method, EX_DEAL_EXPIRED,
+    GetDealProviderCollateralReturn, GetDealProviderReturn, GetDealSectorReturn, GetDealTermReturn,
+    GetDealTotalPriceReturn, GetDealVerifiedReturn, Method, EX_DEAL_EXPIRED, EX_DEAL_NOT_ACTIVATED,
 };
 use fil_actors_runtime::network::EPOCHS_IN_DAY;
 use fil_actors_runtime::runtime::policy_constants::DEAL_UPDATES_INTERVAL;
-use fil_actors_runtime::test_utils::{
-    expect_abort_contains_message, MockRuntime, ACCOUNT_ACTOR_CODE_ID,
-};
+use fil_actors_runtime::test_utils::{expect_abort, MockRuntime, ACCOUNT_ACTOR_CODE_ID};
 use fil_actors_runtime::ActorError;
 use fil_actors_runtime::BURNT_FUNDS_ACTOR_ADDR;
 use harness::*;
@@ -114,6 +112,7 @@ fn activation() {
         query_deal(&rt, Method::GetDealActivationExported, id);
     assert_eq!(EPOCH_UNDEFINED, activation.activated);
     assert_eq!(EPOCH_UNDEFINED, activation.terminated);
+    query_deal_fails(&rt, Method::GetDealSectorExported, id, EX_DEAL_NOT_ACTIVATED);
 
     // activate the deal
     let activate_epoch = start_epoch - 2;
@@ -124,6 +123,10 @@ fn activation() {
         query_deal(&rt, Method::GetDealActivationExported, id);
     assert_eq!(activate_epoch, activation.activated);
     assert_eq!(EPOCH_UNDEFINED, activation.terminated);
+    assert_eq!(
+        GetDealSectorReturn { sector: sector_number },
+        query_deal(&rt, Method::GetDealSectorExported, id)
+    );
 
     // terminate early
     let terminate_epoch = activate_epoch + 100;
@@ -133,6 +136,7 @@ fn activation() {
         query_deal(&rt, Method::GetDealActivationExported, id);
     assert_eq!(activate_epoch, activation.activated);
     assert_eq!(terminate_epoch, activation.terminated);
+    query_deal_fails(&rt, Method::GetDealSectorExported, id, EX_DEAL_EXPIRED);
 
     // Clean up state
     let clean_epoch = terminate_epoch + DEAL_UPDATES_INTERVAL;
@@ -146,24 +150,22 @@ fn activation() {
         ExitCode::OK,
     );
     cron_tick(&rt);
-    expect_abort_contains_message(
-        EX_DEAL_EXPIRED,
-        "expired",
-        query_deal_raw(&rt, Method::GetDealActivationExported, id),
-    );
+    query_deal_fails(&rt, Method::GetDealActivationExported, id, EX_DEAL_EXPIRED);
+    query_deal_fails(&rt, Method::GetDealSectorExported, id, EX_DEAL_EXPIRED);
 
-    // Non-existent deal is NOT FOUND
-    expect_abort_contains_message(
-        ExitCode::USR_NOT_FOUND,
-        "no such deal",
-        query_deal_raw(&rt, Method::GetDealActivationExported, id + 1),
-    );
+    // Non-existent deal is USR_NOT_FOUND
+    query_deal_fails(&rt, Method::GetDealActivationExported, id + 1, ExitCode::USR_NOT_FOUND);
+    query_deal_fails(&rt, Method::GetDealSectorExported, id + 1, ExitCode::USR_NOT_FOUND);
 }
 
 fn query_deal<T: DeserializeOwned>(rt: &MockRuntime, method: Method, id: u64) -> T {
     query_deal_raw(rt, method, id).unwrap().unwrap().deserialize().unwrap()
 }
 
+fn query_deal_fails(rt: &MockRuntime, method: Method, id: u64, expected: ExitCode) {
+    expect_abort(expected, query_deal_raw(rt, method, id));
+}
+
 fn query_deal_raw(
     rt: &MockRuntime,
     method: Method,