Cairo: Add ERC20FlashMint extension#801
Open
immrsd wants to merge 9 commits into
Open
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds ERC20 Flash Mint extension support to Cairo Alpha contracts, including configuration types, core builder logic with storage management, code generation wiring, UI controls, AI function definitions, and comprehensive test coverage. ChangesERC20 Flash Mint Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/core/cairo_alpha/src/tests/with_components_off/erc20/erc20.test.ts (1)
125-173: ⚡ Quick winAdd regression cases for numeric-but-unrepresentable inputs.
These negative tests cover malformed strings, but they still miss the cases most likely to regress here: a
flashMintMaxAmountaboveu256::MAX, and a percent with so many fractional digits that the generated denominator no longer fits the emitted Cairo literal. Adding those would lock down the parser changes on the failure paths that currently escape local validation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/cairo_alpha/src/tests/with_components_off/erc20/erc20.test.ts` around lines 125 - 173, Add two regression tests in the same suite that call buildERC20 to assert validation failures for numeric-but-unrepresentable inputs: one that sets flashMintMaxAmount to a numeric string greater than u256::MAX (expecting the same OptionsError.messages.flashMintMaxAmount = 'Must be "max" or a non-negative number'), and another that sets flashMintFeeMode: 'percent' with flashMintFeePercent as a highly precise fractional string (many fractional digits) that would overflow the denominator used to emit a Cairo literal (expecting OptionsError.messages.flashMintFeePercent = 'Must be a number between 0 and 100'); mirror the existing test structure and assertions around buildERC20 to ensure these failure paths are covered.packages/ui/src/cairo_alpha/ERC20Controls.svelte (1)
38-42: 💤 Low valueConsider guarding against undefined
flashMintMaxAmountduring initialization.If
opts.flashMintMaxAmountisundefinedat component mount (before defaults are fully applied), the ternary only checks for'max', potentially assigningundefinedtolastCustomMaxdespite thestringtype annotation.🛡️ Suggested defensive initialization
- let lastCustomMax: string = opts.flashMintMaxAmount === 'max' ? '0' : opts.flashMintMaxAmount; + let lastCustomMax: string = (!opts.flashMintMaxAmount || opts.flashMintMaxAmount === 'max') ? '0' : opts.flashMintMaxAmount;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/cairo_alpha/ERC20Controls.svelte` around lines 38 - 42, The initialization can assign undefined to lastCustomMax if opts.flashMintMaxAmount is undefined; change the initialization and the reactive update to defensively default undefined to a safe string (e.g., '0') and coerce any non-'max' value to String so lastCustomMax is always a string; update references to the symbols lastCustomMax and opts.flashMintMaxAmount (both in the initial let and the $: reactive block) to perform an explicit undefined check and String(...) conversion instead of only comparing to 'max'.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/cairo_alpha/src/erc20.ts`:
- Around line 397-409: The override created when overridesMax is true for
flashMintConfigTrait::max_flash_loan ignores total_supply and can return a cap
larger than the remaining mint headroom; modify the function body (the fn
created for name 'max_flash_loan') to compute the remaining headroom as
u256::MAX - total_supply and return the minimum of customMax and that remaining
headroom (while still returning 0 if token != get_contract_address()); ensure
u256 arithmetic/underflow is handled correctly so you clamp to remaining =
u256::MAX - total_supply and return min(customMax, remaining).
- Around line 329-349: The parseFlashMintFeePercent function can produce an
arbitrarily large decimalScale/denominator (from decimalDigits) which may
overflow the u256 used later; add a guard after computing
decimalDigits/decimalScale that ensures decimalScale and the resulting
denominator (100n * decimalScale) fit within the u256 limit and reject inputs
with too many fractional digits by throwing OptionsError; specifically check
decimalScale <= MAX_DECIMAL_SCALE (or denominator <= MAX_U256 where MAX_U256 =
2n ** 256n - 1n) and reference the symbols decimalDigits, decimalScale,
denominator, and OptionsError in the check.
- Around line 319-326: The parseFlashMintMaxAmount path currently returns any
BigInt size and can emit out-of-range Cairo literals; modify
parseFlashMintMaxAmount to mirror premint's validation by converting the
computed BigInt (from getInitialSupply(value, Number(decimals))) through
toUint(..., 'u256') and return that value, and if toUint throws/returns an
error, surface it as an OptionsError for flashMintMaxAmount; keep the existing
checks for value === 'max' (return null) and the premintPattern/empty-string
validation that throws OptionsError.
---
Nitpick comments:
In `@packages/core/cairo_alpha/src/tests/with_components_off/erc20/erc20.test.ts`:
- Around line 125-173: Add two regression tests in the same suite that call
buildERC20 to assert validation failures for numeric-but-unrepresentable inputs:
one that sets flashMintMaxAmount to a numeric string greater than u256::MAX
(expecting the same OptionsError.messages.flashMintMaxAmount = 'Must be "max" or
a non-negative number'), and another that sets flashMintFeeMode: 'percent' with
flashMintFeePercent as a highly precise fractional string (many fractional
digits) that would overflow the denominator used to emit a Cairo literal
(expecting OptionsError.messages.flashMintFeePercent = 'Must be a number between
0 and 100'); mirror the existing test structure and assertions around buildERC20
to ensure these failure paths are covered.
In `@packages/ui/src/cairo_alpha/ERC20Controls.svelte`:
- Around line 38-42: The initialization can assign undefined to lastCustomMax if
opts.flashMintMaxAmount is undefined; change the initialization and the reactive
update to defensively default undefined to a safe string (e.g., '0') and coerce
any non-'max' value to String so lastCustomMax is always a string; update
references to the symbols lastCustomMax and opts.flashMintMaxAmount (both in the
initial let and the $: reactive block) to perform an explicit undefined check
and String(...) conversion instead of only comparing to 'max'.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7e84975b-b3e6-4358-99ce-0250a7f0e4ca
⛔ Files ignored due to path filters (2)
packages/core/cairo_alpha/src/tests/with_components_off/erc20/erc20.test.ts.snapis excluded by!**/*.snappackages/core/cairo_alpha/src/tests/with_components_on/erc20/erc20.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
.changeset/erc20-flash-mint.mdpackages/common/src/ai/descriptions/cairo.tspackages/core/cairo_alpha/CHANGELOG.mdpackages/core/cairo_alpha/src/contract.tspackages/core/cairo_alpha/src/erc20.tspackages/core/cairo_alpha/src/generate/erc20.tspackages/core/cairo_alpha/src/print.tspackages/core/cairo_alpha/src/tests/with_components_off/erc20/erc20.test.tspackages/core/cairo_alpha/src/tests/with_components_off/erc20/erc20.test.ts.mdpackages/core/cairo_alpha/src/tests/with_components_on/erc20/erc20.test.tspackages/core/cairo_alpha/src/tests/with_components_on/erc20/erc20.test.ts.mdpackages/ui/api/ai-assistant/function-definitions/cairo-alpha.tspackages/ui/src/cairo_alpha/ERC20Controls.svelte
…inds by upgradeable option
…keep number of generated contracts per run equal to 128
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supports the
ERC20FlashMintComponentfor the ERC20 token in Cairo-alpha.Configurable settings:
Max(default) or a non-negative custom cap;Percentof the loan amount (fractional values supported) orCustom(TODO stub);Burn(default) orFee receiver(passed as constructor arg, stored in storage);