Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat staker v2 no fp and subpointers #67

Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
cb3e4dd
Initial implementation of staked seconds calculation
baitcode Dec 10, 2024
01b3f73
a bit of naming
baitcode Dec 10, 2024
924aef3
added upgrade method. NOTE: constructor changed. Staker now requires …
baitcode Dec 10, 2024
88f8db8
fix import
baitcode Dec 10, 2024
9553035
TODO: removal
baitcode Dec 10, 2024
aedb403
remove todo
baitcode Dec 10, 2024
9ee9324
test
baitcode Dec 11, 2024
c3c0244
local settings
baitcode Dec 11, 2024
692c36a
revert upgrade code for now
baitcode Dec 11, 2024
349919e
removed upgrade due to cyclic dependency with governor
baitcode Dec 11, 2024
d6504e9
finalise
baitcode Dec 11, 2024
5f32326
typo
baitcode Dec 11, 2024
b5ac50b
Added UFixedPoint type for unsigned FixedPoint operations. Currently …
baitcode Dec 13, 2024
2f9516a
removed old tests
baitcode Dec 13, 2024
bb4f99a
* debugged fixed point math. added tests
baitcode Dec 14, 2024
1a2d6fc
some comment fixes
baitcode Jan 3, 2025
b18d702
Extracted staker storage into separate file due to large amount of bo…
baitcode Jan 6, 2025
7fdfb19
rename staker_storage -> staker_log. moved all relevant logic there
baitcode Jan 6, 2025
b82953f
Simplified Fixed Point operations library. Reduced internal storage s…
baitcode Jan 6, 2025
80cf041
simplified more. better naming
baitcode Jan 6, 2025
f507631
clean up bitshifts from tests
baitcode Jan 6, 2025
164f64f
overflow checks
baitcode Jan 6, 2025
6e931c6
old tests did not respect overflow. had to create new ones.
baitcode Jan 7, 2025
fafba15
Debugging, cleaning up, fixing missed issues
baitcode Jan 8, 2025
b55b320
final fix
baitcode Jan 9, 2025
a28971c
final debugging
baitcode Jan 9, 2025
aa07801
cleanup
baitcode Jan 9, 2025
61f085c
cleanup
baitcode Jan 9, 2025
f52fff1
removing UFixedPoint124x128 struct
baitcode Jan 10, 2025
4cc807e
BugFix for a case when fetl252 is overflown
baitcode Jan 10, 2025
7e35137
remove fixed point lib
baitcode Jan 10, 2025
815d539
small fix
baitcode Jan 10, 2025
284ae39
lost deletions
baitcode Jan 10, 2025
c6102a1
Review fixes
baitcode Jan 13, 2025
e3067ab
scarb fmt
baitcode Jan 13, 2025
f622f34
Review fixes
baitcode Jan 14, 2025
56ec424
scarb format
baitcode Jan 14, 2025
974c68d
* remove MAX_FP related code
baitcode Jan 23, 2025
955ef24
simplified and renamed get_seconds_per_total_staked_sum_at
baitcode Jan 31, 2025
83eb22d
uncomment tests + scarb format
baitcode Jan 31, 2025
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: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
.env
.vscode
.starkli
.DS_Store
#######
Scarb
#######
Expand Down
4 changes: 4 additions & 0 deletions src/lib.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ pub mod governor;
mod governor_test;

pub mod staker;

pub mod staker_log;
#[cfg(test)]
pub mod staker_log_test;
#[cfg(test)]
mod staker_test;

