From 04f3843fa8833a939552b88e46e7c09c31d3df10 Mon Sep 17 00:00:00 2001 From: Alex North <445306+anorth@users.noreply.github.com> Date: Tue, 31 Oct 2023 11:52:06 +1100 Subject: [PATCH] Add GetDealSector exported API method to market actor (#1443) --- actors/market/src/lib.rs | 74 ++++++++++++++++++++-------- actors/market/src/state.rs | 1 + actors/market/src/types.rs | 9 ++++ actors/market/tests/deal_api_test.rs | 34 +++++++------ 4 files changed, 81 insertions(+), 37 deletions(-) diff --git a/actors/market/src/lib.rs b/actors/market/src/lib.rs index 56361d72e..1218a0dde 100644 --- a/actors/market/src/lib.rs +++ b/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 { + rt.validate_immediate_caller_accept_any()?; + let st = rt.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, } } diff --git a/actors/market/src/state.rs b/actors/market/src/state.rs index 88424ed32..46067c8c3 100644 --- a/actors/market/src/state.rs +++ b/actors/market/src/state.rs @@ -1086,6 +1086,7 @@ pub fn get_proposal( // 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)) } })?; diff --git a/actors/market/src/types.rs b/actors/market/src/types.rs index 346c83ab5..49bdc6498 100644 --- a/actors/market/src/types.rs +++ b/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"); diff --git a/actors/market/tests/deal_api_test.rs b/actors/market/tests/deal_api_test.rs index 5388c6116..96b61a2ea 100644 --- a/actors/market/tests/deal_api_test.rs +++ b/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(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,