feat(tokenomics): auto-calculate total_supply and burn registry token#28
feat(tokenomics): auto-calculate total_supply and burn registry token#28
Conversation
Remove `total_supply` from JSON config - now auto-calculated from LP + distributions + premints. Burn the registry token after it's returned so circulating supply matches exactly what users specified. Changes: - Script auto-calculates total_supply from component amounts - Factory validation no longer includes ERC20_UNIT in total_needed - StreamToken burns the returned registry token after registration - Mock registries updated to simulate Ekubo's token return behavior - Tests updated for new supply calculation and burn behavior Token flow after changes: 1. Mint 1 token → Registry (for verification) 2. Registry returns 1 token → StreamToken 3. Burn 1 token (new) 4. Mint premints → Recipients 5. Mint remaining → Factory (= LP + distributions) Result: Circulating supply = exactly LP + distributions + premints Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @loothero, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the tokenomics of StreamToken by automating the calculation of the total supply and ensuring precise circulating supply. It achieves this by dynamically calculating Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a great improvement by auto-calculating the total_supply and burning the registry token to ensure supply accuracy. The changes across the Cairo contracts, tests, and the deployment script are well-aligned with this goal. The logic is sound and the test coverage for the new behavior is good. I've identified a bug in the create_stream_token.sh script that occurs when handling empty arrays, and some inconsistencies in the newly added example JSON files. My review includes suggestions to fix these issues.
scripts/create_stream_token.sh
Outdated
| # Calculate distribution total | ||
| DISTRIBUTION_TOTAL=0 | ||
| for i in $(seq 0 $((ORDER_COUNT - 1))); do | ||
| local_amount=$(echo "$CONFIG" | jq -r ".distribution_orders[$i].amount") | ||
| DISTRIBUTION_TOTAL=$((DISTRIBUTION_TOTAL + local_amount)) | ||
| done | ||
|
|
||
| # Calculate premint total | ||
| PREMINT_TOTAL=0 | ||
| for i in $(seq 0 $((PREMINT_COUNT - 1))); do | ||
| local_amount=$(echo "$CONFIG" | jq -r ".premint_allocations[$i].amount") | ||
| PREMINT_TOTAL=$((PREMINT_TOTAL + local_amount)) | ||
| done |
There was a problem hiding this comment.
The current implementation for calculating DISTRIBUTION_TOTAL and PREMINT_TOTAL has a bug when ORDER_COUNT or PREMINT_COUNT is zero. The seq 0 $((COUNT - 1)) command in the for loop will execute with invalid indices if the count is 0, leading to an error when trying to access an array element in jq.
I suggest using jq to perform the summation directly. This is more robust, efficient, and gracefully handles empty arrays.
| # Calculate distribution total | |
| DISTRIBUTION_TOTAL=0 | |
| for i in $(seq 0 $((ORDER_COUNT - 1))); do | |
| local_amount=$(echo "$CONFIG" | jq -r ".distribution_orders[$i].amount") | |
| DISTRIBUTION_TOTAL=$((DISTRIBUTION_TOTAL + local_amount)) | |
| done | |
| # Calculate premint total | |
| PREMINT_TOTAL=0 | |
| for i in $(seq 0 $((PREMINT_COUNT - 1))); do | |
| local_amount=$(echo "$CONFIG" | jq -r ".premint_allocations[$i].amount") | |
| PREMINT_TOTAL=$((PREMINT_TOTAL + local_amount)) | |
| done | |
| # Calculate distribution total | |
| DISTRIBUTION_TOTAL=$(echo "$CONFIG" | jq -r '[.distribution_orders[].amount | tonumber] | add // 0') | |
| # Calculate premint total | |
| PREMINT_TOTAL=$(echo "$CONFIG" | jq -r '[.premint_allocations[].amount | tonumber] | add // 0') |
| "symbol": "TEXLA", | ||
| "total_supply": "305003", |
There was a problem hiding this comment.
The total_supply field is included in this example file, but it's now auto-calculated by the create_stream_token.sh script and this field is ignored. This can be misleading for users copying this example.
To maintain consistency with the new script logic, this field should be removed.
"symbol": "TEXLA"| "symbol": "TPOISONA", | ||
| "total_supply": "6100003", |
There was a problem hiding this comment.
Similar to the other new example files, this file includes a total_supply field which is now auto-calculated by the script. It should be removed to avoid confusion.
"symbol": "TPOISONA"| "symbol": "TREVIVEA", | ||
| "total_supply": "6100003", |
There was a problem hiding this comment.
Similar to the other new example files, this file includes a total_supply field which is now auto-calculated by the script. It should be removed to avoid confusion.
"symbol": "TREVIVEA"There was a problem hiding this comment.
Pull request overview
This pull request implements automatic calculation of total_supply from component amounts and introduces burning of the registry token to ensure circulating supply exactly matches user-specified amounts. The changes eliminate the need for users to manually calculate total supply while improving tokenomics accuracy.
Changes:
- Auto-calculates
total_supplyin the deployment script from LP + distributions + premints - Removes the
ERC20_UNITregistry token from supply calculations in the factory - Burns the registry token after it's returned from registration to match circulating supply to user intent
- Updates mock registries to simulate Ekubo's behavior of returning the verification token
- Updates tests and documentation to reflect the new token flow
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/create_stream_token.sh | Removes bc dependency; adds auto-calculation of total_supply from LP, distribution, and premint amounts; removes total_supply validation from required fields |
| scripts/examples/token_config.example.json | New example config with updated comment explaining auto-calculation |
| scripts/examples/summit_test_attack_potions.json | Example config with updated comment and no total_supply field |
| scripts/examples/summit_test_extralife_potions.json | Example config with outdated total_supply field and comment that need removal/updating |
| scripts/examples/summit_test_revive_potions.json | Example config with outdated total_supply field and comment that need removal/updating |
| scripts/examples/summit_test_poison_potions.json | Example config with outdated total_supply field and comment that need removal/updating |
| packages/tokenomics/src/factory/stream_token_factory.cairo | Removes ERC20_UNIT from total_needed calculation; updates comments to reflect registry token is burned |
| packages/tokenomics/src/tests/test_factory.cairo | Updates test comments to reflect new supply calculations without ERC20_UNIT |
| packages/tokenomics/src/tests/mocks/mock_registry.cairo | Implements token return behavior to simulate Ekubo's registry |
| packages/presets/src/stream_token.cairo | Adds burn call after register_token; updates remaining_supply calculation to exclude ERC20_UNIT |
| packages/presets/src/tests/test_stream_token.cairo | Updates tests to verify registry has 0 balance after burn; adds new test for contract balance |
| packages/presets/src/tests/mocks/mock_registry.cairo | Implements token return behavior to simulate Ekubo's registry |
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens).", | ||
|
|
||
| "name": "TREVIVEA", | ||
| "symbol": "TREVIVEA", | ||
| "total_supply": "6100003", |
There was a problem hiding this comment.
The total_supply field should be removed from this file since it's now auto-calculated by the script. The comment on line 2 should also be updated to match the new behavior, like in token_config.example.json which states "total_supply is auto-calculated from LP + distributions + premints." Additionally, the calculated value (6100003) appears to be incorrect. Based on the config:
- LP: 100000
- Distribution: 6000000
- Premint: 1
- Expected total: 6100001 (not 6100003)
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens).", | |
| "name": "TREVIVEA", | |
| "symbol": "TREVIVEA", | |
| "total_supply": "6100003", | |
| "_comment": "All token amounts assume 18 decimals. total_supply is auto-calculated from LP + distributions + premints. Use human-readable values (e.g., '1000000' for 1M tokens).", | |
| "name": "TREVIVEA", | |
| "symbol": "TREVIVEA", |
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens).", | ||
|
|
||
| "name": "TPOISONA", | ||
| "symbol": "TPOISONA", | ||
| "total_supply": "6100003", |
There was a problem hiding this comment.
The total_supply field should be removed from this file since it's now auto-calculated by the script. The comment on line 2 should also be updated to match the new behavior, like in token_config.example.json which states "total_supply is auto-calculated from LP + distributions + premints." Additionally, the calculated value (6100003) appears to be incorrect. Based on the config:
- LP: 100000
- Distribution: 6000000
- Premint: 1
- Expected total: 6100001 (not 6100003)
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens).", | |
| "name": "TPOISONA", | |
| "symbol": "TPOISONA", | |
| "total_supply": "6100003", | |
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens). total_supply is auto-calculated from LP + distributions + premints.", | |
| "name": "TPOISONA", | |
| "symbol": "TPOISONA", |
scripts/create_stream_token.sh
Outdated
| for i in $(seq 0 $((ORDER_COUNT - 1))); do | ||
| local_amount=$(echo "$CONFIG" | jq -r ".distribution_orders[$i].amount") | ||
| DISTRIBUTION_TOTAL=$((DISTRIBUTION_TOTAL + local_amount)) | ||
| done |
There was a problem hiding this comment.
The validation that checks ORDER_COUNT is greater than zero happens after the loop that uses ORDER_COUNT. While this doesn't cause an error (bash's seq handles negative ranges gracefully by producing no output), it would be better practice to validate ORDER_COUNT before using it in the loop. Consider moving the validation from lines 563-566 to before line 541.
| @@ -0,0 +1,33 @@ | |||
| { | |||
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens).", | |||
There was a problem hiding this comment.
The comment should be updated to indicate that total_supply is now auto-calculated, matching the updated comment in token_config.example.json and summit_test_attack_potions.json which states: "total_supply is auto-calculated from LP + distributions + premints."
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens).", | |
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens). total_supply is auto-calculated from LP + distributions + premints.", |
| @@ -0,0 +1,33 @@ | |||
| { | |||
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens).", | |||
There was a problem hiding this comment.
The comment should be updated to indicate that total_supply is now auto-calculated, matching the updated comment in token_config.example.json and summit_test_attack_potions.json which states: "total_supply is auto-calculated from LP + distributions + premints."
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens).", | |
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens). total_supply is auto-calculated from LP + distributions + premints.", |
| @@ -0,0 +1,33 @@ | |||
| { | |||
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens).", | |||
There was a problem hiding this comment.
The comment should be updated to indicate that total_supply is now auto-calculated, matching the updated comment in token_config.example.json and summit_test_attack_potions.json which states: "total_supply is auto-calculated from LP + distributions + premints."
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens).", | |
| "_comment": "All token amounts assume 18 decimals. Use human-readable values (e.g., '1000000' for 1M tokens). total_supply is auto-calculated from LP + distributions + premints.", |
|
|
||
| "name": "TEXLA", | ||
| "symbol": "TEXLA", | ||
| "total_supply": "305003", |
There was a problem hiding this comment.
The total_supply field should be removed from this file since it's now auto-calculated by the script. The comment on line 2 already indicates this, but the field is still present on line 6. Additionally, the calculated value (305003) appears to be incorrect. Based on the config:
- LP: 5000
- Distribution: 300000
- Premint: 1
- Expected total: 305001 (not 305003)
| "total_supply": "305003", |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8028c380bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
scripts/create_stream_token.sh
Outdated
| # Calculate distribution total | ||
| DISTRIBUTION_TOTAL=0 | ||
| for i in $(seq 0 $((ORDER_COUNT - 1))); do | ||
| local_amount=$(echo "$CONFIG" | jq -r ".distribution_orders[$i].amount") | ||
| DISTRIBUTION_TOTAL=$((DISTRIBUTION_TOTAL + local_amount)) | ||
| done | ||
|
|
||
| # Calculate premint total | ||
| PREMINT_TOTAL=0 | ||
| for i in $(seq 0 $((PREMINT_COUNT - 1))); do | ||
| local_amount=$(echo "$CONFIG" | jq -r ".premint_allocations[$i].amount") | ||
| PREMINT_TOTAL=$((PREMINT_TOTAL + local_amount)) | ||
| done | ||
|
|
||
| # Auto-calculate total_supply: LP + distributions + premints | ||
| # (registry token is minted and burned during deployment, so no extra needed) | ||
| TOTAL_SUPPLY=$((STREAM_TOKEN_AMOUNT + DISTRIBUTION_TOTAL + PREMINT_TOTAL)) | ||
| TOTAL_SUPPLY_WEI=$(to_wei "$TOTAL_SUPPLY") |
There was a problem hiding this comment.
The new auto-calculation uses Bash arithmetic expansion ($((...))) to sum LP, distribution, and premint amounts. Bash arithmetic is limited to signed 64-bit, so larger token supplies (which are valid for u128 and previously supported when total_supply was user-provided) will overflow or error, yielding an incorrect TOTAL_SUPPLY and calldata. This can cause deployments to revert with STREAM_SUPPLY_TOO_LOW or mint the wrong supply. Consider using a big-integer safe path (e.g., bc, python, or jq arithmetic) for these sums.
Useful? React with 👍 / 👎.
Address PR review feedback: - Replace loop-based distribution/premint totals with jq one-liners that handle empty arrays gracefully with `// 0` default - Remove summit_test_*.json from tracking (add to .gitignore) - Keep only token_config.example.json as the committed example Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @loothero's task —— View job Code Review - PR #28: feat(tokenomics): auto-calculate total_supply and burn registry tokenI've completed a comprehensive code review focusing on code quality, potential bugs, performance, security, and test coverage. Here are my findings: ✅ Strengths
🚨 Critical Issues1. Shell Script Arithmetic Overflow Risk (High Severity)File: TOTAL_SUPPLY=$((STREAM_TOKEN_AMOUNT + DISTRIBUTION_TOTAL + PREMINT_TOTAL))Issue: Bash arithmetic is limited to 64-bit signed integers. For token supplies exceeding ~9×10^18, this will overflow and produce incorrect results, leading to deployment failures or wrong token supplies. Recommendation: Use TOTAL_SUPPLY=$(echo "$CONFIG" | jq -r "
(.liquidity_config.stream_token_amount | tonumber) +
([.distribution_orders[].amount | tonumber] | add // 0) +
([.premint_allocations[].amount | tonumber] | add // 0)
")2. Array Loop Edge Case (Medium Severity)File: 🔧 Code Quality Issues3. Inconsistent Example FilesSeveral example JSON files contain obsolete
4. Minor Cairo Code ReviewThe Cairo contract changes are well-implemented and secure:
📈 Performance & Security AssessmentPerformance: ✅ Excellent
Security: ✅ Very Good
🧪 Test Coverage AnalysisTest Quality: ✅ Excellent
Missing: The script arithmetic overflow isn't tested, but this is acceptable for integration scripts. 🎯 Specific Recommendations
📋 Final AssessmentThis is a well-executed feature that significantly improves the user experience and token precision. The implementation follows Cairo best practices and maintains security. The only blocking issue is the potential arithmetic overflow in the shell script. Verdict: ✅ Approve after fixing arithmetic overflow The core functionality is sound, tests are comprehensive, and the feature delivers clear value. Once the shell script arithmetic is made overflow-safe, this is ready to merge. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
Remove
total_supplyfrom JSON config - now auto-calculated from LP + distributions + premints. Burn the registry token after it's returned so circulating supply matches exactly what users specified.Changes
create_stream_token.sh): Auto-calculatestotal_supplyfrom component amounts (LP + distributions + premints), removes it from required fieldsERC20_UNITfromtotal_neededvalidation - supply only needs to cover LP + distributions + premintsregister_token()callToken Flow After Changes
Result: Circulating supply = exactly LP + distributions + premints (what user specified)
Testing
scarb buildpassesscarb fmt --checkpassessnforge test -p game_components_tokenomics- 218 tests passsnforge test -p game_components_presets- 180 tests pass@codex review
🤖 Generated with Claude Code