Skip to content

Commit

Permalink
Simply deal activation return and intermediate data flow
Browse files Browse the repository at this point in the history
  • Loading branch information
anorth committed Jan 12, 2024
1 parent 6e3b5ca commit 7e0abca
Show file tree
Hide file tree
Showing 19 changed files with 251 additions and 282 deletions.
32 changes: 8 additions & 24 deletions actors/market/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,9 +600,7 @@ impl Actor {
validated_proposals.push(proposal);
}

let mut verified_infos = vec![];
let mut unverified_infos: Vec<UnVerifiedDealInfo> = vec![];
let mut nonverified_deal_space: BigInt = BigInt::zero();
let mut activated = vec![];
// Given that all deals validated, prepare the state updates for them all.
// There's no continue below here to ensure updates are consistent.
// Any error must abort.
Expand All @@ -611,21 +609,12 @@ impl Actor {
// Extract and remove any verified allocation ID for the pending deal.
let alloc_id =
pending_deal_allocation_ids.delete(deal_id)?.unwrap_or(NO_ALLOCATION_ID);

if alloc_id != NO_ALLOCATION_ID {
verified_infos.push(VerifiedDealInfo {
client: proposal.client.id().unwrap(),
allocation_id: alloc_id,
data: proposal.piece_cid,
size: proposal.piece_size,
})
} else {
unverified_infos.push(UnVerifiedDealInfo {
data: proposal.piece_cid,
size: proposal.piece_size,
});
nonverified_deal_space += proposal.piece_size.0;
}
activated.push(ActivatedDeal {
client: proposal.client.id().unwrap(),
allocation_id: alloc_id,
data: proposal.piece_cid,
size: proposal.piece_size,
});

// Prepare initial deal state.
deal_states.push((
Expand All @@ -648,12 +637,7 @@ impl Actor {

sectors_deals
.push((sector.sector_number, SectorDealIDs { deals: sector.deal_ids.clone() }));
activations.push(SectorDealActivation {
nonverified_deal_space,
verified_infos,
unverified_infos,
unsealed_cid: data_commitment,
});
activations.push(SectorDealActivation { activated, unsealed_cid: data_commitment });

for (deal_id, proposal) in sector.deal_ids.iter().zip(&validated_proposals) {
emit::deal_activated(
Expand Down
22 changes: 5 additions & 17 deletions actors/market/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,32 +102,20 @@ pub struct BatchActivateDealsParams {
pub compute_cid: bool,
}

// Information about a un-verified deal that has been activated.
// Information about a deal that has been activated.
#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)]
pub struct UnVerifiedDealInfo {
pub data: Cid,
pub size: PaddedPieceSize,
}

// Information about a verified deal that has been activated.
#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)]
pub struct VerifiedDealInfo {
pub struct ActivatedDeal {
pub client: ActorID,
pub allocation_id: AllocationID,
pub allocation_id: AllocationID, // NO_ALLOCATION_ID for unverified deals.
pub data: Cid,
pub size: PaddedPieceSize,
}

// Information about a sector-grouping of deals that have been activated.
#[derive(Serialize_tuple, Deserialize_tuple, Debug, Clone, Eq, PartialEq)]
pub struct SectorDealActivation {
/// The total size of the non-verified deals activated.
#[serde(with = "bigint_ser")]
pub nonverified_deal_space: BigInt,
/// Information about each verified deal activated.
pub verified_infos: Vec<VerifiedDealInfo>,
/// Information about each un-verified deal activated.
pub unverified_infos: Vec<UnVerifiedDealInfo>,
/// Information about each deal activated.
pub activated: Vec<ActivatedDeal>,
/// 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>,
Expand Down
2 changes: 1 addition & 1 deletion actors/market/tests/activate_deal_failures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ fn assert_activation_failure(
deal_ids: vec![deal_id],
}],
false,
vec![],
&[],
)
.unwrap();
let res: BatchActivateDealsResult =
Expand Down
12 changes: 5 additions & 7 deletions actors/market/tests/batch_activate_deals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,7 @@ fn sectors_fail_and_succeed_independently_during_batch_activation() {
},
];

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

Expand Down Expand Up @@ -228,8 +227,7 @@ fn handles_sectors_empty_of_deals_gracefully() {
SectorDeals { sector_number: 3, deal_ids: vec![], sector_type, sector_expiry: END_EPOCH },
];

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

Expand Down Expand Up @@ -275,7 +273,7 @@ fn fails_to_activate_single_sector_duplicate_deals() {
sector_expiry: END_EPOCH,
},
];
let res = batch_activate_deals_raw(&rt, PROVIDER_ADDR, sectors_deals, false, vec![]).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 @@ -330,8 +328,8 @@ fn fails_to_activate_cross_sector_duplicate_deals() {
},
];

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

Expand Down
12 changes: 6 additions & 6 deletions actors/market/tests/cron_tick_deal_slashing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ fn deal_is_slashed() {

// terminate
rt.set_epoch(tc.termination_epoch);
terminate_deals(&rt, PROVIDER_ADDR, &[sector_number], vec![deal_id]);
terminate_deals(&rt, PROVIDER_ADDR, &[sector_number], &[deal_id]);

// cron tick
let cron_tick_epoch = process_epoch(tc.deal_start, deal_id);
Expand Down Expand Up @@ -136,7 +136,7 @@ fn deal_is_slashed_at_the_end_epoch_should_not_be_slashed_and_should_be_consider
// as deal is considered to be expired.

rt.set_epoch(END_EPOCH);
terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER], vec![]);
terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER], &[]);

