Skip to content

Conversation

chanhosuh
Copy link

No description provided.

@chanhosuh chanhosuh requested a review from opz August 3, 2022 18:23
* @dev A negative amount indicates extra tokens not needed for the reserve
* @return The amount of missing tokens
*/
function getReserveTopUpValue() external view returns (int256);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's come up with a new name for this while we have the opportunity. It may make sense to switch the sign and make it a uint256 as well, because it will only be used to determine excess tokens, never tokens that should be returned.

Maybe we should overall avoid the reference to the LP Account, even with transferToLpAccount. All the vault fundamentally cares about is that there is something it can transfer to, that has permission to initiate the transfer, and that it can only transfer the excess tokens.

One way we could think of this is as a 0% interest loan to another contract. Might be the wrong approach, but let's see how that might look:

function lendUnderlyer(uint256 amount) external;

function getMaxUnderlyerLoan() external view returns (uint256);

Could also keep it simpler and stay with the transfer terminology:

function transferUnderlyer(uint256 amount) external;

function getTransferUnderlyerMaxAmount() external view returns (uint256);

In both cases I think it makes sense to drop the sign and invert the calculation so excess tokens is a positive number. Should simplify the rest of any calculations.

* @dev Tokens from the vault are typically borrowed by the LP Account
* @return The USD value. USD prices have 8 decimals.
*/
function _getDeployedValue() internal view returns (uint256) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we go with the lending terminology mentioned above, this could become _getLoanedUsdValue

@chanhosuh chanhosuh changed the title Remove mAPT dependence from Index Token [WIP] Remove mAPT dependence from Index Token Aug 15, 2022
@chanhosuh
Copy link
Author

Address Registry dependence has been removed. Dependencies are all contracts, not EOAs, so their respective setters all validate the passed address is a contract address. initialize now accepts the necessary addresses as args, including the one for DEFAULT_ADMIN_ROLE. One change to consider is to hardcode the emergency safe address for that role as it controls setting the other roles.

Unit tests have been updated but the tests in test-integration/IndexToken.js need to be updated.

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