Skip to content

Commit

Permalink
Use ExitCode::USR_NOT_FOUND for missing state in GetDealActivation (#…
Browse files Browse the repository at this point in the history
…1441)

* hide deals that are asynchronously termianted in get_deal_activation

* add test for EX_DEAL_EXPIRED behaviour
  • Loading branch information
alexytsu authored and anorth committed Jan 25, 2024
1 parent c6227a2 commit d0b8d9c
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 19 deletions.
23 changes: 16 additions & 7 deletions actors/market/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1102,13 +1102,22 @@ impl Actor {
let st = rt.state::<State>()?;
let found = st.find_deal_state(rt.store(), params.id)?;
match found {
Some(state) => Ok(GetDealActivationReturn {
// If we have state, the deal has been activated.
// It may also have completed normally, or been terminated,
// but not yet been cleaned up.
activated: state.sector_start_epoch,
terminated: state.slash_epoch,
}),
Some(state) => {
if state.slash_epoch != EPOCH_UNDEFINED {
// Deal was terminated asynchronously
// TODO: https://github.com/filecoin-project/builtin-actors/issues/1388
Err(ActorError::unchecked(
EX_DEAL_EXPIRED,
format!("deal {} expired", params.id),
))
} else {
// If we have state, the deal has been activated
Ok(GetDealActivationReturn {
activated: state.sector_start_epoch,
terminated: state.slash_epoch,
})
}
}
None => {
// Pass through exit codes if proposal doesn't exist.
let _ = st.get_proposal(rt.store(), params.id)?;
Expand Down
89 changes: 77 additions & 12 deletions actors/market/tests/transient_marked_for_termination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,36 @@ mod harness;

use std::collections::BTreeMap;

use fil_actor_market::{DealSettlementSummary, State, EX_DEAL_EXPIRED};
use fil_actors_runtime::{runtime::Runtime, BURNT_FUNDS_ACTOR_ADDR, EPOCHS_IN_DAY};
use fil_actor_market::{
Actor as MarketActor, DealQueryParams, DealSettlementSummary, Method, State, EX_DEAL_EXPIRED,
};
use fil_actors_runtime::{
runtime::Runtime,
test_utils::{expect_abort_contains_message, MockRuntime},
ActorError, BURNT_FUNDS_ACTOR_ADDR, EPOCHS_IN_DAY,
};
use fvm_ipld_encoding::ipld_block::IpldBlock;
use fvm_shared::{clock::ChainEpoch, error::ExitCode};
use harness::*;

const START_EPOCH: ChainEpoch = 5;
const SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH: ChainEpoch = 200;
const END_EPOCH: ChainEpoch = SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH + 200 * EPOCHS_IN_DAY;

#[test]
fn deal_scheduled_for_termination_cannot_be_settled_manually() {
let start_epoch = 5;
let end_epoch = SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH + 200 * EPOCHS_IN_DAY;
let sector_number = 7;
let rt = setup();
let sector_number = 7;

let (deal_id_1, deal_1_prop) = publish_and_activate_deal_legacy(
&rt,
CLIENT_ADDR,
&MinerAddresses::default(),
sector_number,
start_epoch,
end_epoch,
START_EPOCH,
END_EPOCH,
0,
end_epoch,
END_EPOCH,
);

// mark this deal for termination
Expand All @@ -37,10 +44,10 @@ fn deal_scheduled_for_termination_cannot_be_settled_manually() {
CLIENT_ADDR,
&MinerAddresses::default(),
sector_number,
start_epoch,
end_epoch,
START_EPOCH,
END_EPOCH,
0,
end_epoch,
END_EPOCH,
);

let slashed_epoch = SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH - 1;
Expand Down Expand Up @@ -94,10 +101,68 @@ fn deal_scheduled_for_termination_cannot_be_settled_manually() {
rt.set_epoch(scheduled_epoch + 1);
let ret = settle_deal_payments(&rt, PROVIDER_ADDR, &[deal_id_1, slashed_deal]);
let expected_payment =
deal_1_prop.storage_price_per_epoch * (scheduled_epoch + 1 - start_epoch);
deal_1_prop.storage_price_per_epoch * (scheduled_epoch + 1 - START_EPOCH);
assert_eq!(ret.results.codes(), vec![ExitCode::OK, EX_DEAL_EXPIRED]);
assert_eq!(
ret.settlements[0],
DealSettlementSummary { completed: false, payment: expected_payment }
);
}

#[test]
fn cannot_get_deal_activation_for_marked_for_termination_deal() {
let rt = setup();
let sector_number = 7;
// create a deal and mark it for termination
let (slashed_deal, _) = publish_and_activate_deal_legacy(
&rt,
CLIENT_ADDR,
&MinerAddresses::default(),
sector_number,
START_EPOCH,
END_EPOCH,
0,
END_EPOCH,
);

let slashed_epoch = SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH - 1;
let scheduled_epoch = SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH + 2;

// simulate one of the deals being marked for termination before the code switchover but scheduled for cron after
{
let mut state = rt.get_state::<State>();

// slashing before the code switchover just marks the epoch in DealState
let mut slashed_deal_state =
state.remove_deal_state(rt.store(), slashed_deal).unwrap().unwrap();
slashed_deal_state.slash_epoch = slashed_epoch;
state.put_deal_states(rt.store(), &[(slashed_deal, slashed_deal_state)]).unwrap();

// actual slashing scheduled for cron after the code switchover
let mut deals_by_epoch = BTreeMap::new();
deals_by_epoch.insert(scheduled_epoch, vec![slashed_deal]);
state.put_batch_deals_by_epoch(rt.store(), &deals_by_epoch).unwrap();

rt.replace_state(&state);
}

// code updated before cron is run
rt.set_epoch(SYNCHRONOUS_TERMINATION_SWITCHOVER_EPOCH);

// attempt to get deal activation for the slashed deal - fails because it's marked-for-termination
expect_abort_contains_message(
EX_DEAL_EXPIRED,
&format!("deal {slashed_deal} expired"),
query_deal_raw(&rt, Method::GetDealActivationExported, slashed_deal),
);
}

fn query_deal_raw(
rt: &MockRuntime,
method: Method,
id: u64,
) -> Result<Option<IpldBlock>, ActorError> {
let params = DealQueryParams { id };
rt.expect_validate_caller_any();
rt.call::<MarketActor>(method as u64, IpldBlock::serialize_cbor(&params).unwrap())
}

0 comments on commit d0b8d9c

Please sign in to comment.