// on the next cron tick, it will be processed as expired
let current = END_EPOCH + 300;
Expand Down Expand Up @@ -183,7 +183,7 @@ fn deal_payment_and_slashing_correctly_processed_in_same_crontick() {
// set slash epoch of deal
let slash_epoch = current + Policy::default().deal_updates_interval + 1;
rt.set_epoch(slash_epoch);
terminate_deals(&rt, PROVIDER_ADDR, &[sector_number], vec![deal_id]);
terminate_deals(&rt, PROVIDER_ADDR, &[sector_number], &[deal_id]);

let duration = slash_epoch - current;
let current = current + Policy::default().deal_updates_interval + 2;
Expand Down Expand Up @@ -241,7 +241,7 @@ fn slash_multiple_deals_in_the_same_epoch() {

// set slash epoch of deal at 100 epochs past last process epoch
rt.set_epoch(process_epoch(START_EPOCH, deal_id3) + 100);
terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER], vec![deal_id1, deal_id2, deal_id3]);
terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER], &[deal_id1, deal_id2, deal_id3]);

// process slashing of deals 200 epochs later
rt.set_epoch(process_epoch(START_EPOCH, deal_id3) + 300);
Expand Down Expand Up @@ -310,7 +310,7 @@ fn regular_payments_till_deal_is_slashed_and_then_slashing_is_processed() {

// now terminate the deal 1 epoch later
rt.set_epoch(process_start + Policy::default().deal_updates_interval + 1);
terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER], vec![deal_id]);
terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER], &[deal_id]);

// Setting the epoch to anything less than next schedule will not make any change even though the deal is slashed
rt.set_epoch(process_start + 2 * Policy::default().deal_updates_interval - 1);
Expand Down Expand Up @@ -365,7 +365,7 @@ fn regular_payments_till_deal_expires_and_then_we_attempt_to_slash_it_but_it_wil
// as deal is considered to be expired.
let duration = END_EPOCH - current;
rt.set_epoch(END_EPOCH);
terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER], vec![]);
terminate_deals(&rt, PROVIDER_ADDR, &[SECTOR_NUMBER], &[]);

// next epoch for cron schedule is endEpoch + 300 ->
// setting epoch to higher than that will cause deal to be expired, payment will be made
Expand Down
2 changes: 1 addition & 1 deletion actors/market/tests/deal_api_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ fn activation() {
let terminate_epoch = activate_epoch + 100;
rt.set_epoch(terminate_epoch);

terminate_deals(&rt, PROVIDER_ADDR, &[sector_number], vec![id]);
terminate_deals(&rt, PROVIDER_ADDR, &[sector_number], &[id]);
let activation: GetDealActivationReturn =
query_deal(&rt, Method::GetDealActivationExported, id);
assert_eq!(activate_epoch, activation.activated);
Expand Down
21 changes: 11 additions & 10 deletions actors/market/tests/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ pub fn activate_deals(
current_epoch,
sector_number,
deal_ids,
deal_ids.into(),
deal_ids,
)
}

