Conversation
🧪 Cairo Contract Size Benchmark DiffBYTECODE SIZE (felts) (limit: 81,920 felts)
SIERRA CONTRACT CLASS SIZE (bytes) (limit: 4,089,446 bytes)
This comment was generated automatically from benchmark diffs. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1608 +/- ##
==========================================
- Coverage 94.01% 94.00% -0.02%
==========================================
Files 96 98 +2
Lines 2391 2419 +28
==========================================
+ Hits 2248 2274 +26
- Misses 143 145 +2
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
immrsd
left a comment
immrsd
left a comment
There was a problem hiding this comment.
Looking good and consistent with the Solidity implementation!
Left a couple of comments
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization 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:
WalkthroughThis pull request introduces ERC-3156 Flash Mint support for ERC20 tokens, including new Starknet interfaces for flash lenders and borrowers, a component implementation enabling flash loans, mock contracts for testing, and a comprehensive test suite validating the flash loan workflow. Changes
Sequence DiagramsequenceDiagram
participant Initiator as Initiator<br/>(Flashloan Caller)
participant Lender as ERC20FlashMint<br/>(Lender)
participant Token as ERC20<br/>(Token)
participant Receiver as Borrower<br/>(Receiver Contract)
Initiator->>Lender: flash_loan(receiver, token, amount, data)
Lender->>Lender: Validate amount ≤ max_flash_loan
Lender->>Lender: Calculate fee
Lender->>Token: mint(receiver, amount)
rect rgba(100, 150, 200, 0.5)
Lender->>Receiver: on_flash_loan(initiator, token, amount, fee, data)
Receiver->>Token: transfer/approve operations
Receiver-->>Lender: return magic value
end
Lender->>Lender: Validate return value & approvals
Lender->>Token: burn amount or transfer to fee_receiver
Lender-->>Initiator: true (success)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements the ERC-20 Flash Mint Extension following the ERC-3156 standard for flash loans (issue #1577). It adds a new ERC20FlashMintComponent that allows ERC20 tokens to offer single-transaction flash loans by minting tokens to a borrower and burning them back (plus an optional fee) within the same transaction.
Changes:
- New
ERC20FlashMintComponentimplementingIERC3156FlashLenderwith configurable fee logic viaFeeConfigTrait - New
IERC3156FlashLenderandIERC3156FlashBorrowerinterface definitions - Module registration in
extensions.cairoandtoken.cairo
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
packages/token/src/erc20/extensions/erc20_flash_mint.cairo |
Core flash mint component implementation |
packages/token/src/erc20/extensions.cairo |
Registers the new erc20_flash_mint module |
packages/interfaces/src/token/erc3156.cairo |
Defines IERC3156FlashLender and IERC3156FlashBorrower interfaces |
packages/interfaces/src/token.cairo |
Registers the new erc3156 module |
packages/interfaces/src/token/erc20.cairo |
Minor blank-line cleanup only |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…eat/erc20-flash-mint-#1577
…/cairo-contracts into feat/erc20-flash-mint-#1577
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/token/src/tests/erc20/test_erc20_flash_mint.cairo (1)
152-155: Extract duplicated borrower funding setup into a helper.The pre-funding sequence is duplicated; pulling it into one helper will keep tests leaner.
♻️ Suggested refactor
+fn fund_borrower(token: ContractAddress, borrower: ContractAddress, amount: u256) { + let token_dispatcher = erc20(token); + start_cheat_caller_address(token, OWNER); + assert!(token_dispatcher.transfer(borrower, amount)); + stop_cheat_caller_address(token); +} + #[test] fn flash_loan_burns_fees_when_fee_receiver_is_zero() { @@ - start_cheat_caller_address(token, OWNER); - assert!(token_dispatcher.transfer(borrower, FLASH_FEE)); - stop_cheat_caller_address(token); + fund_borrower(token, borrower, FLASH_FEE); @@ #[test] fn flash_loan_transfers_fee_to_fee_receiver() { @@ - start_cheat_caller_address(token, OWNER); - assert!(token_dispatcher.transfer(borrower, FLASH_FEE)); - stop_cheat_caller_address(token); + fund_borrower(token, borrower, FLASH_FEE);Also applies to: 177-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/token/src/tests/erc20/test_erc20_flash_mint.cairo` around lines 152 - 155, Extract the duplicated pre-funding sequence into a helper (e.g., prefund_borrower) that wraps start_cheat_caller_address(token, OWNER); asserts token_dispatcher.transfer(borrower, FLASH_FEE); and calls stop_cheat_caller_address(token); then replace both occurrences (the block using start_cheat_caller_address/transfer/stop_cheat_caller_address at lines around the two test spots) with a single call to that helper to keep tests DRY.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/token/src/tests/erc20/test_erc20_flash_mint.cairo`:
- Around line 152-155: Extract the duplicated pre-funding sequence into a helper
(e.g., prefund_borrower) that wraps start_cheat_caller_address(token, OWNER);
asserts token_dispatcher.transfer(borrower, FLASH_FEE); and calls
stop_cheat_caller_address(token); then replace both occurrences (the block using
start_cheat_caller_address/transfer/stop_cheat_caller_address at lines around
the two test spots) with a single call to that helper to keep tests DRY.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ccd6fa2-45e2-4e86-b47c-841ee045bdea
📒 Files selected for processing (11)
CHANGELOG.mdpackages/interfaces/src/lib.cairopackages/interfaces/src/token.cairopackages/interfaces/src/token/erc20.cairopackages/interfaces/src/token/erc3156.cairopackages/test_common/src/mocks/erc20.cairopackages/token/Scarb.tomlpackages/token/src/erc20/extensions.cairopackages/token/src/erc20/extensions/erc20_flash_mint.cairopackages/token/src/tests/erc20.cairopackages/token/src/tests/erc20/test_erc20_flash_mint.cairo
💤 Files with no reviewable changes (1)
- packages/interfaces/src/token/erc20.cairo
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| self: @ComponentState<TContractState>, token: ContractAddress, total_supply: u256, | ||
| ) -> u256 { | ||
| let this = get_contract_address(); | ||
| if token != this { |
There was a problem hiding this comment.
Can't you have it one line if token != get_contract_address() {
There was a problem hiding this comment.
I would argue is easier to read for our users like this, since it maight not be quick getting what get_contract_address represent, but no strong opinion.
immrsd
left a comment
immrsd
left a comment
There was a problem hiding this comment.
Looking good! Left a few minor suggestions
What's the plan on adding support for the component in with_components macro, will it be done in a separate PR?
separate PR for the sake of me prioritizing the macro refactor. |
|
LGTM! |
Fixes #1577
PR Checklist
Summary by CodeRabbit
Release Notes
New Features
Tests