Skip to content

Commit 48c8993

Browse files
rvaggrvagg
committed
fix(reward): address review feedback & add additional cbor fixtures
1 parent 4d816b7 commit 48c8993

5 files changed

Lines changed: 253 additions & 75 deletions

File tree

‎actors/reward/src/lib.rs‎

Lines changed: 81 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ impl Actor {
419419
rt.curr_epoch(),
420420
&prior_balance,
421421
&params.gas_reward,
422-
expected_block_reward,
422+
&expected_block_reward,
423423
);
424424
}
425425
};
@@ -429,43 +429,70 @@ impl Actor {
429429
.pending_writes
430430
.first()
431431
.is_some_and(|write| write.effective_epoch <= rt.curr_epoch());
432-
let (next_streams, mut next_accrued, apply_result, liabilities) = if transition_due {
433-
let current_streams = streams.clone();
434-
let mut next_streams = streams;
435-
let mut next_accrued = st.accrued.clone();
436-
let apply_result =
437-
apply_due_writes(&mut next_streams, &mut next_accrued, rt.curr_epoch())
438-
.map_err(|e| {
439-
e.downcast_default(
440-
ExitCode::USR_ILLEGAL_STATE,
441-
"failed to apply due writes",
442-
)
443-
})?;
444-
let liabilities = match compute_service_liability(&next_streams, &next_accrued) {
445-
Ok(liabilities) => liabilities,
446-
Err(error) => {
447-
error!(
448-
"due writes produced invalid explicit-stream accounting at epoch {}: {};\
449-
suspending service accrual",
450-
rt.curr_epoch(),
451-
error
452-
);
453-
return allocate_without_service(
454-
st,
455-
&current_streams,
456-
rt.curr_epoch(),
457-
&prior_balance,
458-
&params.gas_reward,
459-
expected_block_reward,
460-
);
461-
}
432+
let (next_streams, mut next_accrued, apply_result, liabilities, prior_streams) =
433+
if transition_due {
434+
let current_streams = streams.clone();
435+
let mut next_streams = streams;
436+
let mut next_accrued = st.accrued.clone();
437+
let apply_result =
438+
match apply_due_writes(&mut next_streams, &mut next_accrued, rt.curr_epoch())
439+
{
440+
Ok(result) => result,
441+
Err(error) => {
442+
error!(
443+
"failed to apply due writes at epoch {}: {};\
444+
suspending service accrual",
445+
rt.curr_epoch(),
446+
error
447+
);
448+
return allocate_without_service(
449+
st,
450+
&current_streams,
451+
rt.curr_epoch(),
452+
&prior_balance,
453+
&params.gas_reward,
454+
&expected_block_reward,
455+
);
456+
}
457+
};
458+
let liabilities =
459+
match compute_service_liability(&next_streams, &next_accrued) {
460+
Ok(liabilities) => liabilities,
461+
Err(error) => {
462+
error!(
463+
"due writes produced invalid explicit-stream accounting at epoch {}: {};\
464+
suspending service accrual",
465+
rt.curr_epoch(),
466+
error
467+
);
468+
return allocate_without_service(
469+
st,
470+
&current_streams,
471+
rt.curr_epoch(),
472+
&prior_balance,
473+
&params.gas_reward,
474+
&expected_block_reward,
475+
);
476+
}
477+
};
478+
(
479+
next_streams,
480+
next_accrued,
481+
apply_result,
482+
liabilities,
483+
Some(current_streams),
484+
)
485+
} else {
486+
(
487+
streams,
488+
st.accrued.clone(),
489+
ApplyResult::default(),
490+
current_liabilities,
491+
None,
492+
)
462493
};
463-
(next_streams, next_accrued, apply_result, liabilities)
464-
} else {
465-
(streams, st.accrued.clone(), ApplyResult::default(), current_liabilities)
466-
};
467494

468-
let mut block_reward = expected_block_reward;
495+
let mut block_reward = expected_block_reward.clone();
469496
// Due folds leave dust out of the derived liability before its post-transaction
470497
// burn send, so it remains reserved until that send executes.
471498
let reserved = &params.gas_reward + &liabilities + &apply_result.burn;
@@ -514,9 +541,22 @@ impl Actor {
514541
ApplyResult::default(),
515542
));
516543
}
517-
accrue_service(&mut next_accrued, &allocation.service).map_err(|e| {
518-
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to accrue service reward")
519-
})?;
544+
if let Err(error) = accrue_service(&mut next_accrued, &allocation.service) {
545+
error!(
546+
"failed to accrue explicit-stream reward at epoch {}: {};\
547+
suspending service accrual",
548+
rt.curr_epoch(),
549+
error
550+
);
551+
return allocate_without_service(
552+
st,
553+
prior_streams.as_ref().unwrap_or(&next_streams),
554+
rt.curr_epoch(),
555+
&prior_balance,
556+
&params.gas_reward,
557+
&expected_block_reward,
558+
);
559+
}
520560
let service = allocation
521561
.service
522562
.iter()
@@ -644,8 +684,9 @@ fn allocate_without_service(
644684
epoch: ChainEpoch,
645685
prior_balance: &TokenAmount,
646686
gas_reward: &TokenAmount,
647-
mut block_reward: TokenAmount,
687+
block_reward: &TokenAmount,
648688
) -> Result<(TokenAmount, TokenAmount, ApplyResult), ActorError> {
689+
let mut block_reward = block_reward.clone();
649690
let supply_remaining = &*STORAGE_MINING_ALLOCATION - &state.total_minted_reward;
650691
let available_reward = prior_balance - gas_reward;
651692
if supply_remaining <= TokenAmount::zero() || available_reward <= TokenAmount::zero() {

‎actors/reward/src/state.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,6 @@ impl State {
179179
self.this_epoch_reward_smoothed =
180180
filter_reward.next_estimate(self.this_epoch_reward.atto(), delta);
181181
}
182-
183-
pub fn into_total_minted_reward(self) -> TokenAmount {
184-
self.total_minted_reward
185-
}
186182
}
187183

188184
/// Defines vestion function type for reward actor.

‎actors/reward/src/streams.rs‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ fn set_shares_inner(
494494

495495
let mut next_distribution = distribution.clone();
496496
let burn = settle_period(&mut next_distribution, &accrual.amount)?;
497-
let reserved_rows = payable_row_reservation(&next_distribution.payable, &shares);
497+
let reserved_rows = recipient_union_len(&next_distribution.payable, &shares);
498498
ensure!(
499499
reserved_rows <= MAX_PAYABLE_ROWS_PER_STREAM,
500500
"stream {id} payable row reservation {reserved_rows} exceeds maximum {MAX_PAYABLE_ROWS_PER_STREAM}"
@@ -613,22 +613,6 @@ fn claim_payable(payable: &mut Vec<RecipientAmount>, wallets: &[Address]) -> Res
613613
Ok(amounts)
614614
}
615615

616-
fn payable_row_reservation(payable: &[RecipientAmount], shares: &[RecipientShare]) -> usize {
617-
let (mut payable_idx, mut share_idx, mut rows) = (0, 0, 0);
618-
while payable_idx < payable.len() && share_idx < shares.len() {
619-
rows += 1;
620-
match payable[payable_idx].recipient.cmp(&shares[share_idx].recipient) {
621-
std::cmp::Ordering::Less => payable_idx += 1,
622-
std::cmp::Ordering::Greater => share_idx += 1,
623-
std::cmp::Ordering::Equal => {
624-
payable_idx += 1;
625-
share_idx += 1;
626-
}
627-
}
628-
}
629-
rows + payable.len() - payable_idx + shares.len() - share_idx
630-
}
631-
632616
fn validate_period_claims(distribution: &ExplicitDistribution, pool: &TokenAmount) -> Result<()> {
633617
validate_amount_rows(&distribution.payable, "payable")?;
634618
validate_amount_rows(&distribution.claimed_period, "claimed-period")?;
@@ -1126,8 +1110,7 @@ fn validate_stream_configuration_without_weights(streams: &[Stream]) -> Result<(
11261110
validate_shares(&distribution.shares)?;
11271111
validate_amount_rows(&distribution.payable, "payable")?;
11281112
validate_amount_rows(&distribution.claimed_period, "claimed-period")?;
1129-
let reserved_rows =
1130-
payable_row_reservation(&distribution.payable, &distribution.shares);
1113+
let reserved_rows = recipient_union_len(&distribution.payable, &distribution.shares);
11311114
ensure!(
11321115
reserved_rows <= MAX_PAYABLE_ROWS_PER_STREAM,
11331116
"stream {} payable row reservation {reserved_rows} exceeds maximum {MAX_PAYABLE_ROWS_PER_STREAM}",

‎actors/reward/src/streams_test.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,7 @@ fn caps_payable_rows_at_128_rejects_129_atomically_and_recovers_through_claims()
750750
let distribution = streams.streams[0].distribution.as_ref().unwrap();
751751
assert_eq!(
752752
MAX_PAYABLE_ROWS_PER_STREAM,
753-
payable_row_reservation(&distribution.payable, &distribution.shares,)
753+
recipient_union_len(&distribution.payable, &distribution.shares)
754754
);
755755

756756
accruals[0].amount = TokenAmount::from_atto(MAX_RECIPIENTS as u64);

0 commit comments

Comments
 (0)