From 8523af956b7efdac642a2715e7e641fc00bc50a7 Mon Sep 17 00:00:00 2001 From: elenadimitrova Date: Tue, 7 Apr 2020 12:18:15 +0300 Subject: [PATCH] Check the approveAndCall hasn't spent more than the given amount --- contracts/modules/common/BaseTransfer.sol | 2 ++ test/transferManager.js | 42 +++++++++++++++++++++-- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/contracts/modules/common/BaseTransfer.sol b/contracts/modules/common/BaseTransfer.sol index 38029e8f8..7402bbe52 100644 --- a/contracts/modules/common/BaseTransfer.sol +++ b/contracts/modules/common/BaseTransfer.sol @@ -118,6 +118,8 @@ contract BaseTransfer is BaseModule { // Calculate the approved amount that was spent after the call uint256 unusedAllowance = ERC20(_token).allowance(address(_wallet), _spender); uint256 usedAllowance = SafeMath.sub(totalAllowance, unusedAllowance); + // Ensure the amount spent does not exceed the amount approved for this call + require(usedAllowance <= _amount, "BT: insufficient amount for call"); // Restore the original allowance amount. methodData = abi.encodeWithSignature("approve(address,uint256)", _spender, existingAllowance); diff --git a/test/transferManager.js b/test/transferManager.js index 1c3256606..583d82184 100644 --- a/test/transferManager.js +++ b/test/transferManager.js @@ -526,18 +526,54 @@ describe("TransferManager", function () { it("should restore existing approved amount after call", async () => { await transferModule.from(owner).approveToken(wallet.contractAddress, erc20.contractAddress, contract.contractAddress, 10); - const dataToTransfer = contract.contract.interface.functions.setStateAndPayToken.encode([3, erc20.contractAddress, 1]); + const dataToTransfer = contract.contract.interface.functions.setStateAndPayToken.encode([3, erc20.contractAddress, 5]); await transferModule.from(owner).approveTokenAndCallContract( wallet.contractAddress, erc20.contractAddress, contract.contractAddress, - 1, + 5, contract.contractAddress, dataToTransfer, ); const approval = await erc20.allowance(wallet.contractAddress, contract.contractAddress); - // Initial approval of 10 is restored, after approving and spending 1 + + // Initial approval of 10 is restored, after approving and spending 5 assert.equal(approval.toNumber(), 10); + + const erc20Balance = await erc20.balanceOf(contract.contractAddress); + assert.equal(erc20Balance.toNumber(), 5, "the contract should have transfered the tokens"); + }); + + it("should be able to spend less than approved in call", async () => { + await transferModule.from(owner).approveToken(wallet.contractAddress, erc20.contractAddress, contract.contractAddress, 10); + const dataToTransfer = contract.contract.interface.functions.setStateAndPayToken.encode([3, erc20.contractAddress, 4]); + await transferModule.from(owner).approveTokenAndCallContract( + wallet.contractAddress, + erc20.contractAddress, + contract.contractAddress, + 5, + contract.contractAddress, + dataToTransfer, + ); + const approval = await erc20.allowance(wallet.contractAddress, contract.contractAddress); + // Initial approval of 10 is restored, after approving and spending 4 + assert.equal(approval.toNumber(), 10); + + const erc20Balance = await erc20.balanceOf(contract.contractAddress); + assert.equal(erc20Balance.toNumber(), 4, "the contract should have transfered the tokens"); + }); + + it("should not be able to spend more than approved in call", async () => { + await transferModule.from(owner).approveToken(wallet.contractAddress, erc20.contractAddress, contract.contractAddress, 10); + const dataToTransfer = contract.contract.interface.functions.setStateAndPayToken.encode([3, erc20.contractAddress, 6]); + await assert.revertWith(transferModule.from(owner).approveTokenAndCallContract( + wallet.contractAddress, + erc20.contractAddress, + contract.contractAddress, + 5, + contract.contractAddress, + dataToTransfer, + ), "BT: insufficient amount for call"); }); it("should approve the token and call the contract when the token is above the limit and the contract is whitelisted ", async () => {