Remote GSM #451
Open
Remote GSM #451
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
|
@miguelmtzinf all comments have been addressed, thanks for the name suggestions, didn't know what to use! |
src/contracts/facilitators/gsm/interfaces/IOwnableFacilitator.sol
Outdated
Show resolved
Hide resolved
src/contracts/facilitators/gsm/interfaces/IOwnableFacilitator.sol
Outdated
Show resolved
Hide resolved
| @@ -43,14 +44,14 @@ contract Gsm4626 is Gsm, IGsm4626 { | |||
| ) external notSeized onlyRole(CONFIGURATOR_ROLE) returns (uint256) { | |||
| require(amount > 0, 'INVALID_AMOUNT'); | |||
|
|
|||
| (, uint256 ghoMinted) = IGhoToken(GHO_TOKEN).getFacilitatorBucket(address(this)); | |||
| (, uint256 deficit) = _getCurrentBacking(ghoMinted); | |||
| uint256 ghoUsed = _getUsedGho(); | |||
There was a problem hiding this comment.
change to usedGho for consistency
|
|
||
| /// Map of entities and their assigned capacity and amount of GHO used | ||
| mapping(address => GhoUsage) private _ghoUsage; | ||
|
|
There was a problem hiding this comment.
Wondering if we should add global variables for total limit and total used. It's a bit hard to iterate over entities also, to calculate totals
There was a problem hiding this comment.
Would total limit be every time we set a limit +/- to the global variable? For used it's easy to keep track
There was a problem hiding this comment.
yeah, would be keeping global accounting every time an entity updates its usage (limit or used). Not sure it's completely needed
Comment on lines
+46
to
+49
| GhoUsage memory usage = _ghoUsage[msg.sender]; | ||
| require(usage.limit >= usage.used + amount, 'LIMIT_REACHED'); | ||
|
|
||
| _ghoUsage[msg.sender].used += uint128(amount); |
There was a problem hiding this comment.
Suggested change
| GhoUsage memory usage = _ghoUsage[msg.sender]; | |
| require(usage.limit >= usage.used + amount, 'LIMIT_REACHED'); | |
| _ghoUsage[msg.sender].used += uint128(amount); | |
| GhoUsage storage entity = _ghoUsage[msg.sender]; | |
| require(entity.limit >= entity.used + amount, 'LIMIT_REACHED'); | |
| entity.used += uint128(amount); |
| } | ||
|
|
||
| /// @inheritdoc IGhoReserve | ||
| function setEntityLimit(address entity, uint256 limit) external onlyOwner { |
There was a problem hiding this comment.
Suggested change
| function setEntityLimit(address entity, uint256 limit) external onlyOwner { | |
| function setLimit(address entity, uint256 limit) external onlyOwner { |
| */ | ||
| interface IGhoReserve { | ||
| /** | ||
| * Struct representing GSM's maximum allowed GHO usage and amount used |
There was a problem hiding this comment.
Suggested change
| * Struct representing GSM's maximum allowed GHO usage and amount used | |
| * @dev Struct data representing GHO usage. |
Comment on lines
+85
to
+86
| * @notice Returns amount of GHO used by a specified address | ||
| * @param entity Address of the contract that withdrew GHO from reserve |
There was a problem hiding this comment.
Suggested change
| * @notice Returns amount of GHO used by a specified address | |
| * @param entity Address of the contract that withdrew GHO from reserve | |
| * @notice Returns the amount of GHO used by a specified entity | |
| * @param entity The address of the entity | |
| * @return The amount of GHO used |
Comment on lines
+91
to
+94
| * @notice Returns limit of GHO and used amount for a given entity | ||
| * @param entity Address of the contract that can use GHO from reserve | ||
| * @return Limit of GHO that can be used | ||
| * @return Used amount |
There was a problem hiding this comment.
Suggested change
| * @notice Returns limit of GHO and used amount for a given entity | |
| * @param entity Address of the contract that can use GHO from reserve | |
| * @return Limit of GHO that can be used | |
| * @return Used amount | |
| * @notice Returns the usage data of a specified entity | |
| * @param entity The address of the entity | |
| * @return The usage limit | |
| * @return The amount of GHO used |
Comment on lines
+99
to
+100
| * @notice Returns maximum amount of GHO that can be used by a specified address | ||
| * @param entity Address of the contract that uses GHO from reserve |
There was a problem hiding this comment.
Suggested change
| * @notice Returns maximum amount of GHO that can be used by a specified address | |
| * @param entity Address of the contract that uses GHO from reserve | |
| * @notice Returns the usage limit of a specified entity | |
| * @param entity The address of the entity | |
| * @return The usage limit |
| * @param entity The address that can use GHO | ||
| * @param limit The maximum usage limit of the entity | ||
| */ | ||
| event EntityLimitUpdated(address indexed entity, uint256 limit); |
There was a problem hiding this comment.
feels more aligned
Suggested change
| event EntityLimitUpdated(address indexed entity, uint256 limit); | |
| event GhoLimitUpdated(address indexed entity, uint256 limit); |
Comment on lines
+43
to
+44
| * @param entity The address that can use GHO | ||
| * @param limit The maximum usage limit of the entity |
There was a problem hiding this comment.
Suggested change
| * @param entity The address that can use GHO | |
| * @param limit The maximum usage limit of the entity | |
| * @param entity The address of the entity | |
| * @param limit The new usage limit |
Comment on lines
+26
to
+28
| _transferOwnership(initialOwner); | ||
|
|
||
| GHO_TOKEN = ghoAddress; |
There was a problem hiding this comment.
Suggested change
| _transferOwnership(initialOwner); | |
| GHO_TOKEN = ghoAddress; | |
| _transferOwnership(initialOwner); | |
| GHO_TOKEN = ghoAddress; |
Comment on lines
+19
to
+20
| * @param initialOwner Address of the initial owner of the contract | ||
| * @param ghoAddress Address of GHO token on mainnet |
There was a problem hiding this comment.
Suggested change
| * @param initialOwner Address of the initial owner of the contract | |
| * @param ghoAddress Address of GHO token on mainnet | |
| * @param initialOwner The address of the initial owner | |
| * @param ghoAddress The address of GHO token |
Comment on lines
+121
to
+122
| uint256 ghoLevel = _getUsedGho(); | ||
| uint256 ghoLimit = _getLimit(); |
There was a problem hiding this comment.
should we create an internal function _getUsage that returns both?
apart from that, let's rename Level to Used please
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changelog
Add RemoteGSM Implementation
Abstract methods on GSM