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 returnPartition function #680

Merged
merged 3 commits into from
Jun 12, 2019
Merged

Fix the returnPartition function #680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ff637c
fix the returnPartition function
SatyamSB May 25, 2019
b713748
minor fix
SatyamSB May 31, 2019
d4465e3
Merge branch 'dev-3.0.0' into task_3.11
satyamakgec May 31, 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
15 changes: 6 additions & 9 deletions contracts/tokens/SecurityToken.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,21 +527,18 @@ contract SecurityToken is ERC20, ReentrancyGuard, SecurityTokenStorage, IERC1594
// require(balanceOfByPartition(_partition, msg.sender) >= _value);
// NB - Above condition will be automatically checked using the executeTransfer() function execution.
// NB - passing `_additionalBalance` value is 0 because accessing the balance before transfer
uint256 balanceBeforeTransferLocked = _balanceOfByPartition(_partition, _to, 0);
uint256 lockedBalanceBeforeTransfer = _balanceOfByPartition(LOCKED, _to, 0);
Copy link

Choose a reason for hiding this comment

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

argument _partition is required to be UNLOCKED in _isValidPartition but it is not used in _balanceOfByPartition which instead hardcodes LOCKED. toPartition will always be UNLOCKED because of a bug in _returnPartition. finally an event gets emitted with the unused _partition (which needs to be UNLOCKED from the function call args.

I am struggling to understand the changes. @satyamakgec I would appreciate if you could elaborate on how this is working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the Idea is that tokenHolder only allows transacting the UNLOCKED token. _isValidPartition() is used to validate whether the partition is UNLOCKED or not. Then function checks the earlier LOCKED balance of the _to and after transfer, it will check the new LOCKED balance of _to so _returnPartition checks if the difference between both locked balance is equal to the value that is being transferred then it means all value is transferred to LOCKED partition. If not then it means some(or full) value transfferd to UNLOCKED partition of the receiver(_to).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And I agree on the 542 line change I should add the else statement.

_transferWithData(_from, _to, _value, _data);
// NB - passing `_additonalBalance` valie is 0 because balance of `_to` was updated in the transfer call
uint256 balanceAfterTransferLocked = _balanceOfByPartition(_partition, _to, 0);
toPartition = _returnPartition(balanceBeforeTransferLocked, balanceAfterTransferLocked, _value);
uint256 lockedBalanceAfterTransfer = _balanceOfByPartition(LOCKED, _to, 0);
toPartition = _returnPartition(lockedBalanceBeforeTransfer, lockedBalanceAfterTransfer, _value);
emit TransferByPartition(_partition, _operator, _from, _to, _value, _data, _operatorData);
}

function _returnPartition(uint256 _beforeBalance, uint256 _afterBalance, uint256 _value) internal pure returns(bytes32 toPartition) {
// return LOCKED only when the transaction `_value` should be equal to the change in the LOCKED partition
// balance otherwise return UNLOCKED
if (_afterBalance.sub(_beforeBalance) == _value)
toPartition = LOCKED;
// Returning the same partition UNLOCKED
toPartition = UNLOCKED;
toPartition = _afterBalance.sub(_beforeBalance) == _value ? LOCKED : UNLOCKED; // Returning the same partition UNLOCKED
}

///////////////////////
Expand Down Expand Up @@ -975,8 +972,8 @@ contract SecurityToken is ERC20, ReentrancyGuard, SecurityTokenStorage, IERC1594
bool success;
(success, esc, appStatusCode) = _canTransfer(_from, _to, _value, _data);
if (success) {
uint256 beforeBalance = _balanceOfByPartition(_partition, _to, 0);
uint256 afterbalance = _balanceOfByPartition(_partition, _to, _value);
uint256 beforeBalance = _balanceOfByPartition(LOCKED, _to, 0);
uint256 afterbalance = _balanceOfByPartition(LOCKED, _to, _value);
toPartition = _returnPartition(beforeBalance, afterbalance, _value);
}
return (esc, appStatusCode, toPartition);
Expand Down
59 changes: 58 additions & 1 deletion test/o_security_token.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
deployGPMAndVerifyed,
deployCappedSTOAndVerifyed,
deployMockRedemptionAndVerifyed,
deployMockWrongTypeRedemptionAndVerifyed
deployMockWrongTypeRedemptionAndVerifyed,
deployLockUpTMAndVerified
} from "./helpers/createInstances";

