Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds CommD as option in return value from ActivateDeals #1361

Merged
merged 2 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 16 additions & 9 deletions actors/market/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ impl Actor {
let st: State = rt.state()?;
let proposal_array = st.get_proposal_array(rt.store())?;

let mut sectors_data = Vec::with_capacity(params.sectors.len());
let mut unsealed_cids = Vec::with_capacity(params.sectors.len());
for sector in params.sectors.iter() {
let sector_proposals = get_proposals(&proposal_array, &sector.deal_ids, st.next_id)?;
let sector_size = sector
Expand All @@ -518,10 +518,10 @@ impl Actor {
Some(compute_data_commitment(rt, &sector_proposals, sector.sector_type)?)
};

sectors_data.push(SectorDealData { commd });
unsealed_cids.push(commd);
}

Ok(VerifyDealsForActivationReturn { sectors: sectors_data })
Ok(VerifyDealsForActivationReturn { unsealed_cids })
}

/// Activate a set of deals grouped by sector, returning the size and
Expand Down Expand Up @@ -584,9 +584,9 @@ impl Actor {
// This construction could be replaced with a single "update deal state"
// state method, possibly batched over all deal ids at once.
let update_result: Result<(), ActorError> =
proposals.into_iter().try_for_each(|(deal_id, proposal)| {
proposals.iter().try_for_each(|(deal_id, proposal)| {
let s = st
.find_deal_state(rt.store(), deal_id)
.find_deal_state(rt.store(), *deal_id)
.context(format!("error looking up deal state for {}", deal_id))?;

if s.is_some() {
Expand All @@ -597,7 +597,7 @@ impl Actor {
));
}

let propc = rt_deal_cid(rt, &proposal)?;
let propc = rt_deal_cid(rt, proposal)?;

// Confirm the deal is in the pending proposals queue.
// It will be removed from this queue later, during cron.
Expand All @@ -613,7 +613,7 @@ impl Actor {

// Extract and remove any verified allocation ID for the pending deal.
let allocation = st
.remove_pending_deal_allocation_id(rt.store(), deal_id)
.remove_pending_deal_allocation_id(rt.store(), *deal_id)
.context(format!(
"failed to remove pending deal allocation id {}",
deal_id
Expand All @@ -630,23 +630,30 @@ impl Actor {
}

deal_states.push((
deal_id,
*deal_id,
DealState {
sector_start_epoch: curr_epoch,
last_updated_epoch: EPOCH_UNDEFINED,
slash_epoch: EPOCH_UNDEFINED,
verified_claim: allocation,
},
));
activated_deals.insert(deal_id);
activated_deals.insert(*deal_id);
Ok(())
});

let data_commitment = if params.compute_cid && !p.deal_ids.is_empty() {
Some(compute_data_commitment(rt, &proposals, p.sector_type)?)
} else {
None
};

match update_result {
Ok(_) => {
activations.push(SectorDealActivation {
nonverified_deal_space: deal_spaces.deal_space,
verified_infos,
unsealed_cid: data_commitment,
});
batch_gen.add_success();
}
Expand Down
16 changes: 8 additions & 8 deletions actors/market/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,22 +86,19 @@ pub struct SectorDeals {

#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)]
pub struct VerifyDealsForActivationReturn {
pub sectors: Vec<SectorDealData>,
}

#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq, Default)]
pub struct SectorDealData {
/// Option::None signifies commitment to empty sector, meaning no deals.
pub commd: Option<Cid>,
// The unsealed CID computed from the deals specified for each sector.
// A None indicates no deals were specified.
pub unsealed_cids: Vec<Option<Cid>>,
}

#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)]
#[serde(transparent)]
pub struct BatchActivateDealsParams {
/// Deals to activate, grouped by sector.
/// A failed deal activation will cause other deals in the same sector group to also fail,
/// but allow other sectors to proceed.
pub sectors: Vec<SectorDeals>,
/// Requests computation of an unsealed CID for each sector from the provided deals.
pub compute_cid: bool,
}

// Information about a verified deal that has been activated.
Expand All @@ -121,6 +118,9 @@ pub struct SectorDealActivation {
pub nonverified_deal_space: BigInt,
/// Information about each verified deal activated.
pub verified_infos: Vec<VerifiedDealInfo>,
/// Unsealed CID computed from the deals specified for the sector.
/// A None indicates no deals were specified, or the computation was not requested.
pub unsealed_cid: Option<Cid>,
}

#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)]
Expand Down
5 changes: 4 additions & 1 deletion actors/market/tests/activate_deal_failures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ fn fail_when_caller_is_not_the_provider_of_the_deal() {
sector_type: RegisteredSealProof::StackedDRG8MiBV1,
deal_ids: vec![deal_id],
}],
false,
)
.unwrap();
let res: BatchActivateDealsResult =
Expand All @@ -59,7 +60,7 @@ fn fail_when_caller_is_not_a_storage_miner_actor() {
sector_expiry: 0,
sector_type: RegisteredSealProof::StackedDRG8MiBV1,
};
let params = BatchActivateDealsParams { sectors: vec![sector_activation] };
let params = BatchActivateDealsParams { sectors: vec![sector_activation], compute_cid: false };

expect_abort(
ExitCode::USR_FORBIDDEN,
Expand All @@ -85,6 +86,7 @@ fn fail_when_deal_has_not_been_published_before() {
sector_expiry: EPOCHS_IN_DAY,
deal_ids: vec![DealID::from(42u32)],
}],
false,
)
.unwrap();
let res: BatchActivateDealsResult =
Expand Down Expand Up @@ -120,6 +122,7 @@ fn fail_when_deal_has_already_been_activated() {
sector_expiry,
deal_ids: vec![deal_id],
}],
false,
)
.unwrap();
let res: BatchActivateDealsResult =
Expand Down
8 changes: 4 additions & 4 deletions actors/market/tests/batch_activate_deals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ fn activate_deals_across_multiple_sectors() {
(END_EPOCH + 2, vec![unverified_deal_2_id]), // contains unverified deal only
];

let res = batch_activate_deals(&rt, PROVIDER_ADDR, &sectors);
let res = batch_activate_deals(&rt, PROVIDER_ADDR, &sectors, false);

// three sectors activated successfully
assert!(res.activation_results.all_ok());
Expand Down Expand Up @@ -121,7 +121,7 @@ fn sectors_fail_and_succeed_independently_during_batch_activation() {
SectorDeals { deal_ids: vec![id_4], sector_type, sector_expiry: END_EPOCH + 2 }, // sector succeeds
];

let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals).unwrap();
let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals, false).unwrap();
let res: BatchActivateDealsResult =
res.unwrap().deserialize().expect("VerifyDealsForActivation failed!");

