-
Notifications
You must be signed in to change notification settings - Fork 1
STIP 006 - Trade Rebates #12
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused what the goal is with these fees, is this what we want? Don't we actually want something that tracks potential rebates based on the volume traded? The we'd charge the protocol fee as normal but payback a portion of that to the manager?
Added option 4 which is governance chooses the protocol fee and a rebate split % |
#### Recommendation | ||
Option 1 will require the least code changes while giving us the ability to emulate fee rebates. The maxManagerFee can be enforced either on initialization (still susceptible to manager rugging by removing and reinitializing) or stored on the Controller by governance | ||
#### Future work | ||
Tiered rebates: With this base rebate mechanism, manager contracts can be built on top in the future. Individual managers will always have the ability to specify a fee recipient address as their own, but manager contracts for a specific product (e.g. social trading) can abstract away the fee recipient to a shared peripheral contract. This central trading contract tracks cumulative volume done through trading all the Sets that are linked, and perform calculations that split the rebates collected. This way, at the base module level, rebates are flat across Sets but at the manager contract level, rebates can be tiered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing I'd add here as potential future feature is potentially updating the TradeModule to allow each Set to have its own rebate percentage (instead of a global one for now). This could be negotiated with large managers/applications that use the protocol.
STIPS/STIP-006.md
Outdated
### TradeModuleV2 | ||
- Inherit TradeModule | ||
- Override `trade` | ||
- Add `virtual` to TradeModule V1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think you have to do this. Would rather not touch the V1 code.
STIPS/STIP-006.md
Outdated
- Override `trade` | ||
- Add `virtual` to TradeModule V1 | ||
- Add `_accrueManagerFee` | ||
- Override constructor to add `managerRebateRecipient` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean initialize
?
STIPS/STIP-006.md
Outdated
- Override `trade` and `tradeRemainingWETH` | ||
- Add `virtual` to GeneralIndexModule trade functions | ||
- Add `_accrueManagerFee` | ||
- Override constructor to add `managerRebateRecipient` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comments here as above
STIPS/STIP-006.md
Outdated
- Inherit TradeModule | ||
- Override `trade` to update ComponentExchanged event | ||
- Override `_accrueProtocolFee` | ||
- Override initialize to add `managerRebateRecipient` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it'll be this simple since this will require a different interface so isn't a clean override, more of an overload. Also we'll have to track this state somewhere.
#### Public Variables | ||
| Type | Name | Description | | ||
|------ |------ |------------- | | ||
|mapping(ISetToken => address);|managerRebateRecipient|Mapping of SetToken to address of rebate recipient| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense
> initialize(SetToken _setToken, address _managerRebateRecipient) external | ||
- managerRebateRecipient: manager rebate recipient address | ||
```solidity | ||
function initialize( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting this will also require a change in the initialize
function on the GIMExtension
STIPS/STIP-006.md
Outdated
> function _accrueProtocolFee(TradeInfo memory _tradeInfo, uint256 _exchangedQuantity) internal returns (uint256, uint256) | ||
- No changes to interface | ||
```solidity | ||
function _accrueProtocolFee(TradeInfo memory _tradeInfo, uint256 _exchangedQuantity) internal returns (uint256 protocolFeeTotal, uint256 managerRebateTotal) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
override
STIPS/STIP-006.md
Outdated
> function _accrueProtocolFee(TradeInfo memory _tradeInfo) internal returns (uint256, uint256) | ||
- No changes to interface | ||
```solidity | ||
function _accrueProtocolFee(TradeInfo memory _tradeInfo) internal returns (uint256 protocolFeeTotal, uint256 managerRebateTotal) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
override
My main concern is that |
No description provided.