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.

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

Removes activeStakeRequestCount from OSTComposer #775

Merged
merged 4 commits into from
Jun 21, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
23 changes: 4 additions & 19 deletions contracts/gateway/OSTComposer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import "./StakerProxy.sol";
import "./EIP20GatewayInterface.sol";
import "../lib/EIP20Interface.sol";
import "../lib/Mutex.sol";
import "openzeppelin-solidity/contracts/math/SafeMath.sol";
import "./ComposerInterface.sol";

/**
Expand All @@ -36,11 +35,6 @@ import "./ComposerInterface.sol";
*/
contract OSTComposer is Organized, Mutex, ComposerInterface {

/* Usings */

using SafeMath for uint256;


/* Constants */

bytes32 constant public STAKEREQUEST_INTENT_TYPEHASH = keccak256(
Expand Down Expand Up @@ -119,9 +113,6 @@ contract OSTComposer is Organized, Mutex, ComposerInterface {
/* Mapping of staker addresses to their StakerProxy. */
mapping (address => StakerProxy) public stakerProxies;

/* Stores number of all active stake request per staker. */
mapping(address => uint256) public activeStakeRequestCount;

/* Stores all the parameters of stake request based on stake request hash. */
mapping (bytes32 => StakeRequest) public stakeRequests;

Expand Down Expand Up @@ -234,7 +225,6 @@ contract OSTComposer is Organized, Mutex, ComposerInterface {
staker: msg.sender,
gateway: _gateway
});
activeStakeRequestCount[msg.sender] = activeStakeRequestCount[msg.sender].add(1);

EIP20Interface valueToken = _gateway.valueToken();

Expand Down Expand Up @@ -285,8 +275,10 @@ contract OSTComposer is Organized, Mutex, ComposerInterface {
EIP20GatewayInterface gateway = stakeRequest.gateway;

StakerProxy stakerProxy = stakerProxies[stakeRequest.staker];

activeStakeRequestCount[stakeRequest.staker] = activeStakeRequestCount[stakeRequest.staker].sub(1);
require(
address(stakerProxy) != address(0),
"StakerProxy address is null."
);

EIP20Interface valueToken = gateway.valueToken();
require(
Expand Down Expand Up @@ -369,12 +361,6 @@ contract OSTComposer is Organized, Mutex, ComposerInterface {
external
onlyStakerProxy(_owner)
{
// Verify if any previous stake requests are pending.
require(
activeStakeRequestCount[_owner] == 0,
"Stake request is active on gateways."
);

// Resetting the proxy address of the staker.
delete stakerProxies[_owner];
}
Expand All @@ -396,7 +382,6 @@ contract OSTComposer is Organized, Mutex, ComposerInterface {
{
StakeRequest storage stakeRequest = stakeRequests[_stakeRequestHash];
address staker = stakeRequests[_stakeRequestHash].staker;
activeStakeRequestCount[staker] = activeStakeRequestCount[staker].sub(1);

EIP20GatewayInterface gateway = stakeRequests[_stakeRequestHash].gateway;
uint256 amount = stakeRequests[_stakeRequestHash].amount;
Expand Down
16 changes: 0 additions & 16 deletions contracts/test/gateway/TestOSTComposer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -120,22 +120,6 @@ contract TestOSTComposer is OSTComposer {
stakerProxies[_staker] = _stakerProxy;
}

/**
* @notice This is used for testing. It is used to set the
* activeStakeRequestCount storage.
*
* @param _staker Address of the staker.
* @param _count Count of the active gateway request by the staker.
*/
function setActiveStakeRequestCount(
address _staker,
uint256 _count
)
external
{
activeStakeRequestCount[_staker] = _count;
}

/**
* @notice This is used for testing. It is used to set the stakerProxy
* storage by creating the StakerProxy contract address for the staker.
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
"web3": "1.0.0-beta.36"
},
"scripts": {
"update": "git submodule update --init --recursive && npm ci && cd contracts/utilitytoken && npm ci && cd -",
"compile": "truffle compile",
"compile-all": "truffle compile --all",
"compile:ts": "tsc --target es5",
Expand Down
43 changes: 13 additions & 30 deletions test/gateway/ost_composer/accept_stake.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ const OSTComposer = artifacts.require('TestOSTComposer');
const SpyToken = artifacts.require('SpyToken');
const Gateway = artifacts.require('SpyEIP20Gateway');
const MockOrganization = artifacts.require('MockOrganization');

const BN = require('bn.js');
const Utils = require('../../test_lib/utils');
const web3 = require('../../test_lib/web3');

contract('OSTComposer.acceptStakeRequest() ', (accounts) => {
let organization;
Expand All @@ -33,7 +35,6 @@ contract('OSTComposer.acceptStakeRequest() ', (accounts) => {
let stakeHash;
let hashLock;
let worker;
let activeGatewayCountForStaker;
let stakerProxy;

beforeEach(async () => {
Expand Down Expand Up @@ -61,17 +62,11 @@ contract('OSTComposer.acceptStakeRequest() ', (accounts) => {
stakeRequest.nonce,
stakeHash,
);
activeGatewayCountForStaker = 1;
stakerProxy = await ostComposer.generateStakerProxy.call(stakeRequest.staker);
await ostComposer.generateStakerProxy(stakeRequest.staker);
await ostComposer.setActiveStakeRequestCount(stakeRequest.staker, activeGatewayCountForStaker);
});

it('should be able to successfully accept stake', async () => {
const activeGatewayRequestCountBeforeAS = await ostComposer.activeStakeRequestCount(
stakeRequest.staker,
);

const response = await ostComposer.acceptStakeRequest(
stakeHash,
hashLock,
Expand All @@ -80,20 +75,8 @@ contract('OSTComposer.acceptStakeRequest() ', (accounts) => {

assert.strictEqual(response.receipt.status, true, 'Receipt status is unsuccessful');

const activeGatewayRequestCountAfterAS = await ostComposer.activeStakeRequestCount(
stakeRequest.staker,
);

assert.strictEqual(
(activeGatewayRequestCountBeforeAS.sub(activeGatewayRequestCountAfterAS)).eqn(1),
true,
`Expected active gateway request for ${stakeRequest.staker} is `
+ `${activeGatewayRequestCountBeforeAS.sub(activeGatewayRequestCountAfterAS)}`
+ `but got ${activeGatewayRequestCountAfterAS}`,
);

// Verifying the storage stakeRequestHash and stakeRequests references. After the acceptStakeRequest
// is successful, these two storage references are deleted.
// Verifying the storage stakeRequestHash and stakeRequests references.
// After the acceptStakeRequest is successful, these two storage references are deleted.
const stakeRequestHashStorage = await ostComposer.stakeRequestHashes.call(
stakeRequest.staker,
gateway.address,
Expand Down Expand Up @@ -154,36 +137,36 @@ contract('OSTComposer.acceptStakeRequest() ', (accounts) => {
assert.strictEqual(
stakeRequest.amount.eq(await gateway.amount.call()),
true,
"The spy did not record the amount correctly",
'The spy did not record the amount correctly',
);

assert.strictEqual(
stakeRequest.beneficiary,
await gateway.beneficiary(),
"The spy did not record the beneficiary correctly",
'The spy did not record the beneficiary correctly',
);

assert.strictEqual(
stakeRequest.gasPrice.eq(await gateway.gasPrice()),
true,
"The spy did not record the gasPrice correctly",
'The spy did not record the gasPrice correctly',
);

assert.strictEqual(
stakeRequest.gasLimit.eq(await gateway.gasLimit()),
true,
"The spy did not record the gasLimit correctly",
'The spy did not record the gasLimit correctly',
);

assert.strictEqual(
stakeRequest.nonce.eq(await gateway.nonce()),
true,
"The spy did not record the nonce correctly",
'The spy did not record the nonce correctly',
);

});

it('should be able to transfer the value tokens to staker\'s staker proxy contract address', async () => {
it('should be able to transfer the value tokens to staker\'s staker '
+ 'proxy contract address', async () => {
const valueToken = await SpyToken.at(await gateway.valueToken.call());
await ostComposer.acceptStakeRequest(
stakeHash,
Expand All @@ -204,7 +187,8 @@ contract('OSTComposer.acceptStakeRequest() ', (accounts) => {
);
});

it('should be able to transfer the base tokens to staker\'s staker proxy contract address', async () => {
it('should be able to transfer the base tokens to staker\'s staker '
+ 'proxy contract address', async () => {
const baseToken = await SpyToken.at(await gateway.baseToken.call());

await ostComposer.acceptStakeRequest(
Expand Down Expand Up @@ -232,7 +216,6 @@ contract('OSTComposer.acceptStakeRequest() ', (accounts) => {
`Expected amount to be returned to ${stakeRequest.staker} is ${bounty} `
+ `but got ${await baseToken.transferAmount.call()}`,
);

});

it('should fail when stake request hash is null', async () => {
Expand Down
12 changes: 0 additions & 12 deletions test/gateway/ost_composer/reject_stake_request.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ contract('OSTComposer.rejectStakeRequest() ', (accounts) => {
gatewayCount = new BN(2);
stakerProxy = await StakerProxy.new(stakeRequest.staker);
await ostComposer.setStakerProxy(stakeRequest.staker, stakerProxy.address);
await ostComposer.setActiveStakeRequestCount(stakeRequest.staker, gatewayCount);
});

it('should be able to successfully reject stake', async () => {
Expand All @@ -75,17 +74,6 @@ contract('OSTComposer.rejectStakeRequest() ', (accounts) => {

assert.strictEqual(response.receipt.status, true, 'Receipt status is unsuccessful');

const activeGatewayRequestCount = await ostComposer.activeStakeRequestCount(
stakeRequest.staker,
);

assert.strictEqual(
activeGatewayRequestCount.eq(gatewayCount.subn(1)),
true,
`Expected active gateway request for ${stakeRequest.staker} is ${gatewayCount.subn(1)}`
+ `but got ${activeGatewayRequestCount}`,
);

// Verifying the storage stakeRequestHash and stakeRequests. After the rejectStakeRequest
// is successful, these two storage references are cleared.
const stakeRequestHashStorage = await ostComposer.stakeRequestHashes.call(
Expand Down
25 changes: 0 additions & 25 deletions test/gateway/ost_composer/remove_staker_proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ contract('OSTComposer.removeStakerProxy() ', (accounts) => {
expectedActiveGatewayCount = new BN(0);
stakerProxy = accounts[10];
await ostComposer.setStakerProxy(stakeRequest.staker, stakerProxy);
await ostComposer.setActiveStakeRequestCount(stakeRequest.staker, expectedActiveGatewayCount);
});

it('should be able to successfully remove staker proxy', async () => {
Expand All @@ -52,17 +51,6 @@ contract('OSTComposer.removeStakerProxy() ', (accounts) => {

assert.strictEqual(response.receipt.status, true, 'Receipt status is unsuccessful');

const activeGatewayRequestCount = await ostComposer.activeStakeRequestCount(
stakeRequest.staker,
);

assert.strictEqual(
activeGatewayRequestCount.eq(expectedActiveGatewayCount),
true,
`Expected active gateway request for ${stakeRequest.staker} is ${expectedActiveGatewayCount}`
+ `but got ${activeGatewayRequestCount}`,
);

const stakerProxyAddress = await ostComposer.stakerProxies.call(stakeRequest.staker);
assert.strictEqual(
stakerProxyAddress,
Expand Down Expand Up @@ -92,17 +80,4 @@ contract('OSTComposer.removeStakerProxy() ', (accounts) => {
'Caller is invalid proxy address.',
);
});

it('should fail when previous stake request is active for a staker at the gateway', async () => {
// It would fail for any value greater than 0.
await ostComposer.setActiveStakeRequestCount(stakeRequest.staker, 1);

await Utils.expectRevert(
ostComposer.removeStakerProxy(
stakeRequest.staker,
{ from: stakerProxy },
),
'Stake request is active on gateways.',
);
});
});
16 changes: 0 additions & 16 deletions test/gateway/ost_composer/request_stake.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ contract('OSTComposer.requestStake() ', (accounts) => {
});

it('should be able to successfully request stake', async () => {
const beforeRSActiveGatewayRequestCount = await ostComposer.activeStakeRequestCount(
stakeRequest.staker,
);

const response = await ostComposer.requestStake(
stakeRequest.amount,
stakeRequest.beneficiary,
Expand All @@ -78,18 +74,6 @@ contract('OSTComposer.requestStake() ', (accounts) => {

assert.strictEqual(response.receipt.status, true, 'Receipt status is unsuccessful');

const afterRSActiveGatewayRequestCount = await ostComposer.activeStakeRequestCount(
stakeRequest.staker,
);

assert.strictEqual(
(afterRSActiveGatewayRequestCount.sub(beforeRSActiveGatewayRequestCount)).eqn(1),
true,
`Expected active gateway request for ${stakeRequest.staker} is `
+ `${afterRSActiveGatewayRequestCount.sub(beforeRSActiveGatewayRequestCount)} but got `
+ `${afterRSActiveGatewayRequestCount}`,
);

// Verifying the storage `stakeRequestHash` and `stakeRequests`.
const stakeIntentTypeHash = getStakeRequestHash(stakeRequest, gateway);

Expand Down
12 changes: 0 additions & 12 deletions test/gateway/ost_composer/revoke_stake_request.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ contract('OSTComposer.revokeStakeRequest() ', (accounts) => {
stakeHash,
);
gatewayCount = new BN(2);
await ostComposer.setActiveStakeRequestCount(stakeRequest.staker, gatewayCount);
});

it('should be able to successfully revoke stake request', async () => {
Expand All @@ -67,17 +66,6 @@ contract('OSTComposer.revokeStakeRequest() ', (accounts) => {

assert.strictEqual(response.receipt.status, true, 'Receipt status is unsuccessful');

const activeGatewayRequestCount = await ostComposer.activeStakeRequestCount(
stakeRequest.staker,
);

assert.strictEqual(
activeGatewayRequestCount.eq(gatewayCount.subn(1)),
true,
`Expected active gateway request for ${stakeRequest.staker} is ${gatewayCount.subn(1)}`
+ `but got ${activeGatewayRequestCount}`,
);

// Verifying the storage stakeRequestHash and stakeRequests. After the revokeStakeRequest
// is successful, these two storage references are cleared.
const stakeRequestHashStorage = await ostComposer.stakeRequestHashes.call(
Expand Down