Expand Down Expand Up @@ -172,7 +172,7 @@ fn handles_sectors_empty_of_deals_gracefully() {
SectorDeals { deal_ids: vec![], sector_type, sector_expiry: END_EPOCH + 2 }, // empty sector
];

let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals).unwrap();
let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals, false).unwrap();
let res: BatchActivateDealsResult =
res.unwrap().deserialize().expect("VerifyDealsForActivation failed!");

Expand Down Expand Up @@ -221,7 +221,7 @@ fn fails_to_activate_sectors_containing_duplicate_deals() {
SectorDeals { deal_ids: vec![id_3], sector_type, sector_expiry: END_EPOCH },
];

let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals).unwrap();
let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals, false).unwrap();
let res: BatchActivateDealsResult =
res.unwrap().deserialize().expect("VerifyDealsForActivation failed!");

Expand Down
9 changes: 6 additions & 3 deletions actors/market/tests/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ pub fn activate_deals(
deal_ids: &[DealID],
) -> BatchActivateDealsResult {
rt.set_epoch(current_epoch);

let compute_cid = false;
let ret = batch_activate_deals_raw(
rt,
provider,
Expand All @@ -329,6 +329,7 @@ pub fn activate_deals(
sector_expiry,
sector_type: RegisteredSealProof::StackedDRG8MiBV1,
}],
compute_cid,
)
.unwrap();

