-
Notifications
You must be signed in to change notification settings - Fork 71
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: struct packing #503
feat: struct packing #503
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,231 @@ | ||
use sx::types::user_address::UserAddressTrait; | ||
use clone::Clone; | ||
use serde::Serde; | ||
use starknet::ContractAddress; | ||
use starknet::storage_access::StorePacking; | ||
use starknet::Store; | ||
use sx::utils::math::pow; | ||
use traits::{Into, TryInto}; | ||
use sx::types::{FinalizationStatus, UserAddress}; | ||
use option::OptionTrait; | ||
use debug::PrintTrait; | ||
|
||
#[derive(Clone, Drop, Serde, PartialEq, starknet::Store)] | ||
const BITMASK_32: u128 = 0xffffffff; | ||
const BITMASK_64: u128 = 0xffffffffffffffff; | ||
const BITMASK_128: u128 = 0xffffffffffffffffffffffffffffffff; | ||
|
||
const BITMASK_SECOND_U32: u128 = 0xffffffff00000000; | ||
const BITMASK_THIRD_U32: u128 = 0xffffffff0000000000000000; | ||
const BITMASK_FOURTH_U32: u128 = | ||
0xff000000000000000000000000; // Only 0xff because finalization_status is an u8 | ||
|
||
const TWO_POWER_32: u128 = 0x100000000; | ||
const TWO_POWER_64: u128 = 0x10000000000000000; | ||
const TWO_POWER_96: u128 = 0x1000000000000000000000000; | ||
|
||
#[derive(Clone, Drop, Serde, PartialEq)] | ||
struct Proposal { | ||
start_timestamp: u32, | ||
min_end_timestamp: u32, | ||
max_end_timestamp: u32, | ||
finalization_status: FinalizationStatus, | ||
execution_payload_hash: felt252, | ||
execution_strategy: ContractAddress, | ||
author: UserAddress, | ||
finalization_status: FinalizationStatus, | ||
active_voting_strategies: u256 | ||
} | ||
|
||
#[derive(Drop, starknet::Store)] | ||
struct PackedProposal { | ||
timestamps_and_finalization_status: u128, // In order: start, min, max, finalization_status | ||
execution_payload_hash: felt252, | ||
execution_strategy: ContractAddress, | ||
author: UserAddress, | ||
active_voting_strategies: u256, | ||
} | ||
|
||
impl ProposalStorePacking of StorePacking<Proposal, PackedProposal> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. really like cairo's approach to packing. super clean and efficient There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah! agreed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. one thing is make sure you |
||
fn pack(value: Proposal) -> PackedProposal { | ||
let timestamps_and_finalization_status: u128 = (value.start_timestamp.into() | ||
+ value.min_end_timestamp.into() * TWO_POWER_32 | ||
+ value.max_end_timestamp.into() * TWO_POWER_64 | ||
+ value.finalization_status.into() * TWO_POWER_96); | ||
PackedProposal { | ||
timestamps_and_finalization_status, | ||
execution_payload_hash: value.execution_payload_hash, | ||
execution_strategy: value.execution_strategy, | ||
author: value.author, | ||
active_voting_strategies: value.active_voting_strategies, | ||
} | ||
} | ||
|
||
fn unpack(value: PackedProposal) -> Proposal { | ||
let start_timestamp: u32 = (value.timestamps_and_finalization_status & BITMASK_32) | ||
.try_into() | ||
.unwrap(); | ||
let min_end_timestamp: u32 = ((value.timestamps_and_finalization_status | ||
& BITMASK_SECOND_U32) | ||
/ TWO_POWER_32) | ||
.try_into() | ||
.unwrap(); | ||
let max_end_timestamp: u32 = ((value.timestamps_and_finalization_status & BITMASK_THIRD_U32) | ||
/ TWO_POWER_64) | ||
.try_into() | ||
.unwrap(); | ||
let finalization_status: u8 = ((value.timestamps_and_finalization_status | ||
& BITMASK_FOURTH_U32) | ||
/ TWO_POWER_96) | ||
.try_into() | ||
.unwrap(); | ||
|
||
let type_helper: Option<FinalizationStatus> = finalization_status | ||
.try_into(); // For some reason, type couldn't be inferred... | ||
let finalization_status: FinalizationStatus = type_helper.unwrap(); | ||
|
||
Proposal { | ||
start_timestamp, | ||
min_end_timestamp, | ||
max_end_timestamp, | ||
finalization_status, | ||
execution_payload_hash: value.execution_payload_hash, | ||
execution_strategy: value.execution_strategy, | ||
author: value.author, | ||
active_voting_strategies: value.active_voting_strategies, | ||
} | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::{Proposal, PackedProposal, ProposalStorePacking}; | ||
use super::FinalizationStatus; | ||
use starknet::storage_access::StorePacking; | ||
use starknet::contract_address_const; | ||
use sx::types::{UserAddress}; | ||
use clone::Clone; | ||
|
||
#[test] | ||
fn test_pack_zero() { | ||
let proposal = Proposal { | ||
start_timestamp: 0, | ||
min_end_timestamp: 0, | ||
max_end_timestamp: 0, | ||
finalization_status: FinalizationStatus::Pending(()), | ||
execution_payload_hash: 0, | ||
author: UserAddress::Starknet(contract_address_const::<0>()), | ||
execution_strategy: contract_address_const::<0>(), | ||
active_voting_strategies: 0_u256, | ||
}; | ||
|
||
let packed = ProposalStorePacking::pack(proposal.clone()); | ||
assert(packed.timestamps_and_finalization_status == 0, 'invalid zero packing'); | ||
let result = ProposalStorePacking::unpack(packed); | ||
assert(result == proposal, 'invalid zero unpacking'); | ||
} | ||
|
||
|
||
#[test] | ||
fn test_pack_start_timestamp() { | ||
let proposal = Proposal { | ||
start_timestamp: 42, | ||
min_end_timestamp: 0, | ||
max_end_timestamp: 0, | ||
finalization_status: FinalizationStatus::Pending(()), | ||
execution_payload_hash: 0, | ||
author: UserAddress::Starknet(contract_address_const::<0>()), | ||
execution_strategy: contract_address_const::<0>(), | ||
active_voting_strategies: 0_u256, | ||
}; | ||
|
||
let packed = ProposalStorePacking::pack(proposal.clone()); | ||
assert(packed.timestamps_and_finalization_status == 42, 'invalid start packing'); | ||
let result = ProposalStorePacking::unpack(packed); | ||
assert(result == proposal, 'invalid start unpacking'); | ||
} | ||
|
||
#[test] | ||
fn test_pack_min_timestamp() { | ||
let proposal = Proposal { | ||
start_timestamp: 0, | ||
min_end_timestamp: 42, | ||
max_end_timestamp: 0, | ||
finalization_status: FinalizationStatus::Pending(()), | ||
execution_payload_hash: 0, | ||
author: UserAddress::Starknet(contract_address_const::<0>()), | ||
execution_strategy: contract_address_const::<0>(), | ||
active_voting_strategies: 0_u256, | ||
}; | ||
|
||
let packed = ProposalStorePacking::pack(proposal.clone()); | ||
assert(packed.timestamps_and_finalization_status == 0x2a00000000, 'invalid min packing'); | ||
let result = ProposalStorePacking::unpack(packed); | ||
assert(result == proposal, 'invalid min unpacking'); | ||
} | ||
|
||
|
||
#[test] | ||
fn test_pack_max_timestamp() { | ||
let proposal = Proposal { | ||
start_timestamp: 0, | ||
min_end_timestamp: 0, | ||
max_end_timestamp: 42, | ||
finalization_status: FinalizationStatus::Pending(()), | ||
execution_payload_hash: 0, | ||
author: UserAddress::Starknet(contract_address_const::<0>()), | ||
execution_strategy: contract_address_const::<0>(), | ||
active_voting_strategies: 0_u256, | ||
}; | ||
|
||
let packed = ProposalStorePacking::pack(proposal.clone()); | ||
assert( | ||
packed.timestamps_and_finalization_status == 0x2a0000000000000000, 'invalid max packing' | ||
); | ||
let result = ProposalStorePacking::unpack(packed); | ||
assert(result == proposal, 'invalid max unpacking'); | ||
} | ||
|
||
#[test] | ||
fn test_pack_finalization_status() { | ||
let proposal = Proposal { | ||
start_timestamp: 0, | ||
min_end_timestamp: 0, | ||
max_end_timestamp: 0, | ||
finalization_status: FinalizationStatus::Executed(()), | ||
execution_payload_hash: 0, | ||
author: UserAddress::Starknet(contract_address_const::<0>()), | ||
execution_strategy: contract_address_const::<0>(), | ||
active_voting_strategies: 0_u256, | ||
}; | ||
|
||
let packed = ProposalStorePacking::pack(proposal.clone()); | ||
assert( | ||
packed.timestamps_and_finalization_status == 0x01000000000000000000000000, | ||
'invalid status packing' | ||
); | ||
let result = ProposalStorePacking::unpack(packed); | ||
assert(result == proposal, 'invalid status unpacking'); | ||
} | ||
|
||
#[test] | ||
fn test_pack_full() { | ||
let proposal = Proposal { | ||
start_timestamp: 0xffffffff, | ||
min_end_timestamp: 0xffffffff, | ||
max_end_timestamp: 0xffffffff, | ||
finalization_status: FinalizationStatus::Cancelled(()), | ||
execution_payload_hash: 0, | ||
author: UserAddress::Starknet(contract_address_const::<0>()), | ||
execution_strategy: contract_address_const::<0>(), | ||
active_voting_strategies: 0_u256, | ||
}; | ||
|
||
let packed = ProposalStorePacking::pack(proposal.clone()); | ||
assert( | ||
packed.timestamps_and_finalization_status == 0x02ffffffffffffffffffffffff, | ||
'invalid full packing' | ||
); | ||
let result = ProposalStorePacking::unpack(packed); | ||
assert(result == proposal, 'invalid full unpacking'); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Annoying that the
UserAddress
adds storage overhead. one solution could be to store afelt252
hash of it that we then compute when neeeded. probably unnecessary complexity thoughThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, most of the time the
UserAddress
will be ethereum / starknet and will felt in a single felt so the storage will not move (will stay 0) so i don't think it will cost that much?