ERC20 Rebalancing Set Exchange Issuance #486
Conversation
Pull Request Test Coverage Report for Build 5896
💛 - Coveralls |
* @title ERC20RebalancingSetExchangeIssuanceModule | ||
* @author Set Protocol | ||
* | ||
* The ERC20RebalancingSetExchangeIssuanceModule supplementary smart contract allows a user to send Eth and atomically |
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.
send any ERC20 to be used in decentralized exchange
* @author Set Protocol | ||
* | ||
* The ERC20RebalancingSetExchangeIssuanceModule supplementary smart contract allows a user to send Eth and atomically | ||
* issue a rebalancing Set |
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.
nit: use the contract name here. The token they're issuing is the RebalancingSet
/* ============ Public Functions ============ */ | ||
|
||
/** | ||
* Issue a Rebalancing Set using Wrapped Ether to acquire the base components of the Base Set. |
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.
update me
_exchangeIssuanceParams.receiveTokens[0], | ||
address(this) | ||
); | ||
if ( paymentTokenInVault > 0 ) { |
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.
weird spacing
/* ============ Private Functions ============ */ | ||
|
||
/** | ||
* Any unused Wrapped Ether or base Set issued is returned to the caller. |
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.
update me
} | ||
|
||
/** | ||
* Redeems a Rebalancing Set into Wrapped Ether. The Rebalancing Set is redeemed into the Base Set, and |
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.
update me
); | ||
|
||
// Create 0x order for the component, using weth(4) paymentToken as default | ||
zeroExOrder = await setUtils.generateZeroExSignedFillOrder( |
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 as before, this test is not very indicative of what we're going to launch with.
- the set has a single component
- it's a single 0x order
- it's 0x
at the very minimum lets get these switched over to using kyber for the one component
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.
we have a second to breath now, we should circle back and address this the right way this time.
Let's make sure to keep comments updated. Very misleading for auditors |
|
||
// Address and instance of Transfer Proxy contract | ||
address public transferProxy; | ||
ITransferProxy public transferProxyInstance; |
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.
It appears from the ABDK review that these interfaces can also function as an address. So when we store the address and the instance we're just duping the data so maybe it makes sense to remove transferProxy
and exchangeIssuanceModule
.
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.
+1 that is correct
* @param _rebalancingSetAddress Address of the rebalancing Set | ||
* @param _rebalancingSetQuantity Quantity of rebalancing Set to redeem | ||
* @param _exchangeIssuanceParams Struct containing data around the base Set issuance | ||
* @param _orderData Bytecode formatted data with exchange data for acquiring base set components |
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.
selling base set components
* | ||
* @param _rebalancingSetAddress Address of the rebalancing Set | ||
* @param _rebalancingSetQuantity Quantity of rebalancing Set to redeem | ||
* @param _exchangeIssuanceParams Struct containing data around the base Set issuance |
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.
redemption
No description provided.