Expand All @@ -352,6 +353,7 @@ pub fn batch_activate_deals(
rt: &MockRuntime,
provider: Address,
sectors: &[(ChainEpoch, Vec<DealID>)],
compute_cid: bool,
) -> BatchActivateDealsResult {
let sectors_deals: Vec<SectorDeals> = sectors
.iter()
Expand All @@ -361,7 +363,7 @@ pub fn batch_activate_deals(
sector_type: RegisteredSealProof::StackedDRG8MiBV1,
})
.collect();
let ret = batch_activate_deals_raw(rt, provider, sectors_deals).unwrap();
let ret = batch_activate_deals_raw(rt, provider, sectors_deals, compute_cid).unwrap();

let ret: BatchActivateDealsResult =
ret.unwrap().deserialize().expect("VerifyDealsForActivation failed!");
Expand All @@ -376,11 +378,12 @@ pub fn batch_activate_deals_raw(
rt: &MockRuntime,
provider: Address,
sectors_deals: Vec<SectorDeals>,
compute_cid: bool,
) -> Result<Option<IpldBlock>, ActorError> {
rt.set_caller(*MINER_ACTOR_CODE_ID, provider);
rt.expect_validate_caller_type(vec![Type::Miner]);

let params = BatchActivateDealsParams { sectors: sectors_deals };
let params = BatchActivateDealsParams { sectors: sectors_deals, compute_cid };

let ret = rt.call::<MarketActor>(
Method::BatchActivateDeals as u64,
Expand Down
5 changes: 4 additions & 1 deletion actors/market/tests/market_actor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1699,6 +1699,7 @@ fn fail_when_current_epoch_greater_than_start_epoch_of_deal() {
sector_type: RegisteredSealProof::StackedDRG8MiBV1,
deal_ids: vec![deal_id],
}],
false,
)
.unwrap();

Expand Down Expand Up @@ -1733,6 +1734,7 @@ fn fail_when_end_epoch_of_deal_greater_than_sector_expiry() {
sector_type: RegisteredSealProof::StackedDRG8MiBV1,
deal_ids: vec![deal_id],
}],
false,
)
.unwrap();

Expand Down Expand Up @@ -1760,7 +1762,7 @@ fn fail_to_activate_all_deals_if_one_deal_fails() {
start_epoch,
end_epoch,
);
batch_activate_deals(&rt, PROVIDER_ADDR, &[(sector_expiry, vec![deal_id1])]);
batch_activate_deals(&rt, PROVIDER_ADDR, &[(sector_expiry, vec![deal_id1])], false);

let deal_id2 = generate_and_publish_deal(
&rt,
Expand All @@ -1778,6 +1780,7 @@ fn fail_to_activate_all_deals_if_one_deal_fails() {
sector_type: RegisteredSealProof::StackedDRG8MiBV1,
deal_ids: vec![deal_id1, deal_id2],
}],
false,
)
.unwrap();
let res: BatchActivateDealsResult =
Expand Down
10 changes: 5 additions & 5 deletions actors/market/tests/verify_deals_for_activation_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ fn verify_deal_and_activate_to_get_deal_space_for_unverified_deal_proposal() {
);
let a_response = activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, &[deal_id]);
let s_response = a_response.activations.get(0).unwrap();
assert_eq!(1, v_response.sectors.len());
assert_eq!(Some(make_piece_cid("1".as_bytes())), v_response.sectors[0].commd);
assert_eq!(1, v_response.unsealed_cids.len());
assert_eq!(Some(make_piece_cid("1".as_bytes())), v_response.unsealed_cids[0]);
assert!(s_response.verified_infos.is_empty());
assert_eq!(BigInt::from(deal_proposal.piece_size.0), s_response.nonverified_deal_space);

Expand Down Expand Up @@ -87,8 +87,8 @@ fn verify_deal_and_activate_to_get_deal_space_for_verified_deal_proposal() {
let a_response = activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, &[deal_id]);
let s_response = a_response.activations.get(0).unwrap();