Expand All @@ -344,7 +344,7 @@ pub fn activate_deals_for(
current_epoch: ChainEpoch,
sector_number: SectorNumber,
deal_ids: &[DealID],
expected_deal_activations: Vec<DealID>,
expected_deal_activations: &[DealID],
) -> BatchActivateDealsResult {
rt.set_epoch(current_epoch);
let compute_cid = false;
Expand Down Expand Up @@ -397,7 +397,8 @@ pub fn batch_activate_deals(
let deal_ids =
sectors.iter().flat_map(|(_, _, deal_ids)| deal_ids).cloned().collect::<Vec<_>>();

let ret = batch_activate_deals_raw(rt, provider, sectors_deals, compute_cid, deal_ids).unwrap();
let ret =
batch_activate_deals_raw(rt, provider, sectors_deals, compute_cid, &deal_ids).unwrap();

let ret: BatchActivateDealsResult =
ret.unwrap().deserialize().expect("VerifyDealsForActivation failed!");
Expand All @@ -413,20 +414,20 @@ pub fn batch_activate_deals_raw(
provider: Address,
sectors_deals: Vec<SectorDeals>,
compute_cid: bool,
expected_activated_deals: Vec<DealID>,
expected_activated_deals: &[DealID],
) -> 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, compute_cid };

for deal_id in expected_activated_deals {
let dp = get_deal_proposal(rt, deal_id);
let dp = get_deal_proposal(rt, *deal_id);

expect_emitted(
rt,
"deal-activated",
deal_id,
*deal_id,
dp.client.id().unwrap(),
dp.provider.id().unwrap(),
);
Expand Down Expand Up @@ -1244,7 +1245,7 @@ pub fn terminate_deals(
rt: &MockRuntime,
miner_addr: Address,
sectors: &[SectorNumber],
expected_terminations: Vec<DealID>,
expected_terminations: &[DealID],
) {
let ret = terminate_deals_raw(rt, miner_addr, sectors, expected_terminations).unwrap();
assert!(ret.is_none());
Expand All @@ -1255,7 +1256,7 @@ pub fn terminate_deals_raw(
rt: &MockRuntime,
miner_addr: Address,
sector_numbers: &[SectorNumber],
terminated_deals: Vec<DealID>,
terminated_deals: &[DealID],
) -> Result<Option<IpldBlock>, ActorError> {
rt.set_caller(*MINER_ACTOR_CODE_ID, miner_addr);
rt.expect_validate_caller_type(vec![Type::Miner]);
Expand All @@ -1264,11 +1265,11 @@ pub fn terminate_deals_raw(
let params = OnMinerSectorsTerminateParams { epoch: *rt.epoch.borrow(), sectors: bf };

for deal_id in terminated_deals {
let d = get_deal_proposal(rt, deal_id);
let d = get_deal_proposal(rt, *deal_id);
expect_emitted(
rt,
"deal-terminated",
deal_id,
*deal_id,
d.client.id().unwrap(),
d.provider.id().unwrap(),
)
Expand Down
12 changes: 6 additions & 6 deletions actors/market/tests/market_actor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ fn worker_balance_after_withdrawal_must_account_for_slashed_funds() {

// slash the deal
rt.set_epoch(publish_epoch + 1);
terminate_deals(&rt, PROVIDER_ADDR, &[sector_number], vec![deal_id]);
terminate_deals(&rt, PROVIDER_ADDR, &[sector_number], &[deal_id]);
let st = get_deal_state(&rt, deal_id);
assert_eq!(publish_epoch + 1, st.slash_epoch);

Expand Down Expand Up @@ -1486,7 +1486,7 @@ fn slash_a_deal_and_make_payment_for_another_deal_in_the_same_epoch() {
// slash deal1
let slash_epoch = process_epoch(start_epoch, deal_id2) + ChainEpoch::from(100);
rt.set_epoch(slash_epoch);
terminate_deals(&rt, PROVIDER_ADDR, &[sector_number], vec![deal_id1]);
terminate_deals(&rt, PROVIDER_ADDR, &[sector_number], &[deal_id1]);

// cron tick will slash deal1 and make payment for deal2
rt.expect_send_simple(
Expand Down Expand Up @@ -1725,7 +1725,7 @@ fn fail_when_current_epoch_greater_than_start_epoch_of_deal() {
deal_ids: vec![deal_id],
}],
false,
vec![],
&[],
)
.unwrap();

Expand Down Expand Up @@ -1763,7 +1763,7 @@ fn fail_when_end_epoch_of_deal_greater_than_sector_expiry() {
deal_ids: vec![deal_id],
}],
false,
vec![],
&[],
)
.unwrap();

Expand Down Expand Up @@ -1817,7 +1817,7 @@ fn fail_to_activate_all_deals_if_one_deal_fails() {
deal_ids: vec![deal_id1, deal_id2],
}],
false,
vec![],
&[],
)
.unwrap();
let res: BatchActivateDealsResult =
Expand Down Expand Up @@ -1940,7 +1940,7 @@ fn locked_fund_tracking_states() {

// slash deal1
rt.set_epoch(curr + 1);
terminate_deals(&rt, m1.provider, &[sector_number], vec![deal_id1]);
terminate_deals(&rt, m1.provider, &[sector_number], &[deal_id1]);

// cron tick to slash deal1 and expire deal2
rt.set_epoch(end_epoch);
Expand Down
Loading

0 comments on commit 7e0abca

Please sign in to comment.