Merge pull request #1 from status-im/no-double-spending

fix balance check

network-contracts/contracts/KeycardWallet.sol

@@ -11,6 +11,7 @@ contract KeycardWallet {
   uint256 public nonce;
   Settings public settings;
   mapping(address => uint) public pendingWithdrawals;
+  uint256 public totalPendingWithdrawals;
 
   struct Settings {
     uint256 maxTxValue;
@@ -32,6 +33,7 @@ contract KeycardWallet {
     keycard = _keycard;
     settings.maxTxValue = _maxTxValue;
     nonce = 0;
+    totalPendingWithdrawals = 0;
   }
 
   function setKeycard(address _keycard) public onlyOwner {
@@ -81,16 +83,18 @@ contract KeycardWallet {
     // check that the nonce is valid
     require(nonce == _nonce, "invalid nonce");
 
-    // check that balance is enough for this payment
-    require((address(this).balance - _value) >= 0, "balance is not enough");
-
     // check that _value is not greater than settings.maxTxValue
     require(_value <= settings.maxTxValue, "amount not allowed");
 
+    int256 availableBalance = int256(address(this).balance - totalPendingWithdrawals - _value);
+    // check that balance is enough for this payment
+    require(availableBalance >= 0, "balance is not enough");
+
     // increment nonce
     nonce++;
 
     // add pendingWithdrawal
+    totalPendingWithdrawals += _value;
     pendingWithdrawals[_to] += _value;
 
     emit NewPaymentRequest(_nonce, _to, _value);
@@ -101,6 +105,8 @@ contract KeycardWallet {
     require(amount > 0, "no pending withdrawal");
 
     pendingWithdrawals[msg.sender] = 0;
+    totalPendingWithdrawals -= amount;
+
     msg.sender.transfer(amount);
     emit NewWithdrawal(msg.sender, amount);
   }

network-contracts/test/KeycardWallet_spec.js

@@ -197,13 +197,13 @@ contract('KeycardWallet', () => {
     const settingsBefore = await KeycardWallet.methods.settings().call();
     assert.equal(settingsBefore, 0);
 
-    const setSettings = KeycardWallet.methods.setSettings(100);
+    const setSettings = KeycardWallet.methods.setSettings(999);
     await setSettings.send({
       from: owner
     });
 
     const currentSettings = await KeycardWallet.methods.settings().call();
-    assert.equal(currentSettings, 100);
+    assert.equal(currentSettings, 999);
   });
 
   it('requestPayment with value greater than maxTxValue', async () => {
@@ -228,6 +228,28 @@ contract('KeycardWallet', () => {
     }
   });
 
+  it('requestPayment with value greater than balance', async () => {
+    const nonce = await KeycardWallet.methods.nonce().call();
+    const to = merchant;
+    const value = 101;
+
+    const message = await web3.utils.soliditySha3(nonce, to, value);
+    const sig = await web3.eth.sign(message, keycard);
+    const hashToSign = await web3.eth.accounts.hashMessage(message);
+
+    const requestPayment = KeycardWallet.methods.requestPayment(hashToSign, sig, nonce, to, value);
+    try {
+      const estimatedGas = await requestPayment.estimateGas();
+      const receipt = await requestPayment.send({
+        from: merchant,
+        gas: estimatedGas
+      });
+     assert.fail("requestPayment should have failed");
+    } catch (err) {
+      assert.equal(getErrorReason(err), "balance is not enough");
+    }
+  });  
+
   it('requestPayment', async () => {
     const nonce = await KeycardWallet.methods.nonce().call();
     const to = merchant;
@@ -251,8 +273,34 @@ contract('KeycardWallet', () => {
 
     const pendingWithdrawal = await KeycardWallet.methods.pendingWithdrawals(merchant).call();
     assert.equal(pendingWithdrawal, value);
+
+    const totalPendingWithdrawal = await KeycardWallet.methods.totalPendingWithdrawals().call();
+    assert.equal(totalPendingWithdrawal, value);    
   });
 
+  it('requestPayment with value greater than available balance', async () => {
+    const nonce = await KeycardWallet.methods.nonce().call();
+    const to = merchant;
+    const value = 100;
+
+    const message = await web3.utils.soliditySha3(nonce, to, value);
+    const sig = await web3.eth.sign(message, keycard);
+    const hashToSign = await web3.eth.accounts.hashMessage(message);
+
+    const requestPayment = KeycardWallet.methods.requestPayment(hashToSign, sig, nonce, to, value);
+    try {
+      const estimatedGas = await requestPayment.estimateGas();
+      const receipt = await requestPayment.send({
+        from: merchant,
+        gas: estimatedGas
+      });
+     
+      assert.fail("requestPayment should have failed");
+    } catch (err) {
+      assert.equal(getErrorReason(err), "balance is not enough");
+    }
+  });  
+
   it('withdraw from address without pendingWithdrawal', async () => {
     const withdrawalValue = 1;
     const pendingWithdrawalBefore = await KeycardWallet.methods.pendingWithdrawals(thief).call();
@@ -285,6 +333,9 @@ contract('KeycardWallet', () => {
     const pendingWithdrawalAfter = await KeycardWallet.methods.pendingWithdrawals(merchant).call();
     assert.equal(pendingWithdrawalAfter, 0);
 
+    const totalPendingWithdrawalAfter = await KeycardWallet.methods.totalPendingWithdrawals().call();
+    assert.equal(totalPendingWithdrawalAfter, 0);  
+
     const expectedMerchantBalance = (new web3.utils.BN(merchantBalanceBefore)).sub(fullTxPrice).add(new web3.utils.BN(withdrawalValue));
     const merchantBalanceAfter = await web3.eth.getBalance(merchant);
     assert.deepStrictEqual(new web3.utils.BN(merchantBalanceAfter), expectedMerchantBalance);