Expand Down
88 changes: 85 additions & 3 deletions src/staker.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,28 @@ pub trait IStaker<TContractState> {
fn get_average_delegated_over_last(
self: @TContractState, delegate: ContractAddress, period: u64,
) -> u128;

// Gets the cumulative staked amount * per second staked for the given timestamp and account.
Copy link
Member

Choose a reason for hiding this comment

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

is this comment accurate? there were 2 diff quantities we wanted to be able to compute:

average total staked for a period of time

  • requires storing seconds passed * total_staked, a 160+ bit wide value
  • can be computed by taking (snapshotB-snapshotA) / periodLength
  • can be used to calculate the total voting weight at the time a proposal's voting starts, so functionality could be added for proposal to be immediately passed if it reaches quorum with majority of total staked in favor

user's share of total stake over a period

  • requires storing seconds passed / total staked, a 160+ bit wide value
  • computed via stakedAmount * snapshotStakeEnd - snapshotStakeStart
  • this also lets you compute the harmonic mean of total staked over a period, which could be useful for dynamically adjusting quorum

Copy link
Author

Choose a reason for hiding this comment

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

No it's not. It's outdated. I've updated it. This method calculates shapshots for "user's share of total stake over a period"

Fixed. That one. Will add other snapshots soon. Then add methods for 2 diff quantities.

Copy link
Author

@baitcode baitcode Jan 31, 2025

Choose a reason for hiding this comment

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

@moodysalem

computed via stakedAmount * snapshotStakeEnd - snapshotStakeStart

Formula for user's share seems incorrect:

  1. the dimension of the result is seconds. which raises suspicion.
  2. brackets are missing.

I think that correct one is: stakedAmount * (snapshotStakeEnd - snapshotStakeStart) / (snapshotTimestampEnd-snapshotTimestampStart)
Can you please confirm that this is correct?

fn get_cumulative_seconds_per_total_staked_at(self: @TContractState, timestamp: u64) -> u256;
}

#[starknet::contract]
pub mod Staker {
use core::integer::{u512, u512_safe_div_rem_by_u256};
use core::num::traits::zero::{Zero};
use crate::staker_log::{LogOperations, MAX_FP, StakingLog};
use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait};
use starknet::storage::VecTrait;
use starknet::storage::{
Map, StorageMapReadAccess, StorageMapWriteAccess, StoragePathEntry,
StoragePointerReadAccess, StoragePointerWriteAccess,
};

use starknet::{
get_block_timestamp, get_caller_address, get_contract_address,
ContractAddress, get_block_timestamp, get_caller_address, get_contract_address,
storage_access::{StorePacking},
};
use super::{ContractAddress, IStaker};

use super::{IStaker};

