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

KEEP/NU <> T Vending Machine #3

Merged
merged 29 commits into from
Aug 16, 2021
Merged

KEEP/NU <> T Vending Machine #3

merged 29 commits into from
Aug 16, 2021

Conversation

pdyraga
Copy link
Member

@pdyraga pdyraga commented Jul 21, 2021

Refs #1
Depends on #2 (keeping it as a draft until the dependency is merged)

This Pull Request addresses the first two points of the Plan of the game described in #1.

VendingMachine contract implements a special lending protocol to enable KEEP/NU
token holders to wrap their tokens as collateral and borrow T tokens against that collateral
based on the wrap ratio. This will go on indefinitely and enable NU and KEEP token holders
to join T network without needing to buy or sell any assets. Logistically, anyone holding NU
or KEEP can wrap those assets in order to receive T. They can also unwrap T in order to go
back to the underlying asset. There will be a separate instance of this contract deployed for
KEEP holders and a separate instance of this contract deployed for NU holders.

pdyraga added 4 commits July 21, 2021 12:33
The Vending Machine allows to wrap KEEP/NU into T and unwrap it back
with the provided wrap ratio. This is just an initial flow and it will
be extended with a mapping of token holders who wrapped/unwrapped so
that T earned as a stakerd reward or bough on the exchange can not be
unwrapped back to KEEP/NU.
It is not possible to unwrap T into KEEP/NU if the given account has not
previously wrapped KEEP/NU to T. Also, the given account can not unwrap
more than it previously wrapped.
In practice, both tToken and wrappedToken are trusted contracts so there
is no risk of reentrancy.

In theory, there could be a risk of reentrancy if tToken or wrappedToken
are evil or could be exploited and Slither is complaining about it.

Let's make Slither happy and change the order of calls!
package.json Outdated Show resolved Hide resolved
@pdyraga pdyraga requested review from cygnusv, nkuba and vzotova July 21, 2021 13:40
Base automatically changed from setup to main July 27, 2021 07:42
@nkuba nkuba self-assigned this Jul 27, 2021
pdyraga added 4 commits July 30, 2021 17:45
Updated version to point to commit hash from main branch after PRs with
token and clone factory have been merged.
This is the version referenced in contract Solidity files.
The goal is not to use the most recent version per auditor's
recommendation.
@pdyraga pdyraga marked this pull request as ready for review July 30, 2021 16:12
@pdyraga
Copy link
Member Author

pdyraga commented Jul 30, 2021

#2 got merged so I could update the code here and undraft the PR. This is ready!

pdyraga and others added 10 commits July 30, 2021 20:38
Generalizing test amounts and minor refinements
Renamed `_amount` to `amount`. The convention we used so far was to use
underscore only when we had conflicts with other names, for example in
constructor where parameter names are conflicting with contract field
names.
We are always going to let to convert the entire wrapped token supply,
so renaming _maxWrappedToken constructor parameter to
_wrappedTokenSupply should hopefully explain its meaning better.
tToken = _tToken;
ratio =
(FLOATING_POINT_DIVISOR * _tTokenAllocation) /
_wrappedTokenSupply;
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about adding a few required for the input values? I know we control these parameters, but it costs nothing in terms of gas / code size.

address indexed recipient,
uint256 tTokenAmount,
uint256 wrappedTokenAmount
);
Copy link
Contributor

Choose a reason for hiding this comment

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

To make it perfect we can add docstrings for events too (and generate docs from contracts in some moment in a future)

Copy link
Member

Choose a reason for hiding this comment

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

Opened #11

@vzotova vzotova mentioned this pull request Aug 7, 2021
cygnusv added 11 commits August 8, 2021 20:34
…verted.

Previous behavior produced unintended results when the amount converted wasn't exact wrt to the conversion ratio. E.g, for a 10:3 ratio, the vending machine had different balances if the token holder converts 10 (resulting in 3), than converting 5 twice (resulting in 1+1 due to rounding). New behavior converts from the approved amount the max amount that can be converted exactly
We already need to restrict T token supply to uint96 anyway for GovernorBravo
Allow only exact conversions when wrapping/unwrapping
@cygnusv cygnusv merged commit 8ff4857 into main Aug 16, 2021
@cygnusv cygnusv deleted the vending-machine branch August 16, 2021 15:42
@pdyraga pdyraga added this to the solidity/v1.0.0 milestone Dec 31, 2021
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.

4 participants