feat: Collector revision-6 Upgrade. #82
Merged
Conversation
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
…ards compatiblity
sakulstra
reviewed
Dec 3, 2024
sakulstra
reviewed
Dec 3, 2024
…from initialize function. Deprecated fundsAdmin
sakulstra
reviewed
Dec 3, 2024
sakulstra
reviewed
Dec 4, 2024
| TreasuryReport memory treasuryReport; | ||
| bytes32 salt = collectorSalt; | ||
| address treasuryOwner = poolAdmin; | ||
| address aclManager; |
Contributor
There was a problem hiding this comment.
this seems incomplete, you need to pass in the aclManager otherwise i guess deployment would revert.
Collaborator
There was a problem hiding this comment.
@Luigy-Lemon you can pass the aclManager from AaveV3PeripheryBatch contract as a function param to _deployAaveV3Treasury()
src/deployments/contracts/procedures/AaveV3TreasuryProcedure.sol
Outdated
Show resolved
Hide resolved
sakulstra
reviewed
Dec 5, 2024
sakulstra
added a commit
that referenced
this pull request
Feb 10, 2025
### Context The Collector contract (Revision 5) has the limitation of only allowing one address to call its functions, the `_fundsAdmin`, which currently is the Governance Executor contract. To enable the possibility of the [FinanceSteward](https://governance.aave.com/t/arfc-aave-finance-steward/17570) contract to be allowed to call the Collector alongside the Executor, it is recommended to use the `ACL_MANAGER` to manage its access control. A new role is going to be created named `FUNDS_ADMIN` ### Changelog Collector * Initialize function changes to only populate the `nextStreamId` * Created constructor to setup the `ACL_MANAGER` contract address as immutable variable * deprecated `_fundsAdmin` variable * removed getter and setter functions for `_fundsAdmin` and relevant errors and events. * introduced new function to check if an address has the `FUNDS_ADMIN` role named `IsFundsAdmin` * New tests for the Collector Co-authored-by: Luigy-Lemon <[email protected]> Co-authored-by: Harsh Pandey <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Context
The Collector contract (Revision 5) has the limitation of only allowing one address to call its functions, the
_fundsAdmin, which currently is the Governance Executor contract.To enable the possibility of the FinanceSteward contract to be allowed to call the Collector alongside the Executor, it is recommended to use the
ACL_MANAGERto manage its access control.A new role is going to be created named
FUNDS_ADMINPS: identified a problem in the
VersionedInitializablewhich due to the introduction of theinitializingvariable is not backwards compatible with the revision 5 version of the collector. Revision 6 will be launched with a modified version of ______gap length of 49 rather than 50.Changelog
Collector
nextStreamIdACL_MANAGERcontract address as immutable variable_fundsAdminvariable_fundsAdminand relevant errors and events.FUNDS_ADMINrole namedIsFundsAdminProposal
In order to upgrade the Collector:
VersionedInitializablewith ______gap set to 49 and with the currentACL_MANAGERaddress.grantRolewhich should be granted to the current_fundsAdmin, the Executor.