Fixed the approveAndCall method in the token contract, added implementation of the receiveApproval method in the escrow contract

nucypher/blockchain/eth/sol/source/contracts/MinersEscrow.sol

@@ -201,22 +201,62 @@ contract MinersEscrow is Issuer {
         token.safeTransferFrom(msg.sender, address(this), allValue);
     }
 
+    /**
+    * @notice Implementation of the receiveApproval(address,uint256,address,bytes) method
+    * (see NuCypherToken contract). Deposit all tokens that were approved to transfer
+    * @param _from Tokens owner
+    * @param _value Amount of token to deposit
+    * @param _tokenContract Token contract address
+    * @param _extraData Extra data - amount of periods during which tokens will be locked
+    **/
+    function receiveApproval(
+        address _from,
+        uint256 _value,
+        address _tokenContract,
+        bytes _extraData
+    )
+        external
+    {
+        require(_tokenContract == address(token) && msg.sender == address(token));
+        // copy first 32 bytes from _extraData. Position is calculated as
+        // 4 bytes method signature plus 32 * 3 bytes for previous params and
+        // addition 32 bytes to skip _extraData pointer
+        uint256 payloadSize;
+        uint256 payload;
+        assembly {
+            payloadSize := calldataload(0x84)
+            payload := calldataload(0xA4)
+        }
+        payload = payload >> 8*(32 - payloadSize);
+        deposit(_from, _value, payload);
+    }
+
     /**
     * @notice Deposit tokens
     * @param _value Amount of token to deposit
     * @param _periods Amount of periods during which tokens will be locked
     **/