#[derive(Copy, Drop, PartialEq, Debug)]
pub struct DelegatedSnapshot {
Expand All @@ -78,6 +84,7 @@ pub mod Staker {
const TWO_POW_64: u128 = 0x10000000000000000;
const TWO_POW_192: u256 = 0x1000000000000000000000000000000000000000000000000;
const TWO_POW_192_DIVISOR: NonZero<u256> = 0x1000000000000000000000000000000000000000000000000;
const TWO_POW_127: u128 = 0x80000000000000000000000000000000_u128;

pub(crate) impl DelegatedSnapshotStorePacking of StorePacking<DelegatedSnapshot, felt252> {
fn pack(value: DelegatedSnapshot) -> felt252 {
Expand All @@ -104,6 +111,8 @@ pub mod Staker {
amount_delegated: Map<ContractAddress, u128>,
delegated_cumulative_num_snapshots: Map<ContractAddress, u64>,
delegated_cumulative_snapshot: Map<ContractAddress, Map<u64, DelegatedSnapshot>>,
total_staked: u128,
staking_log: StakingLog,
}

#[constructor]
Expand Down Expand Up @@ -253,6 +262,12 @@ pub mod Staker {
self
.amount_delegated
.write(delegate, self.insert_snapshot(delegate, get_block_timestamp()) + amount);

let total_staked = self.total_staked.read();

self.total_staked.write(total_staked + amount);
self.staking_log.log_change(amount, total_staked);
Copy link
Member

Choose a reason for hiding this comment

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

is it required to write total staked before calling log_change? if so, it's not locally obvious from the function names and arguments

Copy link
Author

Choose a reason for hiding this comment

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

it is not. I'll think on how to rename those.


self.emit(Staked { from, delegate, amount });
}

Expand Down Expand Up @@ -280,6 +295,11 @@ pub mod Staker {
.amount_delegated
.write(delegate, self.insert_snapshot(delegate, get_block_timestamp()) - amount);
assert(self.token.read().transfer(recipient, amount.into()), 'TRANSFER_FAILED');

let total_staked = self.total_staked.read();
self.total_staked.write(total_staked - amount);
self.staking_log.log_change(amount, total_staked);

self.emit(Withdrawn { from, delegate, to: recipient, amount });
}

Expand Down Expand Up @@ -333,5 +353,67 @@ pub mod Staker {
let now = get_block_timestamp();
self.get_average_delegated(delegate, now - period, now)
}

fn get_cumulative_seconds_per_total_staked_at(
self: @ContractState, timestamp: u64,
) -> u256 {
if let Option::Some((log_record, idx)) = self
.staking_log
.find_in_change_log(timestamp) {
Copy link
Member

Choose a reason for hiding this comment

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

it's not finding that specific timestamp, maybe rename to find_previous.. or something

Copy link
Author

Choose a reason for hiding this comment

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

Good point.

Copy link
Author

Choose a reason for hiding this comment

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

Changed to find_change_log_on_or_before_timestamp.

let total_staked = if (idx == self.staking_log.len() - 1) {
// if last rescord found
self.total_staked.read()
} else {
// otherwise calculate using cumulative_seconds_per_total_staked difference
let next_log_record = self.staking_log.at(idx + 1).read();

// substract fixed point values
let divisor = next_log_record.cumulative_seconds_per_total_staked
- log_record.cumulative_seconds_per_total_staked;

if divisor.is_zero() {
return 0_u64.into();
Copy link
Member

Choose a reason for hiding this comment

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

in hindsight, we should actually treat total staked of 0 as if it's 1 with regards to this accumulator

that allows us to still compute the harmonic mean for periods where total staked is zero (it will not be perfectly accurate, but ok for it to be off by one)

Copy link
Member

Choose a reason for hiding this comment

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

it also means you don't have to worry about this value being 0 between two indices

Copy link
Author

Choose a reason for hiding this comment

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

I refactored that bit. I calculate total_staked using total_staked * seconds passed. It's much simpler.

}

let diff_seconds: u128 = (next_log_record.timestamp - log_record.timestamp)
.into();

// Divide u64 by fixed point
let (total_staked_fp_medium, _) = u512_safe_div_rem_by_u256(
u512 { limb0: 0, limb1: 0, limb2: 0, limb3: diff_seconds },
divisor.try_into().unwrap(),
);

let total_staked_fp = u256 {
low: total_staked_fp_medium.limb1, high: total_staked_fp_medium.limb2,
};

assert(total_staked_fp.high < MAX_FP, 'FP_OVERFLOW');

// round value
total_staked_fp.high + if (total_staked_fp.low >= TWO_POW_127) {
1
} else {
0
}
};

let seconds_diff = timestamp - log_record.timestamp;

let staked_seconds: u256 = if total_staked == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

variable name is confusing

Copy link
Author

Choose a reason for hiding this comment

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

changed to seconds_per_total_staked

0_u256
Copy link
Member

Choose a reason for hiding this comment

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

here you can just use the value 1 for total_staked

Copy link
Author

Choose a reason for hiding this comment

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

@moodysalem
Do I understand you correctly, that this is a technic to cap the infinity? meaning that we probably wont really calculate stake shares over periods like that and those will be negated when snapshots are subtracted?

} else {
// Divide u64 by u128
u256 { low: 0, high: seconds_diff.into() } / total_staked.into()
};

// Sum fixed posits
let result = log_record.cumulative_seconds_per_total_staked + staked_seconds;
assert(result.high < MAX_FP, 'FP_OVERFLOW');
Copy link
Member

Choose a reason for hiding this comment

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

why are you doing this assertion? if it's because you're storing a u256 as a felt252 in the store packing, then it should be asserted where you're actually doing the storage

Copy link
Author

Choose a reason for hiding this comment

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

Had same though, but decided to fail as early as possible.

Copy link
Author

@baitcode baitcode Jan 23, 2025

Choose a reason for hiding this comment

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

Looks like in storage packing it'll fail anyway during conversion. Removing.

return result;
}

return 0_u256;
}
}
}
178 changes: 178 additions & 0 deletions src/staker_log.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
use starknet::storage::MutableVecTrait;
use starknet::storage::{
Mutable, StorageAsPath, StorageBase, StoragePointerReadAccess, StoragePointerWriteAccess,
};

use starknet::storage::{Vec, VecTrait};
use starknet::storage_access::{StorePacking};
use starknet::{get_block_timestamp};

pub type StakingLog = Vec<StakingLogRecord>;

const TWO_POW_32: u64 = 0x100000000_u64;
const MASK_32_BITS: u128 = 0x100000000_u128 - 1;
const TWO_POW_160: u256 = 0x10000000000000000000000000000000000000000;
pub const MAX_FP: u128 = 0x8000000000000110000000000000000_u128;
Copy link
Member

Choose a reason for hiding this comment

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

what is this value?

Copy link
Author

@baitcode baitcode Jan 13, 2025

Choose a reason for hiding this comment

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

maximum value that can be store in felt252 is MAX_V = 0x800000000000011000000000000000000000000000000000000000000000000.
MAX_FP = MAX_V >> 128 (maximum value of highest 123 bit of felt252).


#[derive(Drop, Serde, Copy)]
pub(crate) struct StakingLogRecord {
pub(crate) timestamp: u64,
// Only 128+32=160 bits are used
pub(crate) cumulative_total_staked: u256,
pub(crate) cumulative_seconds_per_total_staked: u256,
}

#[generate_trait]
pub impl StakingLogOperations of LogOperations {
fn get_total_staked(self: @StorageBase<StakingLog>, timestamp: u64) -> Option<u128> {
Option::Some(0)
}

fn find_in_change_log(
self: @StorageBase<StakingLog>, timestamp: u64,
) -> Option<(StakingLogRecord, u64)> {
let log = self.as_path();
if log.len() == 0 {
return Option::None;
}
let mut left = 0;
let mut right = log.len() - 1;

// To avoid reading from the storage multiple times.
let mut result_ptr: Option<(StakingLogRecord, u64)> = Option::None;

while (left <= right) {
let center = (right + left) / 2;
let record_ptr = log.at(center);
let record = record_ptr.read();

if record.timestamp <= timestamp {
result_ptr = Option::Some((record, center));
left = center + 1;
} else {
right = center - 1;
};
};

if let Option::Some((result, idx)) = result_ptr {
return Option::Some((result, idx));
}

return Option::None;
}

fn log_change(self: StorageBase<Mutable<StakingLog>>, amount: u128, total_staked: u128) {
let log = self.as_path();

let block_timestamp = get_block_timestamp();

if log.len() == 0 {
log
.append()
.write(
StakingLogRecord {
timestamp: block_timestamp,
cumulative_total_staked: 0_u256,
cumulative_seconds_per_total_staked: 0_u64.into(),
},
);

return;
}

let last_record_ptr = log.at(log.len() - 1);

let mut last_record = last_record_ptr.read();

let mut record = if last_record.timestamp == block_timestamp {
// update record
last_record_ptr
} else {
// create new record
log.append()
};

// Might be zero
let seconds_diff = block_timestamp - last_record.timestamp;

let total_staked_by_elapsed_seconds = total_staked.into() * seconds_diff.into();

let staked_seconds_per_total_staked: u256 = if total_staked == 0 {
0_u64.into()
} else {
let res = u256 { low: 0, high: seconds_diff.into() } / total_staked.into();
assert(res.high < MAX_FP, 'FP_OVERFLOW');
res
};

// Add a new record.
record
.write(
StakingLogRecord {
timestamp: block_timestamp,
cumulative_total_staked: last_record.cumulative_total_staked
+ total_staked_by_elapsed_seconds,
cumulative_seconds_per_total_staked: last_record
.cumulative_seconds_per_total_staked
+ staked_seconds_per_total_staked,
},
);
}
}

//
// Storage layout for StakingLogRecord
//

pub(crate) impl StakingLogRecordStorePacking of StorePacking<StakingLogRecord, (felt252, felt252)> {
fn pack(value: StakingLogRecord) -> (felt252, felt252) {
let packed_ts_cumulative_total_staked: felt252 = pack_u64_u256_tuple(
value.timestamp, value.cumulative_total_staked,
);

let cumulative_seconds_per_total_staked: felt252 = value
.cumulative_seconds_per_total_staked
.try_into()
.unwrap();

(packed_ts_cumulative_total_staked, cumulative_seconds_per_total_staked)
}

fn unpack(value: (felt252, felt252)) -> StakingLogRecord {
let (packed_ts_cumulative_total_staked, cumulative_seconds_per_total_staked) = value;
let (timestamp, cumulative_total_staked) = unpack_u64_u256_tuple(
packed_ts_cumulative_total_staked,
);

StakingLogRecord {
timestamp: timestamp,
cumulative_total_staked: cumulative_total_staked,
cumulative_seconds_per_total_staked: cumulative_seconds_per_total_staked
.try_into()
.unwrap(),
}
}
}

pub(crate) fn pack_u64_u256_tuple(val1: u64, val2: u256) -> felt252 {
let cumulative_total_staked_high_32_bits: u128 = val2.high & MASK_32_BITS;
u256 {
high: val1.into() * TWO_POW_32.into() + cumulative_total_staked_high_32_bits.into(),
low: val2.low,
}
.try_into()
.unwrap()
}

pub(crate) fn unpack_u64_u256_tuple(value: felt252) -> (u64, u256) {
let packed_ts_total_staked_u256: u256 = value.into();

let cumulative_total_staked = u256 {
high: packed_ts_total_staked_u256.high & MASK_32_BITS, low: packed_ts_total_staked_u256.low,
};

return (
(packed_ts_total_staked_u256.high / TWO_POW_32.into()).try_into().unwrap(),
cumulative_total_staked,
);
}
33 changes: 33 additions & 0 deletions src/staker_log_test.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use crate::staker_log::{pack_u64_u256_tuple, unpack_u64_u256_tuple};

const MASK_32_BITS: u128 = 0x100000000_u128 - 1;
const MASK_64_BITS: u128 = 0x10000000000000000_u128 - 1;
const MASK_160_BITS: u256 = 0x10000000000000000000000000000000000000000 - 1;


fn assert_packs_and_unpacks(timestamp: u64, total_staked: u256) {
let packed: u256 = pack_u64_u256_tuple(timestamp, total_staked).into();

let first_160_bits: u256 = packed & MASK_160_BITS;

let shifted_160_bits_right: u128 = packed.high / (MASK_32_BITS + 1);

let last_64_bits: u64 = (shifted_160_bits_right & MASK_64_BITS).try_into().unwrap();
assert_eq!(first_160_bits, total_staked);
assert_eq!(last_64_bits, timestamp);

let (unpacked_timestamp, unpacked_cumulative_total_staked) = unpack_u64_u256_tuple(
packed.try_into().unwrap(),
);
assert_eq!(unpacked_timestamp, timestamp);
assert_eq!(unpacked_cumulative_total_staked, total_staked);
}

#[test]
fn test_staking_log_packing() {
assert_packs_and_unpacks(0_u64, 0_u256);
assert_packs_and_unpacks(10_u64, 50_u256);
assert_packs_and_unpacks(
0xffffffffffffffff_u64, 0xffffffffffffffffffffffffffffffffffffffff_u256,
)
}
Loading