Skip to content
This repository was archived by the owner on Jan 18, 2023. It is now read-only.

Commit af0d279

Browse files
bweickasoong
authored andcommitted
Rewrote tests to be able to run and test when an invalid value is returned. Tidied up transferProxy specs.
1 parent 94ac948 commit af0d279

File tree

7 files changed

+369
-132
lines changed

7 files changed

+369
-132
lines changed

contracts/core/lib/TokenInteract.sol

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,68 +20,79 @@ import { GeneralERC20 } from "../../lib/GeneralERC20.sol";
2020
* @title TokenInteract
2121
* @author Set Protocol
2222
*
23-
* This library contains functions for interacting wtih ERC20 tokens
23+
* This library contains functions for interacting wtih ERC20 tokens, even those not fully compliant.
24+
* For all functions we will only accept tokens that return a null or true value, any other values will
25+
* cause the operation to revert.
26+
*
27+
* Inspired by dYdX Trading Inc's TokenInteract contract.
2428
*/
2529
library TokenInteract {
30+
31+
// ============ Constants ============
32+
string constant INVALID_RETURN_VALUE_TRANSFER = "Transferred token does not return null or true on successful trasnfer.";
33+
string constant INVALID_RETURN_VALUE_TRANSFERFROM = "Transferred token does not return null or true on successful transferFrom.";
34+
35+
// ============ Internal Functions ============
36+
2637
function balanceOf(
27-
address token,
28-
address owner
38+
address _tokenAddress,
39+
address _ownerAddress
2940
)
3041
internal
3142
view
3243
returns (uint256)
3344
{
34-
return GeneralERC20(token).balanceOf(owner);
45+
return GeneralERC20(_tokenAddress).balanceOf(_ownerAddress);
3546
}
3647

3748
function transfer(
38-
address token,
39-
address to,
40-
uint256 amount
49+
address _tokenAddress,
50+
address _to,
51+
uint256 _quantity
4152
)
4253
internal
4354
{
4455

45-
GeneralERC20(token).transfer(to, amount);
56+
GeneralERC20(_tokenAddress).transfer(_to, _quantity);
4657

4758
require(
4859
checkSuccess(),
49-
"TokenInteract#transfer: Transfer failed"
60+
INVALID_RETURN_VALUE_TRANSFER
5061
);
5162
}
5263

5364
function transferFrom(
54-
address token,
55-
address from,
56-
address to,
57-
uint256 amount
65+
address _tokenAddress,
66+
address _from,
67+
address _to,
68+
uint256 _quantity
5869
)
5970
internal
6071
{
6172

62-
GeneralERC20(token).transferFrom(from, to, amount);
73+
GeneralERC20(_tokenAddress).transferFrom(_from, _to, _quantity);
6374

6475
require(
6576
checkSuccess(),
66-
"TokenInteract#transferFrom: TransferFrom failed"
77+
INVALID_RETURN_VALUE_TRANSFERFROM
6778
);
6879
}
6980

70-
// ============ Private Helper-Functions ============
81+
// ============ Private Functions ============
7182

7283
/**
7384
* Checks the return value of the previous function up to 32 bytes. Returns true if the previous
74-
* function returned 0 bytes or 32 bytes that are not all-zero.
85+
* function returned 0 bytes or 1.
7586
*/
7687
function checkSuccess(
7788
)
7889
private
7990
pure
8091
returns (bool)
8192
{
93+
//default to failure
8294
uint256 returnValue = 0;
8395

84-
/* solium-disable-next-line security/no-inline-assembly */
8596
assembly {
8697
// check number of bytes returned from last function call
8798
switch returndatasize
@@ -91,7 +102,7 @@ library TokenInteract {
91102
returnValue := 1
92103
}
93104

94-
// 32 bytes returned: check if non-zero
105+
// 32 bytes returned
95106
case 0x20 {
96107
// copy 32 bytes into scratch space
97108
returndatacopy(0x0, 0x0, 0x20)
@@ -104,6 +115,7 @@ library TokenInteract {
104115
default { }
105116
}
106117

107-
return returnValue != 0;
118+
// check if returned value is one or nothing
119+
return returnValue == 1;
108120
}
109121
}

contracts/lib/GeneralERC20.sol

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,30 +18,31 @@ pragma solidity 0.4.24;
1818
* @title GeneralERC20
1919
* @author Set Protocol
2020
*
21-
* Interface for using ERC20 Tokens. We have to use a special interface to call ERC20 functions so
22-
* that we dont automatically revert when calling non-compliant tokens that have no return value for
23-
* transfer(), transferFrom(), or approve().
21+
* Interface for using ERC20 Tokens. This interface is needed to interact with tokens that are not
22+
* fully ERC20 compliant and return something other than true on successful transfers.
23+
*
24+
* Inspired by dYdX Trading Inc's TokenInteract contract.
2425
*/
2526
interface GeneralERC20 {
2627

2728
function balanceOf(
28-
address who
29+
address _owner
2930
)
3031
external
3132
view
3233
returns (uint256);
3334

3435
function transfer(
35-
address to,
36-
uint256 value
36+
address _to,
37+
uint256 _quantity
3738
)
3839
external;
3940

4041

4142
function transferFrom(
42-
address from,
43-
address to,
44-
uint256 value
43+
address _from,
44+
address _to,
45+
uint256 _quantity
4546
)
4647
external;
4748
}
Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
pragma solidity 0.4.24;
2+
3+
import "zeppelin-solidity/contracts/math/SafeMath.sol";
4+
5+
// mock class using BasicToken
6+
contract MockTokenInvalidReturn {
7+
8+
using SafeMath for uint;
9+
mapping(address => uint256) balances;
10+
mapping (address => mapping (address => uint256)) internal allowed;
11+
12+
uint256 public decimals;
13+
string public name;
14+
string public symbol;
15+
uint256 public totalSupply;
16+
17+
event Transfer(
18+
address indexed from,
19+
address indexed to,
20+
uint256 value
21+
);
22+
23+
event Approval(
24+
address indexed owner,
25+
address indexed spender,
26+
uint256 value
27+
);
28+
29+
constructor(
30+
address initialAccount,
31+
uint256 initialBalance,
32+
string _name,
33+
string _symbol,
34+
uint256 _decimals)
35+
public
36+
{
37+
balances[initialAccount] = initialBalance;
38+
totalSupply = initialBalance;
39+
name = _name;
40+
symbol = _symbol;
41+
decimals = _decimals;
42+
}
43+
44+
/**
45+
* @dev Total number of tokens in existence
46+
*/
47+
function totalSupply() public view returns (uint256) {
48+
return totalSupply;
49+
}
50+
51+
/**
52+
* @dev Transfer token for a specified address
53+
* @param _to The address to transfer to.
54+
* @param _value The amount to be transferred.
55+
*/
56+
function transfer(
57+
address _to,
58+
uint256 _value
59+
)
60+
public
61+
returns(uint256)
62+
{
63+
require(_to != address(0));
64+
require(_value <= balances[msg.sender]);
65+
66+
balances[msg.sender] = balances[msg.sender].sub(_value);
67+
balances[_to] = balances[_to].add(_value);
68+
emit Transfer(msg.sender, _to, _value);
69+
return 4;
70+
}
71+
72+
/**
73+
* @dev Gets the balance of the specified address.
74+
* @param _owner The address to query the the balance of.
75+
* @return An uint256 representing the amount owned by the passed address.
76+
*/
77+
function balanceOf(address _owner) public view returns (uint256) {
78+
return balances[_owner];
79+
}
80+
81+
/**
82+
* @dev Transfer tokens from one address to another
83+
* @param _from address The address which you want to send tokens from
84+
* @param _to address The address which you want to transfer to
85+
* @param _value uint256 the amount of tokens to be transferred
86+
*/
87+
function transferFrom(
88+
address _from,
89+
address _to,
90+
uint256 _value
91+
)
92+
public
93+
returns(uint256)
94+
{
95+
require(_to != address(0));
96+
require(_value <= balances[_from]);
97+
require(_value <= allowed[_from][msg.sender]);
98+
99+
balances[_from] = balances[_from].sub(_value);
100+
balances[_to] = balances[_to].add(_value);
101+
allowed[_from][msg.sender] = allowed[_from][msg.sender].sub(_value);
102+
emit Transfer(_from, _to, _value);
103+
return 4;
104+
}
105+
106+
/**
107+
* @dev Approve the passed address to spend the specified amount of tokens on behalf of msg.sender.
108+
* Beware that changing an allowance with this method brings the risk that someone may use both the old
109+
* and the new allowance by unfortunate transaction ordering. One possible solution to mitigate this
110+
* race condition is to first reduce the spender's allowance to 0 and set the desired value afterwards:
111+
* https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
112+
* @param _spender The address which will spend the funds.
113+
* @param _value The amount of tokens to be spent.
114+
*/
115+
function approve(
116+
address _spender,
117+
uint256 _value
118+
)
119+
public
120+
returns(uint256)
121+
{
122+
allowed[msg.sender][_spender] = _value;
123+
emit Approval(msg.sender, _spender, _value);
124+
return 4;
125+
}
126+
127+
/**
128+
* @dev Function to check the amount of tokens that an owner allowed to a spender.
129+
* @param _owner address The address which owns the funds.
130+
* @param _spender address The address which will spend the funds.
131+
* @return A uint256 specifying the amount of tokens still available for the spender.
132+
*/
133+
function allowance(
134+
address _owner,
135+
address _spender
136+
)
137+
public
138+
view
139+
returns (uint256)
140+
{
141+
return allowed[_owner][_spender];
142+
}
143+
144+
/**
145+
* @dev Increase the amount of tokens that an owner allowed to a spender.
146+
* approve should be called when allowed[_spender] == 0. To increment
147+
* allowed value is better to use this function to avoid 2 calls (and wait until
148+
* the first transaction is mined)
149+
* From MonolithDAO Token.sol
150+
* @param _spender The address which will spend the funds.
151+
* @param _addedValue The amount of tokens to increase the allowance by.
152+
*/
153+
function increaseApproval(
154+
address _spender,
155+
uint256 _addedValue
156+
)
157+
public
158+
returns(uint256)
159+
{
160+
allowed[msg.sender][_spender] = (
161+
allowed[msg.sender][_spender].add(_addedValue));
162+
emit Approval(msg.sender, _spender, allowed[msg.sender][_spender]);
163+
return 4;
164+
}
165+
166+
/**
167+
* @dev Decrease the amount of tokens that an owner allowed to a spender.
168+
* approve should be called when allowed[_spender] == 0. To decrement
169+
* allowed value is better to use this function to avoid 2 calls (and wait until
170+
* the first transaction is mined)
171+
* From MonolithDAO Token.sol
172+
* @param _spender The address which will spend the funds.
173+
* @param _subtractedValue The amount of tokens to decrease the allowance by.
174+
*/
175+
function decreaseApproval(
176+
address _spender,
177+
uint256 _subtractedValue
178+
)
179+
public
180+
returns(uint256)
181+
{
182+
uint256 oldValue = allowed[msg.sender][_spender];
183+
if (_subtractedValue > oldValue) {
184+
allowed[msg.sender][_spender] = 0;
185+
} else {
186+
allowed[msg.sender][_spender] = oldValue.sub(_subtractedValue);
187+
}
188+
emit Approval(msg.sender, _spender, allowed[msg.sender][_spender]);
189+
return 4;
190+
}
191+
}

0 commit comments

Comments
 (0)