-
Notifications
You must be signed in to change notification settings - Fork 41
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
Carbon batch router #175
base: dev
Are you sure you want to change the base?
Carbon batch router #175
Conversation
ivanzhelyazkov
commented
Dec 18, 2024
- Add CarbonBatcher contract - creates multiple strategies with one transaction
- Add tests
- Add deployment scripts
contract CarbonBatcher is Upgradeable, Utils, ReentrancyGuard, IERC721Receiver { | ||
using Address for address payable; | ||
|
||
error InsufficientNativeTokenSent(); |
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.
Can this error move to common/shared utils?
|
||
error InsufficientNativeTokenSent(); | ||
|
||
ICarbonController private immutable carbonController; |
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.
_carbonController
?
error InsufficientNativeTokenSent(); | ||
|
||
ICarbonController private immutable carbonController; | ||
IVoucher private immutable voucher; |
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.
_voucher
?
event FundsWithdrawn(Token indexed token, address indexed caller, address indexed target, uint256 amount); | ||
|
||
constructor( | ||
ICarbonController _carbonController, |
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.
We don't prefix function arguments with an _
, but if you did this in order to resolve the conflict with state variables, you can call rename these arguments to initCarbonController
, and initVoucher
, for example
) validAddress(address(_carbonController)) validAddress(address(_voucher)) { | ||
carbonController = _carbonController; | ||
voucher = _voucher; | ||
_disableInitializers(); |
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.
Missing new line?
Token token, | ||
address payable target, | ||
uint256 amount | ||
) external validAddress(target) onlyAdmin nonReentrant { |
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.
Why is this function required?
emit FundsWithdrawn({ token: token, caller: msg.sender, target: target, amount: amount }); | ||
} | ||
|
||
function onERC721Received(address, address, uint256, bytes calldata) external pure returns (bytes4) { |
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.
Missing NATSPEC?
return (uniqueTokens, amounts); | ||
} | ||
|
||
function _findInArray(Token element, Token[] memory array, uint256 arrayLength) private pure returns (uint256) { |
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.
Missing NATSPEC
} | ||
} | ||
|
||
function uncheckedInc(uint256 i) private pure returns (uint256 j) { |
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.
Missing NATSPEC
} | ||
} | ||
|
||
function uncheckedInc(uint256 i) private pure returns (uint256 j) { |
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.
Should it moved to a common/shared utilities?