Core issuance flow #54
Conversation
test/logs/log_utils.ts
Outdated
| case (/^(uint)\d*\[\]/.test(type)): { | ||
| break; | ||
| } | ||
| case (/^(uint)\d*/.test(type)): { |
contracts/core/Core.sol
Outdated
| _; | ||
| } | ||
|
|
||
| // Verify set was created by core and is enabled for issuance |
There was a problem hiding this comment.
don't mention issuance. i use the same for redeem
contracts/core/Core.sol
Outdated
| /* ============ Modifiers ============ */ | ||
|
|
||
| // Validate quantity is multiple of natural unit | ||
| modifier isValidQuantity(uint _quantity, address _setToken) { |
There was a problem hiding this comment.
lets mention natural unit in the modifier, isValidQuantity of what? we have another "quantity verifier" right below with nonZero check. maybe isNaturalUnitMultiple?
contracts/core/Core.sol
Outdated
| /** | ||
| * Issue | ||
| * | ||
| * @param _setAddress Array of the addresses of the ERC20 tokens |
There was a problem hiding this comment.
these descriptions seem off?
contracts/core/Core.sol
Outdated
|
|
||
| // Decrement the vault balance of the owner for the component | ||
| if (amountToDecrement > 0) { | ||
| IVault(vaultAddress).decrementTokenOwner( |
There was a problem hiding this comment.
we need to do this first. Right now i think you have a reentrancy attack. in ITransferProxy, we call ERC20.transferFrom which is a function out of our control. it can re-call into here from the transfer proxy cal on line 243 and the balance on line 235 will not have been updated yet
| } | ||
|
|
||
| // Increment the vault balance of the set token for the component | ||
| IVault(vaultAddress).incrementTokenOwner( |
There was a problem hiding this comment.
are we storing set token balance in vault? @bweick told me we were just storing balances of sets on the SetToken itself?
There was a problem hiding this comment.
this is to tell us the balance of the components in the vault that belong to the set
the set token itself keeps the balance of each user
| } | ||
|
|
||
| /* ============ Getter Functions ============ */ | ||
|
|
There was a problem hiding this comment.
nvm this space is on purpose
| var argValue: any = value; | ||
| switch (true) { | ||
| case (/^(uint)\d*\[\]/.test(type)): { | ||
| break; |
There was a problem hiding this comment.
Does this work? Don't we want to set the argValue to be an array of BigNumbers if this is the case?
There was a problem hiding this comment.
Yup, it worked for me
test/core.spec.ts
Outdated
| await coreWrapper.approveTransfersAsync(components, transferProxy.address); | ||
|
|
||
| const componentAddresses = _.map(components, (token) => token.address); | ||
| componentUnits = _.map(components, () => ether(4)); // Multiple of of naturalUnit |
Core issuance flow. Some interesting diffs: decided not to call deposit in issuance in order to save gas on the extra
incrementVaultOwnercall for the deposited amounts. Just subtract the amount of the component that was already in there.Changed
STANDARD_INITIAL_TOKENSconstant toDEPLOYED_TOKEN_QUANTITY