-
Notifications
You must be signed in to change notification settings - Fork 58
Accounting and Issuance Optimizations #295
Conversation
felix2feng
commented
Nov 21, 2018
- Make usage of batch functions
- Moved deposit, increment, decrement and withdraw quantity calculations into separate functions
- Remove IssuanceComponentDeposited log
Pull Request Test Coverage Report for Build 2968
💛 - Coveralls |
_quantities[i] | ||
); | ||
} | ||
ITransferProxy(state.transferProxy).batchTransfer( |
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.
Would be good to add some comments here
state.vault | ||
); | ||
|
||
IVault(state.vault).batchIncrementTokenOwner( |
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.
Comment
@@ -137,43 +129,37 @@ contract CoreIssuance is | |||
uint256 naturalUnit = setToken.naturalUnit(); | |||
address[] memory components = setToken.getComponents(); | |||
uint256[] memory units = setToken.getUnits(); | |||
uint256[] memory componentQuantities = calculateTransferValues( |
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.
Comments needed in this function
@@ -240,65 +226,40 @@ contract CoreIssuance is | |||
uint256 naturalUnit = setToken.naturalUnit(); | |||
address[] memory components = setToken.getComponents(); | |||
uint256[] memory units = setToken.getUnits(); | |||
uint256[] memory requiredComponentQuantities = calculateTransferValues( |
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.
comment
( | ||
uint256[] memory decrementTokenOwnerValues, | ||
uint256[] memory depositValues | ||
) = calculateDepositAndDecrementQuantities( |
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.
comment
vaultBalance | ||
); | ||
} | ||
vault.batchDecrementTokenOwner( |
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.
comment
amountToDeposit | ||
); | ||
} | ||
ITransferProxy(state.transferProxy).batchTransfer( |
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.
comment
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.
LGTM
52d14d7
to
b4022c3
Compare