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

Gauntlet Aave V3 Ava Recommendations #18

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

reem
Copy link

@reem reem commented Nov 1, 2022

Creates a new steward contract that can be used to implement risk parameter updates to Aave V3 Ava markets, also adds tests and deploy scripts for the contract. This contract can be edited and used for future risk updates as well.

My solidity is alright but certainly not as strong as y'alls so very much interested in feedback on how to improve the contract if you have any!

0x4365F8e70CF38C6cA67DE41448508F2da8825500;

function setUp() public {
vm.createSelectFork(vm.rpcUrl("avalanche"), 18805477);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are forking from too much in the past. Better to use a fresher Avalanche block

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the --fork-block-number 21789453 from CLI will be factually ignored (pretty sure)

address public constant GUARDIAN_AVALANCHE =
0xa35b76E4935449E33C56aB24b23fcd3246f13470;

address public constant CURRENT_ACL_SUPERADMIN =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required, the GUARDIAN_AVALANCHE has all the necessary permissions to send the risk_admin role

@eboadom
Copy link
Contributor

eboadom commented Nov 1, 2022

@reem could you also add to the README the new steward?


uint256 constant NUM_UPDATES = 5;

struct Updates {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, it works and is correct, but I wonder why you decided to declare this way instead of returning in _getUpdates() a ParameterSet[] memory

@reem
Copy link
Author

reem commented Nov 1, 2022

Updated for your comments, thanks. The Updates struct was just a holdover from when I had multiple arrays instead of just one array of ParameterSets. Good catch on the fork block.

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

Successfully merging this pull request may close these issues.

2 participants