-    function deposit(uint256 _value, uint256 _periods) public isInitialized {
+    function deposit(uint256 _value, uint256 _periods) public {
+        deposit(msg.sender, _value, _periods);
+    }
+
+    /**
+    * @notice Deposit tokens
+    * @param _owner Tokens owner
+    * @param _value Amount of token to deposit
+    * @param _periods Amount of periods during which tokens will be locked
+    **/
+    function deposit(address _owner, uint256 _value, uint256 _periods) internal isInitialized {
         require(_value != 0);
-        MinerInfo storage info = minerInfo[msg.sender];
+        MinerInfo storage info = minerInfo[_owner];
-        if (minerInfo[msg.sender].value == 0) {
+        if (info.lastActivePeriod == 0) {
-            miners.push(msg.sender);
+            miners.push(_owner);
             info.lastActivePeriod = getCurrentPeriod();
         }
         info.value = info.value.add(_value);
-        token.safeTransferFrom(msg.sender, address(this), _value);
+        token.safeTransferFrom(_owner, address(this), _value);
-        lock(_value, _periods);
+        lock(_owner, _value, _periods);
-        emit Deposited(msg.sender, _value, _periods);
+        emit Deposited(_owner, _value, _periods);
     }
 
     /**
@@ -225,11 +265,21 @@ contract MinersEscrow is Issuer {
     * @param _periods Amount of periods during which tokens will be locked
     **/
     function lock(uint256 _value, uint256 _periods) public onlyTokenOwner {
-        require(_value != 0 || _periods != 0);
+        lock(msg.sender, _value, _periods);
-        mint();
+    }
 
-        uint256 lockedTokens = getLockedTokens(msg.sender, 1);
+    /**
-        MinerInfo storage info = minerInfo[msg.sender];
+    * @notice Lock some tokens or increase lock
+    * @param _owner Tokens owner
+    * @param _value Amount of tokens which should lock
+    * @param _periods Amount of periods during which tokens will be locked
+    **/
+    function lock(address _owner, uint256 _value, uint256 _periods) internal {
+        require(_value != 0 || _periods != 0);
+        mint(_owner);
+
+        uint256 lockedTokens = getLockedTokens(_owner, 1);
+        MinerInfo storage info = minerInfo[_owner];
         require(_value <= token.balanceOf(address(this)) &&
             _value <= info.value.sub(lockedTokens) &&
             _value >= minAllowableLockedTokens &&
@@ -239,8 +289,8 @@ contract MinersEscrow is Issuer {
         uint256 currentPeriod = getCurrentPeriod();
         info.stakes.push(StakeInfo(currentPeriod.add(uint256(1)), currentPeriod.add(_periods), _value));
 
-        confirmActivity(_value + lockedTokens, _value);
+        confirmActivity(_owner, _value + lockedTokens, _value);
-        emit Locked(msg.sender, _value, currentPeriod + 1, currentPeriod + _periods);
+        emit Locked(_owner, _value, currentPeriod + 1, currentPeriod + _periods);
     }
 
     /**
@@ -294,23 +344,24 @@ contract MinersEscrow is Issuer {
 
     /**
     * @notice Confirm activity for future period
+    * @param _owner Tokens owner
     * @param _lockedValue Locked tokens in future period
     * @param _additional Additional locked tokens in future period.
     * Used only if the period has already been confirmed
     **/
-    function confirmActivity(uint256 _lockedValue, uint256 _additional) internal {
+    function confirmActivity(address _owner, uint256 _lockedValue, uint256 _additional) internal {
         require(_lockedValue > 0);
-        MinerInfo storage info = minerInfo[msg.sender];
+        MinerInfo storage info = minerInfo[_owner];
         uint256 nextPeriod = getCurrentPeriod() + 1;
 
         // update lockedValue if the period has already been confirmed
         if (info.confirmedPeriod1 == nextPeriod) {
             lockedPerPeriod[nextPeriod] = lockedPerPeriod[nextPeriod].add(_additional);
-            emit ActivityConfirmed(msg.sender, nextPeriod, _additional);
+            emit ActivityConfirmed(_owner, nextPeriod, _additional);
             return;
         } else if (info.confirmedPeriod2 == nextPeriod) {
             lockedPerPeriod[nextPeriod] = lockedPerPeriod[nextPeriod].add(_additional);
-            emit ActivityConfirmed(msg.sender, nextPeriod, _additional);
+            emit ActivityConfirmed(_owner, nextPeriod, _additional);
             return;
         }
 
@@ -326,14 +377,14 @@ contract MinersEscrow is Issuer {
             info.downtime.push(Downtime(info.lastActivePeriod + 1, currentPeriod));
         }
         info.lastActivePeriod = nextPeriod;
-        emit ActivityConfirmed(msg.sender, nextPeriod, _lockedValue);
+        emit ActivityConfirmed(_owner, nextPeriod, _lockedValue);
     }
 
     /**
     * @notice Confirm activity for future period and mine for previous period
     **/
     function confirmActivity() external onlyTokenOwner {
-        mint();
+        mint(msg.sender);
         MinerInfo storage info = minerInfo[msg.sender];
         uint256 currentPeriod = getCurrentPeriod();
         uint256 nextPeriod = currentPeriod + 1;
@@ -345,15 +396,23 @@ contract MinersEscrow is Issuer {
         }
 
         uint256 lockedTokens = getLockedTokens(msg.sender, 1);
-        confirmActivity(lockedTokens, 0);
+        confirmActivity(msg.sender, lockedTokens, 0);
     }
 
     /**
     * @notice Mint tokens for sender for previous periods if he locked his tokens and confirmed activity
     **/
     function mint() public onlyTokenOwner {
+        mint(msg.sender);
+    }
+
+    /**
+    * @notice Mint tokens for owner for previous periods if he locked his tokens and confirmed activity
+    * @param _owner Tokens owner
+    **/
+    function mint(address _owner) internal {
         uint256 previousPeriod = getCurrentPeriod().sub(uint(1));
-        MinerInfo storage info = minerInfo[msg.sender];
+        MinerInfo storage info = minerInfo[_owner];
 
         if (info.confirmedPeriod1 > previousPeriod &&
             info.confirmedPeriod2 > previousPeriod ||
@@ -379,51 +438,53 @@ contract MinersEscrow is Issuer {
         uint256 reward = 0;
         if (info.confirmedPeriod1 != EMPTY_CONFIRMED_PERIOD &&
             info.confirmedPeriod1 < info.confirmedPeriod2) {
-            reward = reward.add(mint(info, info.confirmedPeriod1, previousPeriod));
+            reward = reward.add(mint(_owner, info, info.confirmedPeriod1, previousPeriod));
             info.confirmedPeriod1 = EMPTY_CONFIRMED_PERIOD;
         } else if (info.confirmedPeriod2 != EMPTY_CONFIRMED_PERIOD &&
             info.confirmedPeriod2 < info.confirmedPeriod1) {
-            reward = reward.add(mint(info, info.confirmedPeriod2, previousPeriod));
+            reward = reward.add(mint(_owner, info, info.confirmedPeriod2, previousPeriod));
             info.confirmedPeriod2 = EMPTY_CONFIRMED_PERIOD;
         }
         if (info.confirmedPeriod2 <= previousPeriod &&
             info.confirmedPeriod2 > info.confirmedPeriod1) {
-            reward = reward.add(mint(info, info.confirmedPeriod2, previousPeriod));
+            reward = reward.add(mint(_owner, info, info.confirmedPeriod2, previousPeriod));
             info.confirmedPeriod2 = EMPTY_CONFIRMED_PERIOD;
         } else if (info.confirmedPeriod1 <= previousPeriod &&
             info.confirmedPeriod1 > info.confirmedPeriod2) {
-            reward = reward.add(mint(info, info.confirmedPeriod1, previousPeriod));
+            reward = reward.add(mint(_owner, info, info.confirmedPeriod1, previousPeriod));
             info.confirmedPeriod1 = EMPTY_CONFIRMED_PERIOD;
         }
 
         info.value = info.value.add(reward);
-        emit Mined(msg.sender, previousPeriod, reward);
+        emit Mined(_owner, previousPeriod, reward);
     }
 
     /**
     * @notice Calculate reward for one period
     **/
-    function mint(MinerInfo storage info, uint256 period, uint256 previousPeriod)
+    function mint(
+        address _owner,
+        MinerInfo storage _info,
+        uint256 _period,
+        uint256 _previousPeriod
+    )
         internal returns (uint256 reward)
     {
         uint256 amount;
-        for (uint256 i = 0; i < info.stakes.length; i++) {
+        for (uint256 i = 0; i < _info.stakes.length; i++) {
-            StakeInfo storage stake =  info.stakes[i];
+            StakeInfo storage stake =  _info.stakes[i];
-            if (stake.firstPeriod <= period &&
+            if (stake.firstPeriod <= _period &&
-                stake.lastPeriod >= period) {
+                stake.lastPeriod >= _period) {
-                (amount, info.decimals) = mint(
+                (amount, _info.decimals) = mint(
-                    previousPeriod,
+                    _previousPeriod,
                     stake.lockedValue,
-                    lockedPerPeriod[period],
+                    lockedPerPeriod[_period],
-                    stake.lastPeriod.sub(period),
+                    stake.lastPeriod.sub(_period),
-                    info.decimals);
+                    _info.decimals);
                 reward = reward.add(amount);
             }
         }
-        // TODO remove if
+        policyManager.updateReward(_owner, _period);
-        if (address(policyManager) != 0x0) {
-            policyManager.updateReward(msg.sender, period);
-        }
     }
 
     /**

nucypher/blockchain/eth/sol/source/contracts/NuCypherToken.sol

@@ -27,18 +27,31 @@ contract NuCypherToken is StandardToken, DetailedERC20('NuCypher', 'NU', 18), Bu
     * @notice Approves and then calls the receiving contract
     *
     * @dev call the receiveApproval function on the contract you want to be notified.
-    * This crafts the function signature manually so one doesn't have to include a contract in here just for this.
     * receiveApproval(address _from, uint256 _value, address _tokenContract, bytes _extraData)
-    * it is assumed that when does this that the call *should* succeed, otherwise one would use vanilla approve instead.
     **/
     function approveAndCall(address _spender, uint256 _value, bytes _extraData)
         public returns (bool success)
     {
         approve(_spender, _value);
-
+        TokenRecipient(_spender).receiveApproval(msg.sender, _value, address(this), _extraData);
-        require(_spender.call(bytes4(keccak256("receiveApproval(address,uint256,address,bytes)")),
-            msg.sender, _value, this, _extraData));
         return true;
     }
 
 }
+
+
+/**
+* @dev Interface to use the receiveApproval method
+**/
+contract TokenRecipient {
+
+    /**
+    * @notice Receives a notification of approval of the transfer
+    * @param _from Sender of approval
+    * @param _value  The amount of tokens to be spent
+    * @param _tokenContract Address of the token contract
+    * @param _extraData Extra data
+    **/
+    function receiveApproval(address _from, uint256 _value, address _tokenContract, bytes _extraData) external;
+
+}

tests/blockchain/eth/contracts/contracts/ReceiveApprovalMethodMock.sol

@@ -0,0 +1,28 @@
+pragma solidity ^0.4.23;
+
+
+/**
+* @notice Contract for using in token tests
+**/
+contract ReceiveApprovalMethodMock {
+
+    address public sender;
+    uint256 public value;
+    address public tokenContract;
+    bytes public extraData;
+
+    function receiveApproval(
+        address _from,
+        uint256 _value,
+        address _tokenContract,
+        bytes _extraData
+    )
+        external
+    {
+        sender = _from;
+        value = _value;
+        tokenContract = _tokenContract;
+        extraData = _extraData;
+    }
+
+}

tests/blockchain/eth/contracts/test_miners_escrow.py

@@ -48,6 +48,12 @@ def test_escrow(web3, chain, token, escrow_contract):
     divides_log = escrow.events.Divided.createFilter(fromBlock='latest')
     withdraw_log = escrow.events.Withdrawn.createFilter(fromBlock='latest')
 
+    policy_manager, _ = chain.provider.deploy_contract(
+        'PolicyManagerForMinersEscrowMock', token.address, escrow.address
+    )
+    tx = escrow.functions.setPolicyManager(policy_manager.address).transact()
+    chain.wait_for_receipt(tx)
+
     # Give Ursula and Ursula(2) some coins
     tx = token.functions.transfer(ursula1, 10000).transact({'from': creator})
     chain.wait_for_receipt(tx)
@@ -57,12 +63,12 @@ def test_escrow(web3, chain, token, escrow_contract):
     assert 10000 == token.functions.balanceOf(ursula2).call()
 
     # Ursula and Ursula(2) give Escrow rights to transfer
-    tx = token.functions.approve(escrow.address, 3000).transact({'from': ursula1})
+    tx = token.functions.approve(escrow.address, 1100).transact({'from': ursula1})
     chain.wait_for_receipt(tx)
-    assert 3000 == token.functions.allowance(ursula1, escrow.address).call()
+    assert 1100 == token.functions.allowance(ursula1, escrow.address).call()
-    tx = token.functions.approve(escrow.address, 1100).transact({'from': ursula2})
+    tx = token.functions.approve(escrow.address, 500).transact({'from': ursula2})
     chain.wait_for_receipt(tx)
-    assert 1100 == token.functions.allowance(ursula2, escrow.address).call()
+    assert 500 == token.functions.allowance(ursula2, escrow.address).call()
 
     # Ursula's withdrawal attempt won't succeed because nothing to withdraw
     with pytest.raises((TransactionFailed, ValueError)):
@@ -90,16 +96,25 @@ def test_escrow(web3, chain, token, escrow_contract):
 
     # Ursula can't deposit and lock too low value
     with pytest.raises((TransactionFailed, ValueError)):
-        tx = escrow.functions.deposit(1, 1).transact({'from': ursula1})
+        tx = escrow.functions.deposit(1, 10).transact({'from': ursula1})
+        chain.wait_for_receipt(tx)
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = token.functions.approveAndCall(escrow.address, 1, web3.toBytes(10)).transact({'from': ursula1})
         chain.wait_for_receipt(tx)
     # And can't deposit and lock too high value
     with pytest.raises((TransactionFailed, ValueError)):
-        tx = escrow.functions.deposit(1501, 1).transact({'from': ursula1})
+        tx = escrow.functions.deposit(1501, 10).transact({'from': ursula1})
+        chain.wait_for_receipt(tx)
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = token.functions.approveAndCall(escrow.address, 1501, web3.toBytes(10)).transact({'from': ursula1})
         chain.wait_for_receipt(tx)
     # And can't deposit for too short a period
     with pytest.raises((TransactionFailed, ValueError)):
         tx = escrow.functions.deposit(1000, 1).transact({'from': ursula1})
         chain.wait_for_receipt(tx)
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = token.functions.approveAndCall(escrow.address, 1000, web3.toBytes(1)).transact({'from': ursula1})
+        chain.wait_for_receipt(tx)
 
     # Ursula and Ursula(2) transfer some tokens to the escrow and lock them
     tx = escrow.functions.deposit(1000, 2).transact({'from': ursula1})
@@ -181,7 +196,7 @@ def test_escrow(web3, chain, token, escrow_contract):
     assert escrow.functions.getCurrentPeriod().call() + 1 == event_args['period']
     assert 1000 == event_args['value']
 
-    tx = escrow.functions.deposit(500, 2).transact({'from': ursula1})
+    tx = token.functions.approveAndCall(escrow.address, 500, web3.toBytes(2)).transact({'from': ursula1})
     chain.wait_for_receipt(tx)
     assert 2000 == token.functions.balanceOf(escrow.address).call()
     assert 8500 == token.functions.balanceOf(ursula1).call()
@@ -194,7 +209,10 @@ def test_escrow(web3, chain, token, escrow_contract):
 
     # But can't deposit too high value
     with pytest.raises((TransactionFailed, ValueError)):
-        tx = escrow.functions.deposit(1, 2).transact({'from': ursula1})
+        tx = escrow.functions.deposit(100, 2).transact({'from': ursula1})
+        chain.wait_for_receipt(tx)
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = token.functions.approveAndCall(escrow.address, 100, web3.toBytes(2)).transact({'from': ursula1})
         chain.wait_for_receipt(tx)
 
     # Wait 1 period and checks locking
@@ -216,7 +234,7 @@ def test_escrow(web3, chain, token, escrow_contract):
     assert 500 == event_args['value']
 
     # And Ursula can withdraw some tokens
-    tx =  escrow.functions.withdraw(100).transact({'from': ursula1})
+    tx = escrow.functions.withdraw(100).transact({'from': ursula1})
     chain.wait_for_receipt(tx)
     assert 1900 == token.functions.balanceOf(escrow.address).call()
     assert 8600 == token.functions.balanceOf(ursula1).call()
@@ -228,16 +246,16 @@ def test_escrow(web3, chain, token, escrow_contract):
 
     # But Ursula can't withdraw all without mining for locked value
     with pytest.raises((TransactionFailed, ValueError)):
-        tx =  escrow.functions.withdraw(1400).transact({'from': ursula1})
+        tx = escrow.functions.withdraw(1400).transact({'from': ursula1})
         chain.wait_for_receipt(tx)
 
     # And Ursula can't lock again too low value
     with pytest.raises((TransactionFailed, ValueError)):
-        tx =  escrow.functions.lock(1, 1).transact({'from': ursula1})
+        tx = escrow.functions.lock(1, 1).transact({'from': ursula1})
         chain.wait_for_receipt(tx)
 
     # Ursula can deposit and lock more tokens
-    tx = escrow.functions.deposit(500, 2).transact({'from': ursula1})
+    tx = token.functions.approveAndCall(escrow.address, 500, web3.toBytes(2)).transact({'from': ursula1})
     chain.wait_for_receipt(tx)
 
     events = activity_log.get_all_entries()
@@ -278,8 +296,8 @@ def test_escrow(web3, chain, token, escrow_contract):
     chain.time_travel(hours=1)
     assert 1100 == escrow.functions.getLockedTokens(ursula1).call()
 
-    # Ursula(2) increases lock by deposit more tokens
+    # Ursula(2) increases lock by deposit more tokens using approveAndCall
-    tx = escrow.functions.deposit(500, 2).transact({'from': ursula2})
+    tx = token.functions.approveAndCall(escrow.address, 500, web3.toBytes(2)).transact({'from': ursula2})
     chain.wait_for_receipt(tx)
     assert 500 == escrow.functions.getLockedTokens(ursula2).call()
     assert 1000 == escrow.functions.getLockedTokens(ursula2, 1).call()
@@ -607,12 +625,16 @@ def test_mining(web3, chain, token, escrow_contract):
     tx = escrow.functions.lock(500, 2).transact({'from': ursula2})
     chain.wait_for_receipt(tx)
 
-    # Ursula(2) mint only one period
+    # Ursula(2) mint only one period (by using deposit function)
     chain.time_travel(hours=5)
-    tx = escrow.functions.mint().transact({'from': ursula2})
+    tx = token.functions.approveAndCall(escrow.address, 100, web3.toBytes(2)).transact({'from': ursula2})
     chain.wait_for_receipt(tx)
     assert 1152 == escrow.functions.minerInfo(ursula1).call()[VALUE_FIELD]
-    assert 842 == escrow.functions.minerInfo(ursula2).call()[VALUE_FIELD]
+    assert 942 == escrow.functions.minerInfo(ursula2).call()[VALUE_FIELD]
+
+    period = escrow.functions.getCurrentPeriod().call() - 4
+    assert 2 == policy_manager.functions.getPeriodsLength(ursula2).call()
+    assert period == policy_manager.functions.getPeriod(ursula2, 1).call()
 
     events = mining_log.get_all_entries()
     assert 4 == len(events)
@@ -622,7 +644,8 @@ def test_mining(web3, chain, token, escrow_contract):
     assert escrow.functions.getCurrentPeriod().call() - 1 == event_args['period']
 
     # Ursula(2) can withdraw all
-    tx = escrow.functions.withdraw(842).transact({'from': ursula2})
+    chain.time_travel(hours=3)
+    tx = escrow.functions.withdraw(942).transact({'from': ursula2})
     chain.wait_for_receipt(tx)
     assert 10092 == token.functions.balanceOf(ursula2).call()
 
@@ -630,12 +653,12 @@ def test_mining(web3, chain, token, escrow_contract):
     assert 1 == len(events)
     event_args = events[0]['args']
     assert ursula2 == event_args['owner']
-    assert 842 == event_args['value']
+    assert 942 == event_args['value']
 
-    assert 3 == len(deposit_log.get_all_entries())
+    assert 4 == len(deposit_log.get_all_entries())
-    assert 5 == len(lock_log.get_all_entries())
+    assert 6 == len(lock_log.get_all_entries())
     assert 1 == len(divides_log.get_all_entries())
-    assert 6 == len(activity_log.get_all_entries())
+    assert 7 == len(activity_log.get_all_entries())
 
 
 @pytest.mark.slow

tests/blockchain/eth/contracts/test_token.py

@@ -18,13 +18,13 @@ def test_create_token(web3, chain):
     assert txhash is not None
 
     # Account balances
-    assert token.functions.balanceOf(creator).call() == 10 ** 9
+    assert 10 ** 9 == token.functions.balanceOf(creator).call()
-    assert token.functions.balanceOf(account1).call() == 0
+    assert 0 == token.functions.balanceOf(account1).call()
 
     # Basic properties
-    assert token.functions.name().call() == 'NuCypher'
+    assert 'NuCypher' == token.functions.name().call()
-    assert token.functions.decimals().call() == 18
+    assert 18 == token.functions.decimals().call()
-    assert token.functions.symbol().call() == 'NU'
+    assert 'NU' == token.functions.symbol().call()
 
     # Cannot send ethers to the contract
     with pytest.raises((TransactionFailed, ValueError)):
@@ -32,22 +32,54 @@ def test_create_token(web3, chain):
         chain.wait_for_receipt(tx)
 
     # Can transfer tokens
-    tx =  token.functions.transfer(account1, 10000).transact({'from': creator})
+    tx = token.functions.transfer(account1, 10000).transact({'from': creator})
     chain.wait_for_receipt(tx)
-    assert token.functions.balanceOf(account1).call() == 10000
+    assert 10000 == token.functions.balanceOf(account1).call()
-    assert token.functions.balanceOf(creator).call() == 10 ** 9 - 10000
+    assert 10 ** 9 - 10000 == token.functions.balanceOf(creator).call()
 
-    tx =  token.functions.transfer(account2, 10).transact({'from': account1})
+    tx = token.functions.transfer(account2, 10).transact({'from': account1})
     chain.wait_for_receipt(tx)
-    assert token.functions.balanceOf(account1).call() == 10000 - 10
+    assert 10000 - 10 == token.functions.balanceOf(account1).call()
-    assert token.functions.balanceOf(account2).call() == 10
+    assert 10 == token.functions.balanceOf(account2).call()
 
-    tx =  token.functions.transfer(token.address, 10).transact({'from': account1})
+    tx = token.functions.transfer(token.address, 10).transact({'from': account1})
     chain.wait_for_receipt(tx)
-    assert token.functions.balanceOf(token.address).call() == 10
+    assert 10 == token.functions.balanceOf(token.address).call()
 
     # Can burn own tokens
-    tx =  token.functions.burn(1).transact({'from': account2})
+    tx = token.functions.burn(1).transact({'from': account2})
     chain.wait_for_receipt(tx)
-    assert token.functions.balanceOf(account2).call() == 9
+    assert 9 == token.functions.balanceOf(account2).call()
-    assert token.functions.totalSupply().call() == 10 ** 9 - 1
+    assert 10 ** 9 - 1 == token.functions.totalSupply().call()
+
+
+def test_approve_and_call(web3, chain):
+    creator = web3.eth.accounts[0]
+    account1 = web3.eth.accounts[1]
+    account2 = web3.eth.accounts[2]
+
+    token, _ = chain.provider.deploy_contract('NuCypherToken', 10 ** 9)
+    mock, _ = chain.provider.deploy_contract('ReceiveApprovalMethodMock')
+
+    tx = token.functions.approve(account1, 100).transact({'from': creator})
+    chain.wait_for_receipt(tx)
+    assert 100 == token.functions.allowance(creator, account1).call()
+    assert 0 == token.functions.allowance(creator, account2).call()
+    assert 0 == token.functions.allowance(account1, creator).call()
+    assert 0 == token.functions.allowance(account1, account2).call()
+    assert 0 == token.functions.allowance(account2, account1).call()
+
+    tx = token.functions.transferFrom(creator, account2, 50).transact({'from': account1})
+    chain.wait_for_receipt(tx)
+    assert 50 == token.functions.balanceOf(account2).call()
+    assert 50 == token.functions.allowance(creator, account1).call()
+
+    tx = token.functions.approveAndCall(mock.address, 25, web3.toBytes(111)).transact({'from': account1})
+    chain.wait_for_receipt(tx)
+    assert 50 == token.functions.balanceOf(account2).call()
+    assert 50 == token.functions.allowance(creator, account1).call()
+    assert 25 == token.functions.allowance(account1, mock.address).call()
+    assert account1 == mock.functions.sender().call()
+    assert 25 == mock.functions.value().call()
+    assert token.address == mock.functions.tokenContract().call()
+    assert 111 == web3.toInt(mock.functions.extraData().call())