assert_eq!(1, response.sectors.len());
assert_eq!(Some(make_piece_cid("1".as_bytes())), response.sectors[0].commd);
assert_eq!(1, response.unsealed_cids.len());
assert_eq!(Some(make_piece_cid("1".as_bytes())), response.unsealed_cids[0]);
assert_eq!(1, s_response.verified_infos.len());
assert_eq!(deal_proposal.piece_size, s_response.verified_infos[0].size);
assert_eq!(deal_proposal.client.id().unwrap(), s_response.verified_infos[0].client);
Expand Down Expand Up @@ -148,7 +148,7 @@ fn verification_and_weights_for_verified_and_unverified_deals() {
let a_response = activate_deals(&rt, SECTOR_EXPIRY, PROVIDER_ADDR, CURR_EPOCH, &deal_ids);
let s_response = a_response.activations.get(0).unwrap();

assert_eq!(1, response.sectors.len());
assert_eq!(1, response.unsealed_cids.len());
let returned_verified_space: BigInt =
s_response.verified_infos.iter().map(|info| BigInt::from(info.size.0)).sum();
assert_eq!(verified_space, returned_verified_space);
Expand Down
27 changes: 7 additions & 20 deletions actors/miner/src/ext.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use cid::Cid;
use fil_actors_runtime::BatchReturn;
use fvm_ipld_encoding::tuple::*;
use fvm_ipld_encoding::RawBytes;
use fvm_shared::bigint::{bigint_ser, BigInt};
Expand All @@ -12,12 +11,13 @@ use fvm_shared::sector::{RegisteredSealProof, StoragePower};
use fvm_shared::smooth::FilterEstimate;
use fvm_shared::ActorID;

use fil_actors_runtime::BatchReturn;

pub mod account {
pub const PUBKEY_ADDRESS_METHOD: u64 = 2;
}

pub mod market {

use super::*;

pub const VERIFY_DEALS_FOR_ACTIVATION_METHOD: u64 = 5;
Expand All @@ -32,9 +32,9 @@ pub mod market {
}

#[derive(Serialize_tuple, Deserialize_tuple)]
#[serde(transparent)]
pub struct BatchActivateDealsParams {
pub sectors: Vec<SectorDeals>,
pub compute_cid: bool,
}

#[derive(Serialize_tuple, Deserialize_tuple, Clone)]
Expand All @@ -57,24 +57,17 @@ pub mod market {
}

#[derive(Serialize_tuple, Deserialize_tuple, Clone)]
pub struct DealActivation {
pub struct SectorDealActivation {
#[serde(with = "bigint_ser")]
pub nonverified_deal_space: BigInt,
pub verified_infos: Vec<VerifiedDealInfo>,
pub unsealed_cid: Option<Cid>,
}

#[derive(Serialize_tuple, Deserialize_tuple, Clone)]
pub struct BatchActivateDealsResult {
pub activation_results: BatchReturn,
pub activations: Vec<DealActivation>,
}

#[derive(Serialize_tuple, Deserialize_tuple, Clone, Default)]
pub struct DealSpaces {
#[serde(with = "bigint_ser")]
pub deal_space: BigInt,
#[serde(with = "bigint_ser")]
pub verified_deal_space: BigInt,
pub activations: Vec<SectorDealActivation>,
}

#[derive(Serialize_tuple, Deserialize_tuple)]
Expand All @@ -100,15 +93,9 @@ pub mod market {
pub sectors: &'a [SectorDeals],
}

#[derive(Serialize_tuple, Deserialize_tuple, Default, Clone)]
pub struct SectorDealData {
/// Option::None signifies commitment to empty sector, meaning no deals.
pub commd: Option<Cid>,
}

#[derive(Serialize_tuple, Deserialize_tuple, Default, Clone)]
pub struct VerifyDealsForActivationReturn {
pub sectors: Vec<SectorDealData>,
pub unsealed_cids: Vec<Option<Cid>>,
}
}

Expand Down
Loading