Skip to content

Conversation

pblivin0x
Copy link
Contributor

@pblivin0x pblivin0x commented Apr 26, 2022

Notes

  • Bump set-protocol-v2 version
  • Added IAirdropModule, IClaimModule, and IWrapModuleV2 interfaces to set-protocol-v2 [PR]

@pblivin0x pblivin0x requested a review from cgewecke May 3, 2022 14:12
* ONLY OWNER: Update address AirdropModule manager fees are sent to.
*
* @param _setToken Address of SetToken
* @param _newFeeRecipient Address of new fee recipient
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may want an endpoint to distributeFees to the DelegatedManager, similar to the IssuanceExtension, but with the reward token as input.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like fees are auto-distributed in the module whenever someone calls absorb, here. Is there a case where a separate distribution mechanism would be helpful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thinking here was that we accrue to the DelegatedManager first, and then distribute to the owner and methodologist according to the fee split.

With the IssuanceExtension we

  • accrue mint/redeem fees to the DelegatedManager everytime someone calls mint/redeem
  • distribute via IssuanceExtension.distributeFees()

We would follow a similar pattern with the ClaimExtension with

  • accrue absorb fees to the DelegatedManager everytime someone calls absorb
  • distribute via ClaimExtension.distributeFees(rewardToken)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok great, yes agree it makes sense that this would have the same interface & behavior as IssuanceExtension.

Copy link
Contributor

@cgewecke cgewecke left a comment

Choose a reason for hiding this comment

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

This looks basically flawless to me. @pblivin0x Is there anything your uncertain about or that's come up in the last couple weeks with respect to the PRD?

The only substantive thing left to do is bump the upstream contracts package and pull the interfaces from there / remove them from here.

* ONLY OWNER: Update address AirdropModule manager fees are sent to.
*
* @param _setToken Address of SetToken
* @param _newFeeRecipient Address of new fee recipient
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like fees are auto-distributed in the module whenever someone calls absorb, here. Is there a case where a separate distribution mechanism would be helpful?

Copy link
Contributor

@cgewecke cgewecke left a comment

Choose a reason for hiding this comment

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

🎉

@pblivin0x pblivin0x merged commit 3779c80 into master May 20, 2022
@pblivin0x pblivin0x deleted the pranav/add-productive-asset-extensions branch May 20, 2022 17:52
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