const MockSecurityTokenLogic = artifacts.require("./MockSecurityTokenLogic.sol");
Expand All @@ -20,6 +21,7 @@ const SecurityToken = artifacts.require("./SecurityToken.sol");
const GeneralTransferManager = artifacts.require("./GeneralTransferManager");
const GeneralPermissionManager = artifacts.require("./GeneralPermissionManager");
const MockRedemptionManager = artifacts.require("./MockRedemptionManager.sol");
const LockUpTransferManager = artifacts.require("./LockUpTransferManager.sol");
const STGetter = artifacts.require("./STGetter.sol");

const Web3 = require("web3");
Expand Down Expand Up @@ -61,6 +63,8 @@ contract("SecurityToken", async (accounts) => {

// Contract Instance Declaration
let I_GeneralPermissionManagerFactory;
let I_LockUpTransferManagerFactory;
let I_LockUpTransferManager;
let I_SecurityTokenRegistryProxy;
let I_GeneralTransferManagerFactory;
let I_GeneralPermissionManager;
Expand Down Expand Up @@ -160,6 +164,8 @@ contract("SecurityToken", async (accounts) => {
[I_GeneralPermissionManagerFactory] = await deployGPMAndVerifyed(account_polymath, I_MRProxied, 0);
// STEP 3: Deploy the CappedSTOFactory
[I_CappedSTOFactory] = await deployCappedSTOAndVerifyed(account_polymath, I_MRProxied, cappedSTOSetupCost);
// STEP 4(c): Deploy the LockUpVolumeRestrictionTMFactory
[I_LockUpTransferManagerFactory] = await deployLockUpTMAndVerified(account_polymath, I_MRProxied, 0);

// Printing all the contract addresses
console.log(`
Expand Down Expand Up @@ -2070,6 +2076,57 @@ contract("SecurityToken", async (accounts) => {
assert.equal(web3.utils.toUtf8(allDocs[0]), "doc4");
});
});

describe("Test cases for the returnPartition", async() => {
// It will work once the balanceOfByPartition function fixed added
it.skip("Should add the lockup Transfer manager and create a lockup for investor 1", async() => {

console.log(web3.utils.fromWei(await I_SecurityToken.balanceOf.call(account_investor1)));
console.log(web3.utils.fromWei(await I_SecurityToken.balanceOfByPartition.call(web3.utils.toHex("UNLOCKED"),account_investor1)));
console.log(web3.utils.fromWei(await I_SecurityToken.balanceOf.call(account_investor2)));

const tx = await I_SecurityToken.addModule(I_LockUpTransferManagerFactory.address, "0x", 0, 0, false, { from: token_owner });
assert.equal(tx.logs[2].args._types[0].toString(), transferManagerKey, "LockUpVolumeRestrictionTMFactory doesn't get deployed");
assert.equal(
web3.utils.toAscii(tx.logs[2].args._name)
.replace(/\u0000/g, ''),
"LockUpTransferManager",
"LockUpTransferManager module was not added"
);
I_LockUpTransferManager = await LockUpTransferManager.at(tx.logs[2].args._module);
let currentTime = new BN(await latestTime());
await I_LockUpTransferManager.addNewLockUpToUser(
account_investor2,
new BN(web3.utils.toWei("1000")),
currentTime.add(new BN(duration.seconds(1))),
new BN(duration.seconds(400000)),
new BN(duration.seconds(100000)),
web3.utils.fromAscii("a_lockup"),
{
from: token_owner
}
);

// transfer balance of Unlocked partition of invesotor 1 to 2
await increaseTime(10);

console.log(`UNLOCKED balance - ${web3.utils.fromWei(await I_SecurityToken.balanceOfByPartition.call(web3.utils.toHex("UNLOCKED"),account_investor2))}`);
console.log(`Locked Balance - ${web3.utils.fromWei(await I_SecurityToken.balanceOfByPartition.call(web3.utils.toHex("LOCKED"),account_investor2))}`);

let partition = await I_SecurityToken.transferByPartition.call(
web3.utils.toHex("UNLOCKED"),
account_investor2,
new BN(web3.utils.toWei("500")),
"0x0",
{
from: account_investor1
}
);
console.log(`UNLOCKED balance - ${web3.utils.fromWei(await I_SecurityToken.balanceOfByPartition.call(web3.utils.toHex("UNLOCKED"),account_investor2))}`);
console.log(`Locked Balance - ${web3.utils.fromWei(await I_SecurityToken.balanceOfByPartition.call(web3.utils.toHex("LOCKED"),account_investor2))}`);
assert.equal(web3.utils.hexToUtf8(partition), "LOCKED");
});
})
})
});

Expand Down