Skip to content
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

Review RariFuseAllocator #268

Open
ghost opened this issue Mar 19, 2022 · 4 comments
Open

Review RariFuseAllocator #268

ghost opened this issue Mar 19, 2022 · 4 comments
Assignees
Labels
allocator Treasury Asset Allocator

Comments

@ghost
Copy link

ghost commented Mar 19, 2022

Below I am giving you a link to the contract which should be reviewed including tests.

contract RariFuseAllocator is BaseAllocator {

async function setupTestingEnvironment(): Promise<void> {

@ghost ghost added the allocator Treasury Asset Allocator label Mar 19, 2022
@ghost ghost assigned ind-igo, tonypiano and 0xLienid Mar 19, 2022
@ghost
Copy link
Author

ghost commented Mar 19, 2022

Yes, in the future I will work on a local branch and issue this as a PR. Up until now I have been doing this on main. I will keep pushing reorganization though directly to main.

@0xLienid
Copy link
Contributor

0xLienid commented Mar 19, 2022

IERC20Metadata b = IERC20Metadata(address(_tokens[index]));

any reason for using IERC20Metadata vs IERC20 given you just use balanceOf and approve?
also, is there a better variable name option than "b"

@0xLienid
Copy link
Contributor

if (balance > 0) { if (amounts[i] == type(uint256).max) f.redeem(f.balanceOf(address(this)));

looks like you already cache f.balanceOf(address(this)) to the balance variable, can just use here.

@ind-igo
Copy link
Contributor

ind-igo commented Mar 19, 2022

any reason for using IERC20Metadata vs IERC20 given you just use balanceOf and approve? also, is there a better variable name option than "b"

I think this is because he needs the decimals in the _worth function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
allocator Treasury Asset Allocator
Projects
None yet
Development

No branches or pull requests

3 participants