-
Notifications
You must be signed in to change notification settings - Fork 6
feat(distribution): exact O(1) payouts in token units, and fix the O(n^2) basis-point path #120
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
base: main
Are you sure you want to change the base?
Changes from 10 commits
61e6278
338a490
cc04cfe
93c027f
d5e8e4f
54b78be
3ffbe57
d182c69
6d4389d
5b7ae96
eec6466
afcb367
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 |
|---|---|---|
|
|
@@ -16,8 +16,16 @@ pub trait PrizeStoreTrait<T> { | |
| /// are routed via the component's `resolve_prize` before this is | ||
| /// called and never reach the store bridge. | ||
| fn get_token_record(self: @T, prize_id: u64) -> PrizeRecord; | ||
| /// Get custom shares for a prize (reconstructs from packed storage) | ||
| /// Get custom shares for a prize (reconstructs from packed storage). | ||
| /// | ||
| /// O(count/15) storage reads. Only view surfaces need the whole curve — | ||
| /// a claim wants exactly one share, so it calls `get_custom_share_at`. | ||
| fn get_custom_shares(self: @T, prize_id: u64) -> Array<u16>; | ||
| /// One share by 1-indexed position, without rebuilding the array. | ||
| /// | ||
| /// Mirrors `EntryFeeStoreTrait::get_custom_share_at`. Shares are packed | ||
| /// 15 to a felt, so this is a single storage read at any position. | ||
| fn get_custom_share_at(self: @T, prize_id: u64, position: u32) -> u16; | ||
| /// Store a token prize. Takes the host-assigned context + sponsor | ||
| /// alongside the variant payload; converts to StoredPrize for | ||
| /// storage. | ||
|
|
@@ -67,10 +75,19 @@ pub impl PrizeStoreImpl<T, +Store<T>, +Drop<T>> of PrizeStoreTrait<T> { | |
| Option::Some(dist) => { | ||
| match dist { | ||
| game_components_utilities::distribution::structs::Distribution::Custom(_) => { | ||
| let shares = self.get_custom_shares(prize_id); | ||
| // Return the shape with an empty span | ||
| // rather than rebuilding the curve. | ||
| // Loading it is O(count/15) storage | ||
| // reads on a path that runs on every | ||
| // claim, and a claim needs exactly one | ||
| // share — `get_custom_share_at`. | ||
| // View surfaces call | ||
| // `get_custom_shares` explicitly. | ||
| // Mirrors the entry-fee store, which | ||
| // has always done this. | ||
| Option::Some( | ||
| game_components_utilities::distribution::structs::Distribution::Custom( | ||
| shares.span(), | ||
| array![].span(), | ||
| ), | ||
| ) | ||
| }, | ||
|
|
@@ -99,6 +116,13 @@ pub impl PrizeStoreImpl<T, +Store<T>, +Drop<T>> of PrizeStoreTrait<T> { | |
| record | ||
| } | ||
|
|
||
| fn get_custom_share_at(self: @T, prize_id: u64, position: u32) -> u16 { | ||
| let index: u32 = position - 1; | ||
| let slot_index: u8 = (index / CUSTOM_SHARES_PER_SLOT.into()).try_into().unwrap(); | ||
| let index_in_slot: u8 = (index % CUSTOM_SHARES_PER_SLOT.into()).try_into().unwrap(); | ||
| Store::get_custom_shares_packed(self, prize_id, slot_index).get_share(index_in_slot) | ||
| } | ||
|
Comment on lines
+119
to
+124
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. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 8 \
'get_custom_share_at|_get_custom_share_at|PrizeType::Distributed|set_prize_claimed|payout' \
packages --glob '*.cairo'Repository: Provable-Games/game-components Length of output: 50387 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Files:"
fd -a 'prize_store.cail|test_prize_store\.cairo|prize_component\.cairo|prize.*\.cairo' packages/metagame/src/prize packages/metagame/src 2>/dev/null | sed 's#^\./##'
echo
echo "prize_store outline:"
ast-grep outline packages/metagame/src/prize/prize_store.cairo --view expanded || true
echo
echo "Relevant store implementation:"
sed -n '1,190p' packages/metagame/src/prize/prize_store.cairo | cat -n
echo
echo "Relevant tests around custom share:"
sed -n '330,420p' packages/metagame/src/prize/tests/test_prize_store.cairo | cat -n
echo
echo "Claim/payout references in prize files only:"
rg -n -C 10 'set_prize_claimed|payout_prize|claim|Custom|get_custom_share_at|get_custom_shares|positions' packages/metagame/src/prize --glob '*.cairo'Repository: Provable-Games/game-components Length of output: 50387 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Packed share behavior probe:"
python3 - <<'PY'
# Mirrors the documented packed-share arithmetic in packages/metagame/src/prize/prize_store.cairo.
CUSTOM_SHARES_PER_SLOT = 15
positions = [0, 1, 15, 16, 17, 18, 384, 385]
count = 17
for position in positions:
index = position - 1
if index < 0:
print(f"position {position}: underflows u32; no direct slot conversion")
continue
slot_index_u8 = (index // CUSTOM_SHARES_PER_SLOT)
index_in_slot_u8 = (index % CUSTOM_SHARES_PER_SLOT)
readable = 0 <= slot_index_u8 <= 255 and 0 <= index_in_slot_u8 <= 14
status = "readable slot if slot exists" if readable else f"u8 conversion failure ({slot_index_u8=})"
print(f"position {position}: zindex={index}, slot={slot_index_u8}, offset={index_in_slot_u8}, status={status}, in_stored_range={0 < position <= count}")
PY
echo
echo "Error constants and custom share tests:"
rg -n -C 4 'Prize:|Custom share|get_custom_share_at|position + 1|position - 1|CUSTOM_SHARE|SHARES_PER_SLOT' packages/metagame/src/prize --glob '*.cairo'
echo
echo "Read claim resolution in prize component around custom shares:"
rg -n -C 12 'get_custom_share_at|get_payout_position|set_prize_claimed|claim|Distributed|pout' packages/metagame/src/prize/prize_component.cairo | sed -n '1,260p'Repository: Provable-Games/game-components Length of output: 50386 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Determine claim-related code shape around get_custom_share_at callers"
rg -n 'get_custom_share_at|_get_custom_share_at|calculate_share|payout_prize|set_prize_claimed|get_claims|Claimable|claim_position|position' packages/metagame/src/prize/prize_component.cairo
echo
sed -n '1,340p' packages/metagame/src/prize/prize_component.cairo | cat -n
echo
sed -n '340,660p' packages/metagame/src/prize/prize_component.cairo | cat -n
echo
echo "Interface and claim type definitions in package interfaces"
fd -a 'prize.*\.cairo|claim.*\.cairo' packages packages/metagame 2>/dev/null | sed -n '1,200p'
rg -n 'iprize|IPRIZE|claim_prize|claims|ClaimType|position' packages --glob '*.cairo' | sed -n '1,260p'Repository: Provable-Games/game-components Length of output: 50386 Reject out-of-range custom shares before packed lookup.
📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| fn get_custom_shares(self: @T, prize_id: u64) -> Array<u16> { | ||
| let count = Store::get_custom_shares_count(self, prize_id); | ||
| let mut shares = ArrayTrait::new(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ use starknet::storage_access::StorePacking; | |
| mod nz128 { | ||
| pub const TWO_POW_8: NonZero<u128> = 0x100; | ||
| pub const TWO_POW_16: NonZero<u128> = 0x10000; | ||
| pub const TWO_POW_32: NonZero<u128> = 0x100000000; | ||
| } | ||
|
|
||
| // Payout type constants for storage | ||
|
|
@@ -24,16 +25,21 @@ pub const PAYOUT_TYPE_LINEAR: u8 = 1; | |
| pub const PAYOUT_TYPE_EXPONENTIAL: u8 = 2; | ||
| pub const PAYOUT_TYPE_UNIFORM: u8 = 3; | ||
| pub const PAYOUT_TYPE_CUSTOM: u8 = 4; | ||
| pub const PAYOUT_TYPE_GEOMETRIC: u8 = 5; | ||
|
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Exercise the new payout type in the packing tests.
🤖 Prompt for AI Agents |
||
| pub const PAYOUT_TYPE_TIERED: u8 = 6; | ||
|
|
||
| /// Internal packed representation for ERC20 data storage | ||
| /// Layout: [amount: 128 bits][payout_type: 8 bits][param: 16 bits][count: 32 bits] = 184 bits | ||
| /// Layout: [amount: 128 bits][payout_type: 8][param: 16][count: 32][param2: 16][param3: 16] | ||
| /// = 216 bits. param2/param3 carry Tiered's head_count and head_share_bps; 0 otherwise. | ||
| /// This is used internally by StorePacking and not exposed in the API | ||
| #[derive(Copy, Drop)] | ||
| struct PackedERC20Data { | ||
| amount: u128, | ||
| payout_type: u8, | ||
| param: u16, | ||
| count: u32, | ||
| param2: u16, | ||
| param3: u16, | ||
| } | ||
|
|
||
| /// u128-aligned StorePacking for PackedERC20Data. | ||
|
|
@@ -53,7 +59,9 @@ impl PackedERC20DataPacking of StorePacking<PackedERC20Data, felt252> { | |
|
|
||
| let high: u128 = value.payout_type.into() | ||
| + value.param.into() * 0x100_u128 // shift 8 | ||
| + value.count.into() * 0x1000000_u128; // shift 24 | ||
| + value.count.into() * 0x1000000_u128 // shift 24 | ||
| + value.param2.into() * 0x100000000000000_u128 // shift 56 | ||
| + value.param3.into() * 0x1000000000000000000_u128; // shift 72 | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| let packed = u256 { low, high }; | ||
| packed.try_into().unwrap() | ||
|
|
@@ -66,13 +74,17 @@ impl PackedERC20DataPacking of StorePacking<PackedERC20Data, felt252> { | |
|
|
||
| let high = packed.high; | ||
| let (hi, payout_type) = DivRem::div_rem(high, nz128::TWO_POW_8); | ||
| let (count, param) = DivRem::div_rem(hi, nz128::TWO_POW_16); | ||
| let (hi2, param) = DivRem::div_rem(hi, nz128::TWO_POW_16); | ||
| let (hi3, count) = DivRem::div_rem(hi2, nz128::TWO_POW_32); | ||
| let (param3, param2) = DivRem::div_rem(hi3, nz128::TWO_POW_16); | ||
|
|
||
| PackedERC20Data { | ||
| amount, | ||
| payout_type: payout_type.try_into().unwrap(), | ||
| param: param.try_into().unwrap(), | ||
| count: count.try_into().unwrap(), | ||
| param2: param2.try_into().unwrap(), | ||
| param3: param3.try_into().unwrap(), | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -115,30 +127,39 @@ fn pack_token_type(token_type: TokenTypeData) -> PackedTokenTypeData { | |
| match token_type { | ||
| TokenTypeData::erc20(erc20_data) => { | ||
| // Convert ERC20Data to packed format | ||
| let (payout_type, param) = match erc20_data.distribution { | ||
| Option::None => (PAYOUT_TYPE_POSITION, 0_u16), | ||
| let (payout_type, param, param2, param3) = match erc20_data.distribution { | ||
| Option::None => (PAYOUT_TYPE_POSITION, 0_u16, 0_u16, 0_u16), | ||
| Option::Some(dist) => { | ||
| match dist { | ||
| game_components_utilities::distribution::structs::Distribution::Linear(w) => ( | ||
| PAYOUT_TYPE_LINEAR, w, | ||
| PAYOUT_TYPE_LINEAR, w, 0_u16, 0_u16, | ||
| ), | ||
| game_components_utilities::distribution::structs::Distribution::Exponential(w) => ( | ||
| PAYOUT_TYPE_EXPONENTIAL, w, | ||
| PAYOUT_TYPE_EXPONENTIAL, w, 0_u16, 0_u16, | ||
| ), | ||
| game_components_utilities::distribution::structs::Distribution::Uniform => ( | ||
| PAYOUT_TYPE_UNIFORM, 0_u16, | ||
| PAYOUT_TYPE_UNIFORM, 0_u16, 0_u16, 0_u16, | ||
| ), | ||
| game_components_utilities::distribution::structs::Distribution::Custom(_) => ( | ||
| PAYOUT_TYPE_CUSTOM, 0_u16, | ||
| PAYOUT_TYPE_CUSTOM, 0_u16, 0_u16, 0_u16, | ||
| ), | ||
| game_components_utilities::distribution::structs::Distribution::Geometric(( | ||
| a, b, | ||
| )) => (PAYOUT_TYPE_GEOMETRIC, a * 256 + b, 0_u16, 0_u16), | ||
| game_components_utilities::distribution::structs::Distribution::Tiered(cfg) => { | ||
| let (a, b) = cfg.head_ratio; | ||
| (PAYOUT_TYPE_TIERED, a * 256 + b, cfg.head_count, cfg.head_share_bps) | ||
| }, | ||
| } | ||
| }, | ||
| }; | ||
| let count = match erc20_data.distribution_count { | ||
| Option::Some(c) => c, | ||
| Option::None => 0_u32, | ||
| }; | ||
| let packed = PackedERC20Data { amount: erc20_data.amount, payout_type, param, count }; | ||
| let packed = PackedERC20Data { | ||
| amount: erc20_data.amount, payout_type, param, count, param2, param3, | ||
| }; | ||
| PackedTokenTypeData::erc20(PackedERC20DataPacking::pack(packed)) | ||
| }, | ||
| TokenTypeData::erc721(erc721_data) => PackedTokenTypeData::erc721(erc721_data), | ||
|
|
@@ -170,6 +191,22 @@ fn unpack_token_type(packed_token_type: PackedTokenTypeData) -> TokenTypeData { | |
| Option::Some( | ||
| game_components_utilities::distribution::structs::Distribution::Uniform, | ||
| ) | ||
| } else if packed.payout_type == PAYOUT_TYPE_GEOMETRIC { | ||
| Option::Some( | ||
| game_components_utilities::distribution::structs::Distribution::Geometric( | ||
| (packed.param / 256, packed.param % 256), | ||
| ), | ||
| ) | ||
| } else if packed.payout_type == PAYOUT_TYPE_TIERED { | ||
| Option::Some( | ||
| game_components_utilities::distribution::structs::Distribution::Tiered( | ||
| game_components_utilities::distribution::structs::TieredConfig { | ||
| head_ratio: (packed.param / 256, packed.param % 256), | ||
| head_count: packed.param2, | ||
| head_share_bps: packed.param3, | ||
| }, | ||
| ), | ||
| ) | ||
| } else { | ||
| Option::Some( | ||
| game_components_utilities::distribution::structs::Distribution::Custom( | ||
|
|
@@ -240,7 +277,7 @@ mod packed_erc20_data_tests { | |
| fn build_packed_erc20( | ||
| amount: u128, payout_type: u8, param: u16, count: u32, | ||
| ) -> PackedERC20Data { | ||
| PackedERC20Data { amount, payout_type, param, count } | ||
| PackedERC20Data { amount, payout_type, param, count, param2: 0, param3: 0 } | ||
| } | ||
|
|
||
| fn assert_roundtrip(data: PackedERC20Data) { | ||
|
|
@@ -250,6 +287,21 @@ mod packed_erc20_data_tests { | |
| assert!(unpacked.payout_type == data.payout_type, "payout_type mismatch"); | ||
| assert!(unpacked.param == data.param, "param mismatch"); | ||
| assert!(unpacked.count == data.count, "count mismatch"); | ||
| assert!(unpacked.param2 == data.param2, "param2 mismatch"); | ||
| assert!(unpacked.param3 == data.param3, "param3 mismatch"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_tiered_params_roundtrip_in_the_widened_slots() { | ||
| let data = PackedERC20Data { | ||
| amount: 0xffffffffffffffffffffffffffffffff, // u128::MAX alongside full params | ||
| payout_type: 6, | ||
| param: 10 * 256 + 7, | ||
| count: 10000, | ||
| param2: 39, | ||
| param3: 8000, | ||
| }; | ||
| assert_roundtrip(data); | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------------- | ||
|
|
||
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.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: Provable-Games/game-components
Length of output: 50387
🏁 Script executed:
Repository: Provable-Games/game-components
Length of output: 23459
🏁 Script executed:
Repository: Provable-Games/game-components
Length of output: 13998
Preserve the full custom share span on
IPrize.get_prize.PrizeComponentImpl::get_prizereturns the result ofget_token_record, which converts every storedDistribution::CustomtoCustom(array![].span()). The publicIPrizeview therefore exposes an incompletePrizeRecord. Keepget_prizereturning the reconstructed custom shares, and have claim paths call the indexed share helper instead.📍 Affects 2 files
packages/metagame/src/prize/prize_store.cairo#L78-L90(this comment)packages/metagame/src/prize/tests/test_prize_store.cairo#L401-L411🤖 Prompt for AI Agents