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

Add governance support for T coverage pool underwriter token holders #199

Merged
merged 9 commits into from
Oct 26, 2021

Conversation

tomaszslabon
Copy link
Contributor

@tomaszslabon tomaszslabon commented Oct 14, 2021

This PR extends the UnderwriterToken by Compound-like voting and delegation features.

The following changes were made:

  • updated thesis/solidity-contracts to the newest version
  • added dependency threshold-network/solidity-contracts
  • made UnderwriterToken be derived from Checkpoints (implemented functions delegate() and delegateBySig())
  • overridden function beforeTokenTransfer so that voting power is given on minting and burning tokens.

Checkpoints can only work with up to 2^96 - 1 tokens. To ensure this requirement is handled properly, there is a check that prevents minting more UnderwriterTokens than 2^96-1.

@tomaszslabon tomaszslabon force-pushed the underwriter-delegation-support branch from fa814cb to 29980f0 Compare October 15, 2021 13:48
@tomaszslabon tomaszslabon requested a review from pdyraga October 15, 2021 13:48
@pdyraga
Copy link
Member

pdyraga commented Oct 18, 2021

TODO: Checkpoints can only work with up to 2^96 - 1 tokens. Ensure this requirement is handled properly in UnderwriterToken.

I think (and please validate it carefully) that all we need is to reduce precision in AssetPool._calculateTokensToMint and return uint96. If someone deposits so small amount of tokens that this function returns 0, the deposit transaction should revert thanks to this check:

require(
amountToMint > 0,
"Minted tokens amount must be greater than 0"
);

@tomaszslabon tomaszslabon force-pushed the underwriter-delegation-support branch 2 times, most recently from 5961f6e to c70b8ce Compare October 20, 2021 15:11
@tomaszslabon tomaszslabon force-pushed the underwriter-delegation-support branch from c70b8ce to fde97c4 Compare October 20, 2021 16:09
@tomaszslabon
Copy link
Contributor Author

tomaszslabon commented Oct 22, 2021

TODO: Checkpoints can only work with up to 2^96 - 1 tokens. Ensure this requirement is handled properly in UnderwriterToken.

I think (and please validate it carefully) that all we need is to reduce precision in AssetPool._calculateTokensToMint and return uint96. If someone deposits so small amount of tokens that this function returns 0, the deposit transaction should revert thanks to this check:

require(
amountToMint > 0,
"Minted tokens amount must be greater than 0"
);

@piotr, if I understand correctly we just need to ensure that the total supply of UnderwriterToken is no greater than type(uint96).max. This way even if only one user has all the UnderwriterTokens, we will not have problems storing their voting power in a checkpoint.

And this is ensured in the beforeTokenTransfer function, which is called while minting.

@pdyraga
Copy link
Member

pdyraga commented Oct 22, 2021

TODO: Checkpoints can only work with up to 2^96 - 1 tokens. Ensure this requirement is handled properly in UnderwriterToken.

I think (and please validate it carefully) that all we need is to reduce precision in AssetPool._calculateTokensToMint and return uint96. If someone deposits so small amount of tokens that this function returns 0, the deposit transaction should revert thanks to this check:

require(
amountToMint > 0,
"Minted tokens amount must be greater than 0"
);

@piotr, if I understand correctly we just need to ensure that the total supply of UnderwriterToken is no greater than type(uint96).max. This way even if only one user has all the UnderwriterTokens, we will not have problems storing their voting power in a checkpoint.

And this is ensured in the beforeTokenTransfer function, which is called while minting.

Correct though I think there is an incompatibility of precisions. _calculateTokensToMint operates on uin256 so 18 decimals precision, same beforeTokenTransfer and maxSupply comparison does not lose the precision.

@tomaszslabon tomaszslabon force-pushed the underwriter-delegation-support branch from 9ca7e29 to e08955d Compare October 26, 2021 09:20
@tomaszslabon tomaszslabon marked this pull request as ready for review October 26, 2021 09:54
@tomaszslabon tomaszslabon force-pushed the underwriter-delegation-support branch from d330ff5 to 10ba5cb Compare October 26, 2021 14:04
@pdyraga pdyraga merged commit 31895aa into main Oct 26, 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.

2 participants