Skip to content

Commit

Permalink
Additional tests for synchronous termination and SettleDealPayments (#…
Browse files Browse the repository at this point in the history
…1423)

* rename some tests and comments to no longer reference slashed_epoch which will never be observable

* port cron tick tests for deal termination

* move deal termination tests

* modify generate and publish deal to return proposal inline

* fix bug when settling payments between between publish and activation

* deal termination edge cases

* pr review
  • Loading branch information
alexytsu authored and anorth committed Jan 25, 2024
1 parent 21baa60 commit c6227a2
Show file tree
Hide file tree
Showing 10 changed files with 419 additions and 207 deletions.
1 change: 1 addition & 0 deletions actors/market/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1216,6 +1216,7 @@ impl Actor {
completed: false,
payment: TokenAmount::zero(),
});
batch_gen.add_success();
continue;
}
LoadDealState::ProposalExpired(penalty) => {
Expand Down
6 changes: 2 additions & 4 deletions actors/market/tests/cron_tick_timedout_deals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ const END_EPOCH: ChainEpoch = START_EPOCH + 200 * EPOCHS_IN_DAY;
#[test]
fn timed_out_deal_is_slashed_and_deleted() {
let rt = setup();
let deal_id = generate_and_publish_deal(
let (deal_id, deal_proposal) = generate_and_publish_deal(
&rt,
CLIENT_ADDR,
&MinerAddresses::default(),
START_EPOCH,
END_EPOCH,
);
let deal_proposal = get_deal_proposal(&rt, deal_id);

let c_escrow = get_balance(&rt, &CLIENT_ADDR).balance;

Expand Down Expand Up @@ -64,14 +63,13 @@ fn timed_out_deal_is_slashed_and_deleted() {
fn publishing_timed_out_deal_again_should_work_after_cron_tick_as_it_should_no_longer_be_pending() {
const START_EPOCH: ChainEpoch = 0;
let rt = setup();
let deal_id = generate_and_publish_deal(
let (deal_id, deal_proposal) = generate_and_publish_deal(
&rt,
CLIENT_ADDR,
&MinerAddresses::default(),
START_EPOCH,
END_EPOCH,
);
let deal_proposal = get_deal_proposal(&rt, deal_id);

// publishing will fail as it will be in pending
let deal_proposal2 = generate_deal_and_add_funds(
Expand Down
234 changes: 234 additions & 0 deletions actors/market/tests/deal_termination.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
use fil_actor_market::{DealSettlementSummary, EX_DEAL_EXPIRED};
use fil_actors_runtime::EPOCHS_IN_DAY;
use fvm_shared::{clock::ChainEpoch, econ::TokenAmount};

mod harness;
use harness::*;
use num_traits::Zero;

#[test]
fn deal_is_terminated() {
struct Case {
name: &'static str,
deal_start: ChainEpoch,
deal_end: ChainEpoch,
activation_epoch: ChainEpoch,
termination_epoch: ChainEpoch,
termination_payment: TokenAmount,
}

let cases = [
Case {
name: "deal is terminated after the startepoch and then settle payments before the endepoch",
deal_start: 10,
deal_end: 10 + 200 * EPOCHS_IN_DAY,
activation_epoch: 5,
termination_epoch: 15,
termination_payment: TokenAmount::from_atto(50), // (15 - 10) * 10 as deal storage fee is 10 per epoch
},
Case {
name: "deal is terminated after the startepoch and then settle payments after the endepoch",
deal_start: 10,
deal_end: 10 + 200 * EPOCHS_IN_DAY,
activation_epoch: 5,
termination_epoch: 15,
termination_payment: TokenAmount::from_atto(50), // (15 - 10) * 10 as deal storage fee is 10 per epoch
},
Case {
name: "deal is terminated at the startepoch and then settle payments before the endepoch",
deal_start: 10,
deal_end: 10 + 200 * EPOCHS_IN_DAY,
activation_epoch: 5,
termination_epoch: 10,
termination_payment: TokenAmount::zero(), // (10 - 10) * 10
},
Case {
name: "deal is terminated at the startepoch and then settle payments after the endepoch",
deal_start: 10,
deal_end: 10 + 200 * EPOCHS_IN_DAY,
activation_epoch: 5,
termination_epoch: 10,
termination_payment: TokenAmount::zero(), // (10 - 10) * 10
},
Case {
name: "deal is terminated at the activationepoch and then settle payments before the startepoch",
deal_start: 10,
deal_end: 10 + 200 * EPOCHS_IN_DAY,
activation_epoch: 5,
termination_epoch: 5,
termination_payment: TokenAmount::zero(), // (10 - 10) * 10
},
Case {
name: "deal is terminated at the activationepoch and then settle payments after the startepoch",
deal_start: 10,
deal_end: 10 + 200 * EPOCHS_IN_DAY,
activation_epoch: 5,
termination_epoch: 5,
termination_payment: TokenAmount::zero(), // (10 - 10) * 10
},
Case {
name: "deal is terminated at the activationepoch and then settle payments after the endepoch",
deal_start: 10,
deal_end: 10 + 200 * EPOCHS_IN_DAY,
activation_epoch: 5,
termination_epoch: 5,
termination_payment: TokenAmount::zero(), // (10 - 10) * 10
},
];

for tc in cases {
eprintln!("running test case: {}", tc.name);
let rt = setup();
let sector_number = 7;
// publish and activate
rt.set_epoch(tc.activation_epoch);
let (deal_id, deal_proposal) = publish_and_activate_deal(
&rt,
CLIENT_ADDR,
&MinerAddresses::default(),
sector_number,
tc.deal_start,
tc.deal_end,
tc.activation_epoch,
tc.deal_end,
);

// terminate
rt.set_epoch(tc.termination_epoch);
let (pay, slashed) =
terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[sector_number]);

assert_eq!(tc.termination_payment, pay);
assert_eq!(deal_proposal.provider_collateral, slashed);

assert_deal_deleted(&rt, deal_id, &deal_proposal, sector_number);

// assert that trying to settle is always a no-op after termination

// immediately after termination
settle_deal_payments_no_change(&rt, PROVIDER_ADDR, CLIENT_ADDR, PROVIDER_ADDR, &[deal_id]);
let mut epoch = tc.termination_epoch + 1;
rt.set_epoch(epoch);

// at deal start (if deal was terminated before start)
if epoch < tc.deal_start {
epoch = tc.deal_start;
rt.set_epoch(epoch);
settle_deal_payments_no_change(
&rt,
PROVIDER_ADDR,
CLIENT_ADDR,
PROVIDER_ADDR,
&[deal_id],
);
}

// during deal (if deal was terminated before end)
if epoch < tc.deal_end {
epoch = tc.deal_end;
rt.set_epoch(epoch);
settle_deal_payments_no_change(
&rt,
PROVIDER_ADDR,
CLIENT_ADDR,
PROVIDER_ADDR,
&[deal_id],
);
}

if epoch < tc.deal_end + 1 {
epoch = tc.deal_end + 1;
rt.set_epoch(epoch);
settle_deal_payments_no_change(
&rt,
PROVIDER_ADDR,
CLIENT_ADDR,
PROVIDER_ADDR,
&[deal_id],
);
}

check_state(&rt);
}
}

#[test]
fn settle_payments_then_terminate_deal_in_the_same_epoch() {
let start_epoch = ChainEpoch::from(50);
let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY;
let termination_epoch = start_epoch + 100;
let sector_number = 7;
let sector_expiry = end_epoch + 100;
let deal_duration = termination_epoch - start_epoch;

let rt = setup();

let (deal_id, proposal) = publish_and_activate_deal(
&rt,
CLIENT_ADDR,
&MinerAddresses::default(),
sector_number,
start_epoch,
end_epoch,
0,
sector_expiry,
);

let client_before = get_balance(&rt, &CLIENT_ADDR);
let provider_before = get_balance(&rt, &PROVIDER_ADDR);

// settle payments then terminate
rt.set_epoch(termination_epoch);
let expected_payment = deal_duration * &proposal.storage_price_per_epoch;
let ret = settle_deal_payments(&rt, PROVIDER_ADDR, &[deal_id]);
assert_eq!(
ret.settlements.get(0).unwrap(),
&DealSettlementSummary { completed: false, payment: expected_payment.clone() }
);
terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[sector_number]);
assert_deal_deleted(&rt, deal_id, &proposal, sector_number);

// end state should be equivalent to only calling termination
let client_after = get_balance(&rt, &CLIENT_ADDR);
let provider_after = get_balance(&rt, &PROVIDER_ADDR);
let expected_slash = proposal.provider_collateral;
assert_eq!(&client_after.balance, &(client_before.balance - &expected_payment));
assert!(&client_after.locked.is_zero());
assert_eq!(
&provider_after.balance,
&(provider_before.balance + &expected_payment - expected_slash)
);
assert!(&provider_after.locked.is_zero());

check_state(&rt);
}

#[test]
fn terminate_a_deal_then_settle_it_in_the_same_epoch() {
let start_epoch = ChainEpoch::from(50);
let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY;
let termination_epoch = start_epoch + 100;
let sector_expiry = end_epoch + 100;
let sector_number = 7;
let rt = setup();

let (deal_id, proposal) = publish_and_activate_deal(
&rt,
CLIENT_ADDR,
&MinerAddresses::default(),
sector_number,
start_epoch,
end_epoch,
0,
sector_expiry,
);

// terminate then attempt to settle payment
rt.set_epoch(termination_epoch);
terminate_deals_and_assert_balances(&rt, CLIENT_ADDR, PROVIDER_ADDR, &[sector_number]);
let ret = settle_deal_payments(&rt, PROVIDER_ADDR, &[deal_id]);
assert_eq!(ret.results.codes(), vec![EX_DEAL_EXPIRED]);
assert_deal_deleted(&rt, deal_id, &proposal, sector_number);

check_state(&rt);
}
54 changes: 51 additions & 3 deletions actors/market/tests/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,30 @@ pub fn settle_deal_payments(
res
}

pub fn settle_deal_payments_no_change(
rt: &MockRuntime,
caller: Address,
client_addr: Address,
provider_addr: Address,
deal_ids: &[DealID],
) {
let st: State = rt.get_state();
let epoch_cid = st.deal_ops_by_epoch;

// fetch current client escrow balances
let client_acct = get_balance(rt, &client_addr);
let provider_acct = get_balance(rt, &provider_addr);

settle_deal_payments(rt, caller, deal_ids);

let st: State = rt.get_state();
let new_client_acct = get_balance(rt, &client_addr);
let new_provider_acct = get_balance(rt, &provider_addr);
assert_eq!(epoch_cid, st.deal_ops_by_epoch);
assert_eq!(client_acct, new_client_acct);
assert_eq!(provider_acct, new_provider_acct);
}

pub fn settle_deal_payments_and_assert_balances(
rt: &MockRuntime,
client_addr: Address,
Expand Down Expand Up @@ -1239,11 +1263,11 @@ pub fn generate_and_publish_deal(
addrs: &MinerAddresses,
start_epoch: ChainEpoch,
end_epoch: ChainEpoch,
) -> DealID {
) -> (DealID, DealProposal) {
let deal = generate_deal_and_add_funds(rt, client, addrs, start_epoch, end_epoch);
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, addrs.worker);
let deal_ids = publish_deals(rt, addrs, &[deal], TokenAmount::zero(), NO_ALLOCATION_ID); // unverified deal
deal_ids[0]
let deal_ids = publish_deals(rt, addrs, &[deal.clone()], TokenAmount::zero(), NO_ALLOCATION_ID); // unverified deal
(deal_ids[0], deal)
}

pub fn generate_and_publish_verified_deal(
Expand Down Expand Up @@ -1505,6 +1529,30 @@ pub fn terminate_deals(rt: &MockRuntime, miner_addr: Address, sectors: &[SectorN
rt.verify();
}

pub fn terminate_deals_expect_abort(
rt: &MockRuntime,
miner_addr: Address,
sectors: &[SectorNumber],
expected_exit_code: ExitCode,
) {
rt.set_caller(*MINER_ACTOR_CODE_ID, miner_addr);
rt.expect_validate_caller_type(vec![Type::Miner]);
let params = OnMinerSectorsTerminateParams {
epoch: *rt.epoch.borrow(),
sectors: BitField::try_from_bits(sectors.iter().copied()).unwrap(),
};

expect_abort(
expected_exit_code,
rt.call::<MarketActor>(
Method::OnMinerSectorsTerminate as u64,
IpldBlock::serialize_cbor(&params).unwrap(),
),
);

rt.verify();
}

pub fn terminate_deals_raw(
rt: &MockRuntime,
miner_addr: Address,
Expand Down
9 changes: 3 additions & 6 deletions actors/market/tests/market_actor_legacy_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,14 +326,11 @@ fn locked_fund_tracking_states() {
assert!(st.total_client_storage_fee.is_zero());

// Publish deal1, deal2, and deal3 with different client and provider
let deal_id1 = generate_and_publish_deal(&rt, c1, &m1, start_epoch, end_epoch);
let d1 = get_deal_proposal(&rt, deal_id1);
let (deal_id1, d1) = generate_and_publish_deal(&rt, c1, &m1, start_epoch, end_epoch);

let deal_id2 = generate_and_publish_deal(&rt, c2, &m2, start_epoch, end_epoch);
let d2 = get_deal_proposal(&rt, deal_id2);
let (deal_id2, d2) = generate_and_publish_deal(&rt, c2, &m2, start_epoch, end_epoch);

let deal_id3 = generate_and_publish_deal(&rt, c3, &m3, start_epoch, end_epoch);
let d3 = get_deal_proposal(&rt, deal_id3);
let (deal_id3, d3) = generate_and_publish_deal(&rt, c3, &m3, start_epoch, end_epoch);

let csf = d1.total_storage_fee() + d2.total_storage_fee() + d3.total_storage_fee();
let plc = &d1.provider_collateral + d2.provider_collateral + &d3.provider_collateral;
Expand Down
Loading

0 comments on commit c6227a2

Please sign in to comment.