diff --git a/vm/actor/src/builtin/init/mod.rs b/vm/actor/src/builtin/init/mod.rs index cc3879de698a..31f25957a3d6 100644 --- a/vm/actor/src/builtin/init/mod.rs +++ b/vm/actor/src/builtin/init/mod.rs @@ -77,7 +77,7 @@ impl Actor { let id_address: Address = rt.transaction(|s: &mut State, rt| { s.map_address_to_new_id(rt.store(), &robust_address) .map_err(|e| actor_error!(ErrIllegalState; "failed to allocate ID address: {}", e)) - })??; + })?; // Create an empty actor rt.create_actor(params.code_cid, &id_address)?; diff --git a/vm/actor/src/builtin/market/mod.rs b/vm/actor/src/builtin/market/mod.rs index 4e725b487129..2ee285f18a1f 100644 --- a/vm/actor/src/builtin/market/mod.rs +++ b/vm/actor/src/builtin/market/mod.rs @@ -100,7 +100,7 @@ impl Actor { let (nominal, _, _) = escrow_address(rt, &provider_or_client)?; - rt.transaction::<State, Result<_, ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { let mut msm = st.mutator(rt.store()); msm.with_escrow_table(Permission::Write) .with_locked_table(Permission::Write) @@ -120,7 +120,7 @@ impl Actor { .map_err(|e| actor_error!(ErrIllegalState; "failed to flush state: {}", e))?; Ok(()) - })??; + })?; Ok(()) } @@ -146,36 +146,35 @@ impl Actor { // for clients -> only the client i.e the recipient can withdraw rt.validate_immediate_caller_is(&approved)?; - let amount_extracted = - rt.transaction::<State, Result<TokenAmount, ActorError>, _>(|st, rt| { - let mut msm = st.mutator(rt.store()); - msm.with_escrow_table(Permission::Write) - .with_locked_table(Permission::Write) - .build() - .map_err(|e| actor_error!(ErrIllegalState; "failed to load state: {}", e))?; - - // The withdrawable amount might be slightly less than nominal - // depending on whether or not all relevant entries have been processed - // by cron - let min_balance = msm.locked_table.as_ref().unwrap().get(&nominal).map_err( - |e| actor_error!(ErrIllegalState; "failed to get locked balance: {}", e), - )?; + let amount_extracted = rt.transaction(|st: &mut State, rt| { + let mut msm = st.mutator(rt.store()); + msm.with_escrow_table(Permission::Write) + .with_locked_table(Permission::Write) + .build() + .map_err(|e| actor_error!(ErrIllegalState; "failed to load state: {}", e))?; - let ex = msm - .escrow_table - .as_mut() - .unwrap() - .subtract_with_minimum(&nominal, ¶ms.amount, &min_balance) - .map_err(|e| { - actor_error!(ErrIllegalState; + // The withdrawable amount might be slightly less than nominal + // depending on whether or not all relevant entries have been processed + // by cron + let min_balance = msm.locked_table.as_ref().unwrap().get(&nominal).map_err( + |e| actor_error!(ErrIllegalState; "failed to get locked balance: {}", e), + )?; + + let ex = msm + .escrow_table + .as_mut() + .unwrap() + .subtract_with_minimum(&nominal, ¶ms.amount, &min_balance) + .map_err(|e| { + actor_error!(ErrIllegalState; "failed to subtract from escrow table: {}", e) - })?; + })?; - msm.commit_state() - .map_err(|e| actor_error!(ErrIllegalState; "failed to flush state: {}", e))?; + msm.commit_state() + .map_err(|e| actor_error!(ErrIllegalState; "failed to flush state: {}", e))?; - Ok(ex) - })??; + Ok(ex) + })?; rt.send( recipient, @@ -313,7 +312,7 @@ impl Actor { msm.commit_state() .map_err(|e| actor_error!(ErrIllegalState; "failed to flush state: {}", e))?; Ok(()) - })??; + })?; for deal in ¶ms.deals { // Check VerifiedClient allowed cap and deduct PieceSize from cap. @@ -483,7 +482,7 @@ impl Actor { msm.commit_state() .map_err(|e| actor_error!(ErrIllegalState; "failed to flush state: {}", e))?; Ok(()) - })??; + })?; Ok(()) } @@ -502,7 +501,7 @@ impl Actor { rt.validate_immediate_caller_type(std::iter::once(&*MINER_ACTOR_CODE_ID))?; let miner_addr = *rt.message().caller(); - rt.transaction::<State, Result<(), ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { let mut msm = st.mutator(rt.store()); msm.with_deal_states(Permission::Write) .with_deal_proposals(Permission::ReadOnly) @@ -555,7 +554,7 @@ impl Actor { msm.commit_state() .map_err(|e| actor_error!(ErrIllegalState; "failed to flush state: {}", e))?; Ok(()) - })??; + })?; Ok(()) } @@ -611,7 +610,7 @@ impl Actor { let curr_epoch = rt.curr_epoch(); let mut timed_out_verified_deals: Vec<DealProposal> = Vec::new(); - rt.transaction::<State, Result<(), ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { let last_cron = st.last_cron; let mut updates_needed: AHashMap<ChainEpoch, Vec<DealID>> = AHashMap::new(); let mut msm = st.mutator(rt.store()); @@ -822,7 +821,7 @@ impl Actor { msm.commit_state() .map_err(|e| actor_error!(ErrIllegalState; "failed to flush state: {}", e))?; Ok(()) - })??; + })?; for d in timed_out_verified_deals { let res = rt.send( diff --git a/vm/actor/src/builtin/miner/mod.rs b/vm/actor/src/builtin/miner/mod.rs index e90b825de39d..33004952b4e4 100644 --- a/vm/actor/src/builtin/miner/mod.rs +++ b/vm/actor/src/builtin/miner/mod.rs @@ -187,7 +187,7 @@ impl Actor { RT: Runtime<BS>, { let mut effective_epoch = ChainEpoch::default(); - rt.transaction::<State, Result<(), ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { rt.validate_immediate_caller_is(std::iter::once(&st.info.owner))?; let worker = resolve_worker_address(rt, params.new_worker)?; @@ -199,7 +199,7 @@ impl Actor { effective_at: effective_epoch, }); Ok(()) - })??; + })?; let cron_payload = CronEventPayload { event_type: CRON_EVENT_WORKER_KEY_CHANGE, @@ -214,11 +214,11 @@ impl Actor { BS: BlockStore, RT: Runtime<BS>, { - rt.transaction::<State, Result<(), ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { rt.validate_immediate_caller_is(std::iter::once(&st.info.worker))?; st.info.peer_id = params.new_id; Ok(()) - })??; + })?; Ok(()) } @@ -230,11 +230,11 @@ impl Actor { BS: BlockStore, RT: Runtime<BS>, { - rt.transaction::<State, Result<(), ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { rt.validate_immediate_caller_is(std::iter::once(&st.info.worker))?; st.info.multi_address = params.new_multi_addrs; Ok(()) - })??; + })?; Ok(()) } @@ -252,152 +252,151 @@ impl Actor { let mut recovered_sectors: Vec<SectorOnChainInfo> = Vec::new(); let mut penalty = TokenAmount::default(); - let sec_size = - rt.transaction::<State, Result<SectorSize, ActorError>, _>(|st: &mut State, rt| { - rt.validate_immediate_caller_is(std::iter::once(&st.info.worker))?; - - let partition_size = st.info.window_post_partition_sectors; - let submission_partition_limit = window_post_message_partitions_max(partition_size); - if params.partitions.len() as u64 > submission_partition_limit { - return Err(ActorError::new( - ExitCode::ErrIllegalArgument, - format!( - "too many partitions {}, limit {}", - params.partitions.len(), - submission_partition_limit - ), - )); - } - let deadline = st.deadline_info(current_epoch); - let mut deadlines = st.load_deadlines(rt.store()).map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("failed to load deadlines: {}", e), - ) - })?; + let sec_size = rt.transaction(|st: &mut State, rt| { + rt.validate_immediate_caller_is(std::iter::once(&st.info.worker))?; - // Traverse earlier submissions and enact detected faults. - // This isn't strictly necessary, but keeps the power table up to date eagerly and can force payment - // of penalties if locked pledge drops too low. - let (detected_faults, p) = - detect_faults_this_period(rt, st, rt.store(), &deadline, &mut deadlines)?; - detected_faults_sector = detected_faults; - penalty = p; + let partition_size = st.info.window_post_partition_sectors; + let submission_partition_limit = window_post_message_partitions_max(partition_size); + if params.partitions.len() as u64 > submission_partition_limit { + return Err(ActorError::new( + ExitCode::ErrIllegalArgument, + format!( + "too many partitions {}, limit {}", + params.partitions.len(), + submission_partition_limit + ), + )); + } + let deadline = st.deadline_info(current_epoch); + let mut deadlines = st.load_deadlines(rt.store()).map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to load deadlines: {}", e), + ) + })?; - if !deadline.period_started() { - return Err(ActorError::new( - ExitCode::ErrIllegalArgument, - format!( - "proving period {} not yet open at {}", - deadline.period_start, current_epoch - ), - )); - } + // Traverse earlier submissions and enact detected faults. + // This isn't strictly necessary, but keeps the power table up to date eagerly and can force payment + // of penalties if locked pledge drops too low. + let (detected_faults, p) = + detect_faults_this_period(rt, st, rt.store(), &deadline, &mut deadlines)?; + detected_faults_sector = detected_faults; + penalty = p; + + if !deadline.period_started() { + return Err(ActorError::new( + ExitCode::ErrIllegalArgument, + format!( + "proving period {} not yet open at {}", + deadline.period_start, current_epoch + ), + )); + } - if params.deadline != deadline.index as u64 { - return Err(ActorError::new( - ExitCode::ErrIllegalArgument, - format!( - "invalid deadline {} at epoch {}, expected {}", - params.deadline, current_epoch, deadline.index - ), - )); - } + if params.deadline != deadline.index as u64 { + return Err(ActorError::new( + ExitCode::ErrIllegalArgument, + format!( + "invalid deadline {} at epoch {}, expected {}", + params.deadline, current_epoch, deadline.index + ), + )); + } - // Verify locked funds are are at least the sum of sector initial pledges. - // Note that this call does not actually compute recent vesting, so the reported locked funds may be - // slightly higher than the true amount (i.e. slightly in the miner's favour). - // Computing vesting here would be almost always redundant since vesting is quantized to ~daily units. - // Vesting will be at most one proving period old if computed in the cron callback. - verify_pledge_meets_initial_requirements(rt, &st); + // Verify locked funds are are at least the sum of sector initial pledges. + // Note that this call does not actually compute recent vesting, so the reported locked funds may be + // slightly higher than the true amount (i.e. slightly in the miner's favour). + // Computing vesting here would be almost always redundant since vesting is quantized to ~daily units. + // Vesting will be at most one proving period old if computed in the cron callback. + verify_pledge_meets_initial_requirements(rt, &st); + + // TODO WPOST (follow-up): process Skipped as faults + + // Work out which sectors are due in the declared partitions at this deadline. + let partitions_sectors = compute_partitions_sector( + &mut deadlines, + partition_size, + deadline.index, + ¶ms.partitions, + ) + .map_err(|_| { + ActorError::new( + ExitCode::ErrIllegalState, + format!( + "failed to compute partitions sectors at deadline {}, partitions {:?}", + deadline.index, params.partitions + ), + ) + })?; - // TODO WPOST (follow-up): process Skipped as faults + let proven_sectors = BitField::union(&partitions_sectors); - // Work out which sectors are due in the declared partitions at this deadline. - let partitions_sectors = compute_partitions_sector( - &mut deadlines, - partition_size, - deadline.index, - ¶ms.partitions, - ) - .map_err(|_| { + let (sector_infos, declared_recoveries) = st + .load_sector_infos_for_proof(rt.store(), proven_sectors) + .map_err(|e| { ActorError::new( ExitCode::ErrIllegalState, - format!( - "failed to compute partitions sectors at deadline {}, partitions {:?}", - deadline.index, params.partitions - ), + format!("failed to load proven sector info: {}", e), ) })?; - let proven_sectors = BitField::union(&partitions_sectors); + // Verify the proof. + // A failed verification doesn't immediately cause a penalty; the miner can try again. + verify_windowed_post(rt, deadline.challenge, §or_infos, params.proofs.clone())?; - let (sector_infos, declared_recoveries) = st - .load_sector_infos_for_proof(rt.store(), proven_sectors) - .map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("failed to load proven sector info: {}", e), - ) - })?; - - // Verify the proof. - // A failed verification doesn't immediately cause a penalty; the miner can try again. - verify_windowed_post(rt, deadline.challenge, §or_infos, params.proofs.clone())?; + // Record the successful submission + let posted_partitions: BitField = + params.partitions.iter().map(|&i| i as usize).collect(); + let contains = st.post_submissions.contains_any(&posted_partitions); + if contains { + return Err(ActorError::new( + ExitCode::ErrIllegalArgument, + "duplicate PoSt partition".to_string(), + )); + } + st.add_post_submissions(posted_partitions).map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!( + "failed to record submissions for partitions: {:?}, {}", + params.partitions, e + ), + ) + })?; - // Record the successful submission - let posted_partitions: BitField = - params.partitions.iter().map(|&i| i as usize).collect(); - let contains = st.post_submissions.contains_any(&posted_partitions); - if contains { - return Err(ActorError::new( - ExitCode::ErrIllegalArgument, - "duplicate PoSt partition".to_string(), - )); - } - st.add_post_submissions(posted_partitions).map_err(|e| { + // If the PoSt was successful, the declared recoveries should be restored + st.remove_faults(rt.store(), &declared_recoveries) + .map_err(|e| { ActorError::new( ExitCode::ErrIllegalState, - format!( - "failed to record submissions for partitions: {:?}, {}", - params.partitions, e - ), + format!("failed to remove recoveries from faults: {}", e), ) })?; - // If the PoSt was successful, the declared recoveries should be restored - st.remove_faults(rt.store(), &declared_recoveries) - .map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("failed to remove recoveries from faults: {}", e), - ) - })?; - - st.remove_recoveries(&declared_recoveries).map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("failed to remove recoveries: {}", e), - ) - })?; + st.remove_recoveries(&declared_recoveries).map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to remove recoveries: {}", e), + ) + })?; - // Load info for recovered sectors for recovery of power outside this state transaction. - if declared_recoveries.is_empty() { - Ok(st.info.sector_size) - } else { - let mut sectors_by_number: HashMap<SectorNumber, SectorOnChainInfo> = - HashMap::new(); - for sec in sector_infos { - sectors_by_number.insert(sec.info.sector_number, sec); - } - declared_recoveries.iter().for_each(|i| { - let key = i as u64; - let s = sectors_by_number.get(&key).cloned().unwrap(); - recovered_sectors.push(s); - }); - Ok(st.info.sector_size) + // Load info for recovered sectors for recovery of power outside this state transaction. + if declared_recoveries.is_empty() { + Ok(st.info.sector_size) + } else { + let mut sectors_by_number: HashMap<SectorNumber, SectorOnChainInfo> = + HashMap::new(); + for sec in sector_infos { + sectors_by_number.insert(sec.info.sector_number, sec); } - })??; + declared_recoveries.iter().for_each(|i| { + let key = i as u64; + let s = sectors_by_number.get(&key).cloned().unwrap(); + recovered_sectors.push(s); + }); + Ok(st.info.sector_size) + } + })?; // Remove power for new faults, and burn penalties. request_begin_faults(rt, sec_size, &detected_faults_sector)?; burn_funds_and_notify_pledge_change(rt, penalty)?; @@ -544,7 +543,7 @@ impl Actor { ) })?; Ok(newly_vested_amount) - })??; + })?; notify_pledge_change(rt, &newly_vested_amount.neg())?; let mut bf = BitField::new(); @@ -721,11 +720,15 @@ impl Actor { let info = precommit.info; let deposit = precommit.pre_commit_deposit; - let vested_amount = - rt.transaction::<State, Result<TokenAmount, ActorError>, _>(|st, rt| { - let newly_vested_fund = st.unlock_vested_funds(rt.store(), current_epoch).map_err(|e| { - ActorError::new(ExitCode::ErrIllegalState, format!("failed to vest new funds: {}", e)) - })?; + let vested_amount = rt.transaction(|st: &mut State, rt| { + let newly_vested_fund = + st.unlock_vested_funds(rt.store(), current_epoch) + .map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to vest new funds: {}", e), + ) + })?; // unlock deposit for successful proof, make it available for lock-up as initial pledge st.subtract_pre_commit_deposit(&deposit); @@ -736,41 +739,70 @@ impl Actor { // lock up initial pledge for new sector let available_balance = st.get_available_balance(&rt.current_balance()?); if available_balance < initial_pledge { - return Err(ActorError::new(ExitCode::ErrInsufficientFunds, format!("insufficient funds for initial pledge requirement {}, available: {}", initial_pledge, available_balance))); + return Err(ActorError::new( + ExitCode::ErrInsufficientFunds, + format!( + "insufficient funds for initial pledge requirement {}, available: {}", + initial_pledge, available_balance + ), + )); } - st.add_locked_funds(rt.store(), current_epoch, &initial_pledge, PLEDGE_VESTING_SPEC).map_err(|e| { - ActorError::new(ExitCode::ErrIllegalState, format!("failed to add pledge: {}", e)) + st.add_locked_funds( + rt.store(), + current_epoch, + &initial_pledge, + PLEDGE_VESTING_SPEC, + ) + .map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to add pledge: {}", e), + ) })?; st.assert_balance_invariants(&rt.current_balance()?); - let new_sector_info = SectorOnChainInfo{ + let new_sector_info = SectorOnChainInfo { info, activation_epoch: current_epoch, deal_weight: deal_weights.deal_weight, - verified_deal_weight: deal_weights.verified_deal_weight + verified_deal_weight: deal_weights.verified_deal_weight, }; st.put_sector(rt.store(), new_sector_info).map_err(|e| { - ActorError::new(ExitCode::ErrIllegalState, format!("failed to prove commit: {}", e)) + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to prove commit: {}", e), + ) })?; - st.delete_precommitted_sector(rt.store(), num).map_err(|e| { - ActorError::new(ExitCode::ErrIllegalState, format!("failed to delete precommit for sector {}: {}", num, e)) - })?; + st.delete_precommitted_sector(rt.store(), num) + .map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to delete precommit for sector {}: {}", num, e), + ) + })?; - st.add_sector_expirations(rt.store(), expired_epoch, &[num]).map_err(|e| { - ActorError::new(ExitCode::ErrIllegalState, format!("failed to add new sector {} expiration: {}", num, e)) - })?; + st.add_sector_expirations(rt.store(), expired_epoch, &[num]) + .map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to add new sector {} expiration: {}", num, e), + ) + })?; // Add to new sectors, a staging ground before scheduling to a deadline at end of proving period. st.add_new_sectors(&[num]).map_err(|e| { - ActorError::new(ExitCode::ErrIllegalState, format!("failed to add new sector number {}: {}", num, e)) + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to add new sector number {}: {}", num, e), + ) })?; Ok(newly_vested_fund) - })??; + })?; notify_pledge_change(rt, &(initial_pledge - vested_amount))?; } @@ -887,7 +919,7 @@ impl Actor { })?; Ok(()) - })? + }) } fn terminate_sectors<BS, RT>( @@ -1049,7 +1081,7 @@ impl Actor { } Ok((penalty, st.info.sector_size)) - })??; + })?; // remove power for new faulty sectors detected_fault_sectors.append(&mut declared_fault_sectors); @@ -1149,7 +1181,7 @@ impl Actor { })?; Ok((penalty, st.info.sector_size)) - })??; + })?; // remove power for new faulty sectors request_begin_faults(rt, sector_size, &detected_fault_sectors)?; @@ -1195,7 +1227,7 @@ impl Actor { ) })?; Ok(newly_vested_amount) - })??; + })?; let delta = amount - vested_amount; notify_pledge_change(rt, &delta)?; Ok(()) @@ -1272,20 +1304,19 @@ impl Actor { // TODO negative amount requested will have inconsistent exit code // (we throw serialization error, will checked a signed integer and throw illegal argument) let st: State = rt.state()?; - let vested_amount = - rt.transaction::<State, Result<TokenAmount, ActorError>, _>(|st, rt| { - rt.validate_immediate_caller_is(std::iter::once(&st.info.owner))?; - let newly_vested_amount = st - .unlock_vested_funds(rt.store(), rt.curr_epoch()) - .map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("Failed to vest funds {:}", e), - ) - })?; + let vested_amount = rt.transaction(|st: &mut State, rt| { + rt.validate_immediate_caller_is(std::iter::once(&st.info.owner))?; + let newly_vested_amount = st + .unlock_vested_funds(rt.store(), rt.curr_epoch()) + .map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("Failed to vest funds {:}", e), + ) + })?; - Ok(newly_vested_amount) - })??; + Ok(newly_vested_amount) + })?; let curr_balance = rt.current_balance()?; let amount_withdrawn = std::cmp::min( @@ -1334,18 +1365,17 @@ where { // Vest locked funds. // This happens first so that any subsequent penalties are taken from locked pledge, rather than free funds. - let vested_amount = - rt.transaction::<State, Result<TokenAmount, ActorError>, _>(|st, rt| { - let newly_vested_fund = st - .unlock_vested_funds(rt.store(), rt.curr_epoch()) - .map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("failed to vest funds {:}", e), - ) - })?; - Ok(newly_vested_fund) - })??; + let vested_amount = rt.transaction(|st: &mut State, rt| { + let newly_vested_fund = st + .unlock_vested_funds(rt.store(), rt.curr_epoch()) + .map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to vest funds {:}", e), + ) + })?; + Ok(newly_vested_fund) + })?; notify_pledge_change(rt, &vested_amount.neg())?; @@ -1360,32 +1390,31 @@ where let mut detected_fault_sectors: Vec<SectorOnChainInfo> = Vec::new(); let curr_epoch = rt.curr_epoch(); let mut penalty = TokenAmount::default(); - let (sector_size, deadline) = - rt.transaction::<State, Result<_, ActorError>, _>(|st: &mut State, rt| { - let deadline = st.deadline_info(curr_epoch); + let (sector_size, deadline) = rt.transaction(|st: &mut State, rt| { + let deadline = st.deadline_info(curr_epoch); - if deadline.period_started() { - // Skip checking faults on the first, incomplete period. - let mut deadlines = st.load_deadlines(rt.store()).map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("failed to load deadlines {:}", e), - ) - })?; - let (detected_faults, p) = process_missing_post_faults( - rt, - st, - rt.store(), - &mut deadlines, - deadline.period_start, - deadline.index, - curr_epoch, - )?; - detected_fault_sectors = detected_faults; - penalty = p; - } - Ok((st.info.sector_size, deadline)) - })??; + if deadline.period_started() { + // Skip checking faults on the first, incomplete period. + let mut deadlines = st.load_deadlines(rt.store()).map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to load deadlines {:}", e), + ) + })?; + let (detected_faults, p) = process_missing_post_faults( + rt, + st, + rt.store(), + &mut deadlines, + deadline.period_start, + deadline.index, + curr_epoch, + )?; + detected_fault_sectors = detected_faults; + penalty = p; + } + Ok((st.info.sector_size, deadline)) + })?; // Remove power for new faults, and burn penalties. request_begin_faults(rt, sector_size, &detected_fault_sectors)?; @@ -1395,7 +1424,7 @@ where { // Expire sectors that are due. - let expired_sectors = rt.transaction::<State, Result<_, ActorError>, _>(|st, rt| { + let expired_sectors = rt.transaction(|st: &mut State, rt| { Ok( pop_sector_expirations(st, rt.store(), deadline.period_end()).map_err(|e| { ActorError::new( @@ -1404,7 +1433,7 @@ where ) })?, ) - })??; + })?; // Terminate expired sectors (sends messages to power and market actors). terminate_sectors(rt, &expired_sectors, SECTOR_TERMINATION_EXPIRED)?; @@ -1412,21 +1441,21 @@ where { // Terminate sectors with faults that are too old, and pay fees for ongoing faults. - let (expired_faults, ongoing_fault_penalty) = rt - .transaction::<State, Result<_, ActorError>, _>(|st, rt| { - let (expired_faults, ongoing_faults) = - pop_expired_faults(st, rt.store(), deadline.period_end() - FAULT_MAX_AGE) - .map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("failed to load fault sectors: {}", e), - ) - })?; + let (expired_faults, ongoing_fault_penalty) = rt.transaction(|st: &mut State, rt| { + let (expired_faults, ongoing_faults) = + pop_expired_faults(st, rt.store(), deadline.period_end() - FAULT_MAX_AGE).map_err( + |e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to load fault sectors: {}", e), + ) + }, + )?; - // Load info for ongoing faults. - // TODO: this is potentially super expensive for a large miner with ongoing faults - let ongoing_fault_info = st - .load_sector_infos(rt.store(), &ongoing_faults) + // Load info for ongoing faults. + // TODO: this is potentially super expensive for a large miner with ongoing faults + let ongoing_fault_info = + st.load_sector_infos(rt.store(), &ongoing_faults) .map_err(|e| { ActorError::new( ExitCode::ErrIllegalState, @@ -1434,22 +1463,22 @@ where ) })?; - // Unlock penalty for ongoing faults. - let ongoing_fault_penalty = unlock_penalty( - st, - rt.store(), - deadline.period_end(), - &ongoing_fault_info, - &pledge_penalty_for_sector_declared_fault, + // Unlock penalty for ongoing faults. + let ongoing_fault_penalty = unlock_penalty( + st, + rt.store(), + deadline.period_end(), + &ongoing_fault_info, + &pledge_penalty_for_sector_declared_fault, + ) + .map_err(|e| { + ActorError::new( + ExitCode::ErrIllegalState, + format!("failed to charge fault fee: {}", e), ) - .map_err(|e| { - ActorError::new( - ExitCode::ErrIllegalState, - format!("failed to charge fault fee: {}", e), - ) - })?; - Ok((expired_faults, ongoing_fault_penalty)) - })??; + })?; + Ok((expired_faults, ongoing_fault_penalty)) + })?; terminate_sectors(rt, &expired_faults, SECTOR_TERMINATION_FAULTY)?; burn_funds_and_notify_pledge_change(rt, ongoing_fault_penalty)?; @@ -1457,7 +1486,7 @@ where let proving_period_start = { // Establish new proving sets and clear proofs. - rt.transaction::<State, Result<_, ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { let mut deadlines = st.load_deadlines(rt.store()).map_err(|e| { ActorError::new( ExitCode::ErrIllegalState, @@ -1515,7 +1544,7 @@ where st.proving_period_start += WPOST_PROVING_PERIOD; } Ok(st.proving_period_start) - })?? + })? }; // Schedule cron callback for next period @@ -1804,7 +1833,7 @@ where { // initialize here to add together for all sectors and minimize calls across actors let mut deposit_burn = TokenAmount::default(); - rt.transaction::<State, Result<(), ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { if let Some(sectors) = optional_sectors { sectors .iter() @@ -1834,7 +1863,7 @@ where st.pre_commit_deposit -= &deposit_burn; Ok(()) - })??; + })?; // This deposit was locked separately to pledge collateral so there's no pledge change here. burn_funds(rt, deposit_burn)?; Ok(()) @@ -1860,7 +1889,7 @@ where let state: State = rt.state()?; let current_epoch = &rt.curr_epoch(); - rt.transaction::<State, Result<(), ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { let max_allowed_faults = st.get_max_allowed_faults(rt.store()).map_err(|e| { ActorError::new( ExitCode::ErrIllegalState, @@ -1931,7 +1960,7 @@ where .unwrap(); } Ok(()) - })??; + })?; // End any fault state before terminating sector power. request_end_faults(rt, state.info.sector_size, &faulty_sectors)?; @@ -2248,7 +2277,7 @@ where } Ok(()) - })? + }) } /// Verifies that the total locked balance exceeds the sum of sector initial pledges. diff --git a/vm/actor/src/builtin/multisig/mod.rs b/vm/actor/src/builtin/multisig/mod.rs index d52a11d211be..dd1f70e099c1 100644 --- a/vm/actor/src/builtin/multisig/mod.rs +++ b/vm/actor/src/builtin/multisig/mod.rs @@ -155,7 +155,7 @@ impl Actor { )?; Ok((t_id, txn)) - })??; + })?; let (applied, ret, code) = Self::approve_transaction(rt, txn_id, txn)?; @@ -191,7 +191,7 @@ impl Actor { // Go implementation holds reference to state after transaction so state must be cloned // to match to handle possible exit code inconsistency Ok((st.clone(), txn)) - })??; + })?; let (applied, ret, code) = execute_transaction_if_approved(rt, &st, id, txn.clone())?; if !applied { @@ -256,7 +256,7 @@ impl Actor { )?; Ok(()) - })? + }) } /// Multisig actor function to add signers to multisig @@ -289,7 +289,7 @@ impl Actor { } Ok(()) - })? + }) } /// Multisig actor function to remove signers to multisig @@ -330,7 +330,7 @@ impl Actor { st.num_approvals_threshold -= 1; } Ok(()) - })??; + })?; Ok(()) } @@ -376,7 +376,7 @@ impl Actor { st.signers.push(params.to); Ok(()) - })??; + })?; Ok(()) } @@ -402,7 +402,7 @@ impl Actor { // Update threshold on state st.num_approvals_threshold = params.new_threshold; Ok(()) - })??; + })?; Ok(()) } @@ -423,7 +423,7 @@ impl Actor { } } - let st = rt.transaction::<State, Result<_, ActorError>, _>(|st, rt| { + let st = rt.transaction(|st: &mut State, rt| { let mut ptx = make_map_with_root(&st.pending_txs, rt.store()).map_err( |e| actor_error!(ErrIllegalState; "failed to load pending transactions: {}", e), )?; @@ -443,7 +443,7 @@ impl Actor { // Go implementation holds reference to state after transaction so this must be cloned // to match to handle possible exit code inconsistency Ok(st.clone()) - })??; + })?; execute_transaction_if_approved(rt, &st, tx_id, txn) } @@ -479,7 +479,7 @@ where } applied = true; - rt.transaction::<State, Result<_, ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { let mut ptx = make_map_with_root::<_, Transaction>(&st.pending_txs, rt.store()) .map_err( |e| actor_error!(ErrIllegalState; "failed to load pending transactions: {}", e), @@ -497,7 +497,7 @@ where |e| actor_error!(ErrIllegalState; "failed to flush pending transactions: {}", e), )?; Ok(()) - })??; + })?; } Ok((applied, out, code)) diff --git a/vm/actor/src/builtin/paych/mod.rs b/vm/actor/src/builtin/paych/mod.rs index d3db4f8fb87f..9d3ff30a07dd 100644 --- a/vm/actor/src/builtin/paych/mod.rs +++ b/vm/actor/src/builtin/paych/mod.rs @@ -253,7 +253,7 @@ impl Actor { .flush() .map_err(|e| actor_error!(ErrIllegalState; "failed to save lanes: {}", e))?; Ok(()) - })? + }) } pub fn settle<BS, RT>(rt: &mut RT) -> Result<(), ActorError> @@ -274,7 +274,7 @@ impl Actor { } Ok(()) - })? + }) } pub fn collect<BS, RT>(rt: &mut RT) -> Result<(), ActorError> diff --git a/vm/actor/src/builtin/power/mod.rs b/vm/actor/src/builtin/power/mod.rs index 2c7c5f5c7f76..c19a96862a4e 100644 --- a/vm/actor/src/builtin/power/mod.rs +++ b/vm/actor/src/builtin/power/mod.rs @@ -104,7 +104,7 @@ impl Actor { )? .deserialize()?; - rt.transaction::<State, Result<(), ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { let mut claims = make_map_with_root(&st.claims, rt.store()) .map_err(|e| actor_error!(ErrIllegalState; "failed to load claims: {}", e))?; set_claim(&mut claims, &id_address, Claim::default()).map_err(|e| { @@ -117,7 +117,7 @@ impl Actor { .flush() .map_err(|e| actor_error!(ErrIllegalState; "failed to flush claims: {}", e))?; Ok(()) - })??; + })?; Ok(CreateMinerReturn { id_address, robust_address, @@ -162,7 +162,7 @@ impl Actor { .flush() .map_err(|e| actor_error!(ErrIllegalState; "failed to flush claims: {}", e))?; Ok(()) - })? + }) } fn enroll_cron_event<BS, RT>( @@ -186,7 +186,7 @@ impl Actor { "cron event epoch {} cannot be less than zero", params.event_epoch)); } - rt.transaction::<State, Result<_, ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { let mut events = Multimap::from_root(rt.store(), &st.cron_event_queue) .map_err(|e| actor_error!(ErrIllegalState; "failed to load cron events {}", e))?; @@ -197,7 +197,7 @@ impl Actor { .root() .map_err(|e| actor_error!(ErrIllegalState; "failed to flush cron events: {}", e))?; Ok(()) - })??; + })?; Ok(()) } @@ -220,7 +220,9 @@ impl Actor { st.update_smoothed_estimate(delta); st.last_processed_cron_epoch = rt.curr_epoch(); - Serialized::serialize(&BigIntSer(&st.this_epoch_raw_byte_power)) + Ok(Serialized::serialize(&BigIntSer( + &st.this_epoch_raw_byte_power, + ))) })?; // Update network KPA in reward actor @@ -243,6 +245,7 @@ impl Actor { rt.validate_immediate_caller_type(std::iter::once(&*MINER_ACTOR_CODE_ID))?; rt.transaction(|st: &mut State, _| { st.add_pledge_total(pledge_delta); + Ok(()) }) } @@ -300,7 +303,7 @@ impl Actor { .flush() .map_err(|e| actor_error!(ErrIllegalState; "failed to flush claims: {}", e))?; Ok(()) - })??; + })?; Ok(()) } @@ -349,7 +352,7 @@ impl Actor { rt.charge_gas("OnSubmitVerifySeal", GAS_ON_SUBMIT_VERIFY_SEAL)?; st.proof_validation_batch = Some(mmrc); Ok(()) - })??; + })?; Ok(()) } @@ -382,7 +385,7 @@ impl Actor { // Index map is needed here to preserve insertion order, miners must be iterated based // on order iterated through multimap. let mut verifies = IndexMap::new(); - rt.transaction::<State, Result<_, ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { if st.proof_validation_batch.is_none() { return Ok(()); } @@ -428,7 +431,7 @@ impl Actor { st.proof_validation_batch = None; Ok(()) - })??; + })?; // TODO update this to not need to create vector to verify these things (ref batch_v_s) let verif_arr: Vec<(Address, &Vec<SealVerifyInfo>)> = @@ -477,7 +480,7 @@ impl Actor { { let rt_epoch = rt.curr_epoch(); let mut cron_events = Vec::new(); - rt.transaction::<_, Result<_, ActorError>, _>(|st: &mut State, rt| { + rt.transaction(|st: &mut State, rt| { let mut events = Multimap::from_root(rt.store(), &st.cron_event_queue) .map_err(|e| actor_error!(ErrIllegalState; "failed to load cron events: {}", e))?; @@ -507,7 +510,7 @@ impl Actor { .map_err(|e| actor_error!(ErrIllegalState; "failed to flush events: {}", e))?; Ok(()) - })??; + })?; let mut failed_miner_crons = Vec::new(); for event in cron_events { @@ -530,7 +533,7 @@ impl Actor { failed_miner_crons.push(event.miner_addr) } } - rt.transaction::<State, Result<(), ActorError>, _>(|st, rt| { + rt.transaction(|st: &mut State, rt| { let mut claims = make_map_with_root(&st.claims, rt.store()) .map_err(|e| actor_error!(ErrIllegalState; "failed to load claims: {}", e))?; @@ -577,7 +580,7 @@ impl Actor { .flush() .map_err(|e| actor_error!(ErrIllegalState; "failed to flush claims: {}", e))?; Ok(()) - })??; + })?; Ok(()) } } diff --git a/vm/actor/src/builtin/reward/mod.rs b/vm/actor/src/builtin/reward/mod.rs index a1662abf4226..e75dbf01c995 100644 --- a/vm/actor/src/builtin/reward/mod.rs +++ b/vm/actor/src/builtin/reward/mod.rs @@ -99,7 +99,7 @@ impl Actor { .resolve_address(¶ms.miner)? .ok_or_else(|| actor_error!(ErrNotFound; "failed to resolve given owner address"))?; - let total_reward = rt.transaction::<State, Result<_, ActorError>, _>(|st, rt| { + let total_reward = rt.transaction(|st: &mut State, rt| { let mut block_reward = (&st.this_epoch_reward * params.win_count) / EXPECTED_LEADERS_PER_EPOCH; let mut total_reward = params.gas_reward.clone() + &block_reward; @@ -124,7 +124,7 @@ impl Actor { } st.total_mined += block_reward; Ok(total_reward) - })??; + })?; // Cap the penalty at the total reward value. let penalty = std::cmp::min(¶ms.penalty, &total_reward); @@ -224,6 +224,7 @@ impl Actor { st.update_to_next_epoch_with_reward(&curr_realized_power); st.update_smoothed_estimates(st.epoch - prev); + Ok(()) })?; Ok(()) } diff --git a/vm/actor/src/builtin/verifreg/mod.rs b/vm/actor/src/builtin/verifreg/mod.rs index 3d8958d28290..b63280e82c01 100644 --- a/vm/actor/src/builtin/verifreg/mod.rs +++ b/vm/actor/src/builtin/verifreg/mod.rs @@ -104,7 +104,7 @@ impl Actor { .map_err(|e| actor_error!(ErrIllegalState; "failed to flush verifiers: {}", e))?; Ok(()) - })??; + })?; Ok(()) } @@ -133,7 +133,7 @@ impl Actor { .flush() .map_err(|e| actor_error!(ErrIllegalState; "failed to flush verifiers: {}", e))?; Ok(()) - })??; + })?; Ok(()) } @@ -247,7 +247,7 @@ impl Actor { )?; Ok(()) - })??; + })?; Ok(()) } @@ -324,7 +324,7 @@ impl Actor { |e| actor_error!(ErrIllegalState; "failed to flush verified clients: {}", e), )?; Ok(()) - })??; + })?; Ok(()) } @@ -392,7 +392,7 @@ impl Actor { |e| actor_error!(ErrIllegalState; "failed to flush verified clients: {}", e), )?; Ok(()) - })??; + })?; Ok(()) } diff --git a/vm/actor/src/util/puppet/mod.rs b/vm/actor/src/util/puppet/mod.rs index 6ab4832679a1..776289ae0a1d 100644 --- a/vm/actor/src/util/puppet/mod.rs +++ b/vm/actor/src/util/puppet/mod.rs @@ -127,6 +127,7 @@ impl Actor { rt.transaction(|st: &mut State, _| { st.opt_fail = vec![UnmarshallableCBOR]; + Ok(()) })?; Ok(()) diff --git a/vm/actor/tests/common/mod.rs b/vm/actor/tests/common/mod.rs index 8353d8fe7939..d5efd38d5ce0 100644 --- a/vm/actor/tests/common/mod.rs +++ b/vm/actor/tests/common/mod.rs @@ -545,16 +545,17 @@ impl Runtime<MemoryDB> for MockRuntime { .unwrap()) } - fn transaction<C: Cbor, R, F>(&mut self, f: F) -> Result<R, ActorError> + fn transaction<C, RT, F>(&mut self, f: F) -> Result<RT, ActorError> where - F: FnOnce(&mut C, &mut Self) -> R, + C: Cbor, + F: FnOnce(&mut C, &mut Self) -> Result<RT, ActorError>, { if self.in_transaction { return Err(actor_error!(SysErrorIllegalActor; "nested transaction")); } let mut read_only = self.state()?; self.in_transaction = true; - let ret = f(&mut read_only, self); + let ret = f(&mut read_only, self)?; self.state = Some(self.put(&read_only).unwrap()); self.in_transaction = false; Ok(ret) diff --git a/vm/interpreter/src/default_runtime.rs b/vm/interpreter/src/default_runtime.rs index f5d89bafc021..15d284d3905f 100644 --- a/vm/interpreter/src/default_runtime.rs +++ b/vm/interpreter/src/default_runtime.rs @@ -460,7 +460,7 @@ where fn transaction<C, RT, F>(&mut self, f: F) -> Result<RT, ActorError> where C: Cbor, - F: FnOnce(&mut C, &mut Self) -> RT, + F: FnOnce(&mut C, &mut Self) -> Result<RT, ActorError>, { // get actor let act = self.state.get_actor(self.message().receiver()) @@ -476,7 +476,7 @@ where // Update the state self.allow_internal = false; - let r = f(&mut state, self); + let r = f(&mut state, self)?; self.allow_internal = true; let c = self.put(&state)?; diff --git a/vm/runtime/src/lib.rs b/vm/runtime/src/lib.rs index 66d70a6f0527..7845809deb61 100644 --- a/vm/runtime/src/lib.rs +++ b/vm/runtime/src/lib.rs @@ -98,9 +98,10 @@ pub trait Runtime<BS: BlockStore> { /// If the state is modified after this function returns, execution will abort. /// /// The gas cost of this method is that of a Store.Put of the mutated state object. - fn transaction<C: Cbor, R, F>(&mut self, f: F) -> Result<R, ActorError> + fn transaction<C, RT, F>(&mut self, f: F) -> Result<RT, ActorError> where - F: FnOnce(&mut C, &mut Self) -> R; + C: Cbor, + F: FnOnce(&mut C, &mut Self) -> Result<RT, ActorError>; /// Returns reference to blockstore fn store(&self) -> &BS;