Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Scarb.lock
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,15 @@ dependencies = [
[[package]]
name = "starkware_utils"
version = "1.0.0"
source = "git+https://github.com/starkware-libs/starkware-starknet-utils?rev=e1955423808045de868987b8fb0b43f5cbdf5699#e1955423808045de868987b8fb0b43f5cbdf5699"
source = "git+https://github.com/starkware-libs/starkware-starknet-utils?rev=7b46975d5612fb1b920bb248941030bf6c295d44#7b46975d5612fb1b920bb248941030bf6c295d44"
dependencies = [
"openzeppelin",
]

[[package]]
name = "starkware_utils_testing"
version = "1.0.0"
source = "git+https://github.com/starkware-libs/starkware-starknet-utils?rev=e1955423808045de868987b8fb0b43f5cbdf5699#e1955423808045de868987b8fb0b43f5cbdf5699"
source = "git+https://github.com/starkware-libs/starkware-starknet-utils?rev=7b46975d5612fb1b920bb248941030bf6c295d44#7b46975d5612fb1b920bb248941030bf6c295d44"
dependencies = [
"openzeppelin",
"snforge_std",
Expand Down
4 changes: 2 additions & 2 deletions Scarb.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ edition = "2024_07"
[dependencies]
starknet = "2.12.0"
openzeppelin = "2.0.0"
starkware_utils = { git = "https://github.com/starkware-libs/starkware-starknet-utils", rev = "e1955423808045de868987b8fb0b43f5cbdf5699" }
starkware_utils = { git = "https://github.com/starkware-libs/starkware-starknet-utils", rev = "7b46975d5612fb1b920bb248941030bf6c295d44" }

[dev-dependencies]
assert_macros = "2.12.0"
snforge_std = "0.49.0"
starkware_utils_testing = { git = "https://github.com/starkware-libs/starkware-starknet-utils", rev = "e1955423808045de868987b8fb0b43f5cbdf5699" }
starkware_utils_testing = { git = "https://github.com/starkware-libs/starkware-starknet-utils", rev = "7b46975d5612fb1b920bb248941030bf6c295d44" }

[scripts]
test = "snforge test"
Expand Down
1 change: 1 addition & 0 deletions src/flow_test/flow_ideas.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@
- advance blocks with different block times and check the avg is calculated correctly
- update_rewards for blocks in same epoch - same rewards, then advance epoch, different rewards, update rewards for blocks in same epoch - same rewards.
- update rewards is not called every block, still rewards is updated correctly (miss block, miss first block in epoch, miss epoch)
- set block time config and test rewards after
13 changes: 10 additions & 3 deletions src/reward_supplier/reward_supplier.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ pub mod RewardSupplier {
use starkware_utils::erc20::erc20_utils::CheckedIERC20DispatcherTrait;
use starkware_utils::errors::OptionAuxTrait;
use starkware_utils::interfaces::identity::Identity;
use starkware_utils::math::utils::{ceil_of_division, mul_wide_and_div};
use starkware_utils::math::utils::{Clamp, ceil_of_division, mul_wide_and_div};
use starkware_utils::time::time::Timestamp;

pub const CONTRACT_IDENTITY: felt252 = 'Reward Supplier';
pub const CONTRACT_VERSION: felt252 = '3.0.0';
/// Scale factor for block duration measurements. 100 implies granularity of 100th of second.
Expand Down Expand Up @@ -367,11 +368,17 @@ pub mod RewardSupplier {
// We calculate `num_blocks` instead of using the configured value to keep the average
// accurate even if some calls are missed.
let num_blocks = current_block_number - snapshot_block_number;
let calculated_block_duration = mul_wide_and_div(
let mut calculated_block_duration = mul_wide_and_div(
lhs: time_delta, rhs: BLOCK_DURATION_SCALE, div: num_blocks,
)
.expect_with_err(err: Error::BLOCK_DURATION_OVERFLOW);
// TODO: Adjust calculated_block_duration with MIN_BLOCK_TIME and MAX_BLOCK_TIME.
// Adjust calculated_block_duration with min and max block duration.
let block_duration_config = self.block_duration_config.read();
calculated_block_duration = calculated_block_duration
.clamp(
block_duration_config.min_block_duration,
block_duration_config.max_block_duration,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clamp with uninitialized config zeroes out block rewards

High Severity

For upgraded contracts (where the constructor doesn't run), block_duration_config remains at its default storage value of {0, 0} since the EIC to initialize it is only a TODO. The new .clamp(0, 0) call forces calculated_block_duration to 0 on every epoch, which then sets avg_block_duration to 0, making total_rewards permanently zero. Before this change, the raw calculated value was used directly, allowing the system to self-correct. Now there's no recovery without an external set_block_duration_config call or EIC deployment.

Additional Locations (1)
Fix in Cursor Fix in Web

self.avg_block_duration.write(calculated_block_duration);
}
}
Expand Down
92 changes: 89 additions & 3 deletions src/reward_supplier/test.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ use starkware_utils_testing::test_utils::{
advance_block_number_global, assert_panic_with_error, cheat_caller_address_once, check_identity,
};
use test_utils::{
StakingInitConfig, advance_epoch_global, advance_k_epochs_global, advance_time_global, fund,
general_contract_system_deployment, initialize_reward_supplier_state_from_cfg, load_one_felt,
stake_for_testing_using_dispatcher,
StakingInitConfig, advance_epoch_global, advance_epoch_global_custom_time,
advance_k_epochs_global, advance_time_global, fund, general_contract_system_deployment,
initialize_reward_supplier_state_from_cfg, load_one_felt, stake_for_testing_using_dispatcher,
};

#[test]
Expand Down Expand Up @@ -602,3 +602,89 @@ fn test_set_block_duration_config_assertions() {
:result, expected_error: Error::INVALID_MIN_MAX_BLOCK_DURATION.describe(),
);
}

#[test]
fn test_update_current_epoch_block_rewards_with_adjustments() {
let mut cfg: StakingInitConfig = Default::default();
general_contract_system_deployment(ref :cfg);
stake_for_testing_using_dispatcher(:cfg);
advance_k_epochs_global();
let reward_supplier = cfg.staking_contract_info.reward_supplier;
let reward_supplier_dispatcher = IRewardSupplierDispatcher {
contract_address: reward_supplier,
};
let minting_curve_dispatcher = IMintingCurveDispatcher {
contract_address: cfg.reward_supplier.minting_curve_contract,
};
let staking_contract = cfg.test_info.staking_contract;
// First snapshot, not update avg_block_time. Rewards are calculated using the default avg block
// time.
cheat_caller_address_once(contract_address: reward_supplier, caller_address: staking_contract);
let (_, _) = reward_supplier_dispatcher.update_current_epoch_block_rewards();
let mut curr_avg_block_time = DEFAULT_AVG_BLOCK_DURATION;
// Adjust avg_block_time to MIN (avg is less than min).
let min_block_time = DEFAULT_BLOCK_DURATION_CONFIG.min_block_duration;
advance_epoch_global_custom_time(
block_time: TimeDelta { seconds: min_block_time / BLOCK_DURATION_SCALE - 1 },
);
cheat_caller_address_once(contract_address: reward_supplier, caller_address: staking_contract);
let (strk_rewards, btc_rewards) = reward_supplier_dispatcher
.update_current_epoch_block_rewards();
// Test avg_block_time.
curr_avg_block_time = min_block_time;
let avg_block_time = load_one_felt(
target: reward_supplier, storage_address: selector!("avg_block_duration"),
)
.try_into()
.unwrap();
assert!(avg_block_time == curr_avg_block_time);
// Test rewards.
let yearly_mint = minting_curve_dispatcher.yearly_mint();
let expected_rewards = mul_wide_and_div(
lhs: yearly_mint,
rhs: curr_avg_block_time.into(),
div: BLOCK_DURATION_SCALE.into() * SECONDS_IN_YEAR.into(),
)
.expect_with_err(err: InternalError::REWARDS_COMPUTATION_OVERFLOW);
let expected_btc_rewards = mul_wide_and_div(
lhs: expected_rewards, rhs: ALPHA, div: ALPHA_DENOMINATOR,
)
.unwrap();
let expected_strk_rewards = expected_rewards - expected_btc_rewards;
assert!(expected_strk_rewards.is_non_zero());
assert!(expected_btc_rewards.is_non_zero());
assert!(strk_rewards == expected_strk_rewards);
assert!(btc_rewards == expected_btc_rewards);
// Adjust avg_block_time to MAX (avg is more than max).
let max_block_time = DEFAULT_BLOCK_DURATION_CONFIG.max_block_duration;
advance_epoch_global_custom_time(
block_time: TimeDelta { seconds: max_block_time / BLOCK_DURATION_SCALE + 1 },
);
cheat_caller_address_once(contract_address: reward_supplier, caller_address: staking_contract);
let (strk_rewards, btc_rewards) = reward_supplier_dispatcher
.update_current_epoch_block_rewards();
// Test avg_block_time.
curr_avg_block_time = max_block_time;
let avg_block_time = load_one_felt(
target: reward_supplier, storage_address: selector!("avg_block_duration"),
)
.try_into()
.unwrap();
assert!(avg_block_time == curr_avg_block_time);
// Test rewards.
let expected_rewards = mul_wide_and_div(
lhs: yearly_mint,
rhs: curr_avg_block_time.into(),
div: BLOCK_DURATION_SCALE.into() * SECONDS_IN_YEAR.into(),
)
.expect_with_err(err: InternalError::REWARDS_COMPUTATION_OVERFLOW);
let expected_btc_rewards = mul_wide_and_div(
lhs: expected_rewards, rhs: ALPHA, div: ALPHA_DENOMINATOR,
)
.unwrap();
let expected_strk_rewards = expected_rewards - expected_btc_rewards;
assert!(expected_strk_rewards.is_non_zero());
assert!(expected_btc_rewards.is_non_zero());
assert!(strk_rewards == expected_strk_rewards);
assert!(btc_rewards == expected_btc_rewards);
}
7 changes: 7 additions & 0 deletions src/test_utils.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,13 @@ pub(crate) fn advance_blocks(blocks: u64, block_duration: Seconds) {
advance_time_global(time: TimeDelta { seconds: block_duration * blocks });
}

/// Advance one epoch with the given `block_time` per block.
pub(crate) fn advance_epoch_global_custom_time(block_time: TimeDelta) {
advance_block_number_global(blocks: EPOCH_LENGTH.into());
let time = TimeDelta { seconds: block_time.seconds * EPOCH_LENGTH.into() };
advance_time_global(:time);
}

// ---- Calculate Rewards - V0 (index based) -----

/// Update rewards for STRK pool.
Expand Down
Loading