Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix the BalanceOfPartition audit item #679

Merged
merged 4 commits into from
Jun 13, 2019
Merged

Fix the BalanceOfPartition audit item #679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f0883f2
fix balance of partition function
SatyamSB May 24, 2019
8fa30e9
fix balance of Partition function
SatyamSB May 25, 2019
2556399
Merge branch 'dev-3.0.0' into task_3.10
satyamakgec May 25, 2019
ef06a10
return balance in the parent implementation of the getTokensByPartiti…
SatyamSB Jun 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions contracts/modules/TransferManager/TransferManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,14 @@ contract TransferManager is ITransferManager, Module {

/**
* @notice return the amount of tokens for a given user as per the partition
* @dev returning the balance of token holder against the UNLOCKED partition.
* This condition is valid only when the base contract doesn't implement the
* `getTokensByPartition()` function.
*/
function getTokensByPartition(bytes32 /*_partition*/, address /*_tokenHolder*/, uint256 /*_additionalBalance*/) external view returns(uint256) {
return 0;
function getTokensByPartition(bytes32 _partition, address _tokenHolder, uint256 /*_additionalBalance*/) external view returns(uint256) {
if (_partition == UNLOCKED)
return ISecurityToken(securityToken).balanceOf(_tokenHolder);
return uint256(0);
}

/**
Expand Down
13 changes: 10 additions & 3 deletions contracts/tokens/SecurityToken.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,13 +487,20 @@ contract SecurityToken is ERC20, ReentrancyGuard, SecurityTokenStorage, IERC1594
return _balanceOfByPartition(_partition, _tokenHolder, 0);
}

function _balanceOfByPartition(bytes32 _partition, address _tokenHolder, uint256 _additionalBalance) internal view returns(uint256 max) {
function _balanceOfByPartition(bytes32 _partition, address _tokenHolder, uint256 _additionalBalance) internal view returns(uint256 partitionBalance) {
address[] memory tms = modules[TRANSFER_KEY];
uint256 amount;
for (uint256 i = 0; i < tms.length; i++) {
amount = ITransferManager(tms[i]).getTokensByPartition(_partition, _tokenHolder, _additionalBalance);
if (max < amount) {
max = amount;
// In UNLOCKED partition we are returning the minimum of all the unlocked balances
if (_partition == UNLOCKED) {
if (amount < partitionBalance || i == 0)
Copy link

Choose a reason for hiding this comment

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

Some transfer managers do not implement this getTokensByPartition() properly and just return 0 (default implementation in baseclass). This might need to be fixed by implementing getTokensByPartition function in every TM or adding a flag that shows whether this function is implemented. Returning zero because it is not implemented will dominate the minimum calculation for UNLOCKED partitions.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tintinweb As above the default implementation in TM of getTokensByPartition now returns the user balance for UNLOCKED to address this.

partitionBalance = amount;
}
// In locked partition we are returning the maximum of all the Locked balances
else {
if (partitionBalance < amount)
partitionBalance = amount;
}
}
}
Expand Down