diff --git a/contracts/teller-network/Escrow.sol b/contracts/teller-network/Escrow.sol index 327d5742..fbd0c3d1 100644 --- a/contracts/teller-network/Escrow.sol +++ b/contracts/teller-network/Escrow.sol @@ -309,31 +309,35 @@ contract Escrow is Pausable, MessageSigned, Fees, Arbitrable { require(_escrowId < transactions.length, INVALID_ESCROW_ID); EscrowTransaction storage trx = transactions[_escrowId]; - require(trx.expirationTime < block.timestamp, "Transaction has not expired"); require(trx.status == EscrowStatus.FUNDED || trx.status == EscrowStatus.CREATED, "Only transactions in created or funded state can be canceled"); - _cancel(_escrowId, trx, false); + + address seller = metadataStore.getOfferOwner(trx.offerId); + require(trx.buyer == msg.sender || seller == msg.sender, "Function can only be invoked by the escrow buyer or seller"); + + if(trx.status == EscrowStatus.FUNDED){ + if(msg.sender == seller){ + require(trx.expirationTime < block.timestamp, "Can only be canceled after expiration"); + } + } + + _cancel(_escrowId, trx); } /** * @dev Cancel transaction and send funds back to seller * @param _escrowId Id of the escrow * @param trx EscrowTransaction with details of transaction to be marked as canceled - * @param _ignoreExpiration Determines if the require rule for expiration time will apply or not */ - function _cancel(uint _escrowId, EscrowTransaction storage trx, bool _ignoreExpiration) private { + function _cancel(uint _escrowId, EscrowTransaction storage trx) private { + address payable seller = metadataStore.getOfferOwner(trx.offerId); + if(trx.status == EscrowStatus.FUNDED){ - require(msg.sender == metadataStore.getOfferOwner(trx.offerId), "Only seller can cancel transaction"); - - if(!_ignoreExpiration){ - require(trx.expirationTime < block.timestamp, "Can only be canceled after expiration"); - } - address token = metadataStore.getAsset(trx.offerId); if(token == address(0)){ - msg.sender.transfer(trx.tokenAmount); + seller.transfer(trx.tokenAmount); } else { ERC20Token erc20token = ERC20Token(token); - require(erc20token.transfer(msg.sender, trx.tokenAmount)); + require(erc20token.transfer(seller, trx.tokenAmount), "Transfer failed"); } } @@ -352,7 +356,7 @@ contract Escrow is Pausable, MessageSigned, Fees, Arbitrable { require(_escrowId < transactions.length, INVALID_ESCROW_ID); EscrowTransaction storage trx = transactions[_escrowId]; require(trx.status == EscrowStatus.FUNDED, "Cannot withdraw from escrow in a stage different from FUNDED. Open a case"); - _cancel(_escrowId, trx, true); + _cancel(_escrowId, trx); } /** @@ -437,7 +441,7 @@ contract Escrow is Pausable, MessageSigned, Fees, Arbitrable { if(_releaseFunds){ _release(_escrowId, trx); } else { - _cancel(_escrowId, trx, true); + _cancel(_escrowId, trx); } } diff --git a/contracts/teller-network/MetadataStore.sol b/contracts/teller-network/MetadataStore.sol index 347dc316..5e66bc96 100644 --- a/contracts/teller-network/MetadataStore.sol +++ b/contracts/teller-network/MetadataStore.sol @@ -264,7 +264,7 @@ contract MetadataStore is Ownable { ); } - function getOfferOwner(uint256 _id) public view returns (address) { + function getOfferOwner(uint256 _id) public view returns (address payable) { require(_id < offers.length, "Invalid offer id"); return (offers[_id].owner); } diff --git a/test/escrow_spec.js b/test/escrow_spec.js index ef88de38..921af19d 100644 --- a/test/escrow_spec.js +++ b/test/escrow_spec.js @@ -215,6 +215,20 @@ contract("Escrow", function() { describe("Canceling an escrow", async () => { let created; + it("A seller cannot cancel an unexpired funded escrow", async () => { + await SNT.methods.approve(Escrow.options.address, feeAmount).send({from: accounts[0]}); + receipt = await Escrow.methods.create_and_fund(accounts[1], ethOfferId, value, expirationTime, 123, FIAT, 140).send({from: accounts[0], value}); + created = receipt.events.Created; + escrowId = created.returnValues.escrowId; + + try { + receipt = await Escrow.methods.cancel(escrowId).send({from: accounts[0]}); + assert.fail('should have reverted before'); + } catch (error) { + assert.strictEqual(error.message, "VM Exception while processing transaction: revert Can only be canceled after expiration"); + } + }); + it("A seller can cancel their ETH escrows", async () => { await SNT.methods.approve(Escrow.options.address, feeAmount).send({from: accounts[0]}); receipt = await Escrow.methods.create_and_fund(accounts[1], ethOfferId, value, expirationTime, 123, FIAT, 140).send({from: accounts[0], value}); @@ -258,6 +272,22 @@ contract("Escrow", function() { assert.equal(contractBalanceBeforeCreation, contractBalanceAfterCancelation, "Invalid contract balance"); }); + it("A buyer can cancel an escrow that hasn't been funded yet", async () => { + receipt = await Escrow.methods.create(accounts[1], ethOfferId, 123, FIAT, 140, [0], "L", "U").send({from: accounts[1]}); + receipt = await Escrow.methods.cancel(receipt.events.Created.returnValues.escrowId).send({from: accounts[1]}); + let Canceled = receipt.events.Canceled; + assert(!!Canceled, "Canceled() not triggered"); + }); + + it("A buyer can cancel an escrow that has been funded", async () => { + await SNT.methods.approve(Escrow.options.address, feeAmount).send({from: accounts[0]}); + receipt = await Escrow.methods.create_and_fund(accounts[1], ethOfferId, value, expirationTime, 123, FIAT, 140).send({from: accounts[0], value}); + escrowId = receipt.events.Created.returnValues.escrowId; + receipt = await Escrow.methods.cancel(escrowId).send({from: accounts[1]}); + let Canceled = receipt.events.Canceled; + assert(!!Canceled, "Canceled() not triggered"); + }); + it("An escrow can only be canceled once", async () => { await SNT.methods.approve(Escrow.options.address, feeAmount).send({from: accounts[0]}); receipt = await Escrow.methods.create_and_fund(accounts[1], ethOfferId, value, expirationTime, 123, FIAT, 140).send({from: accounts[0], value}); @@ -277,12 +307,28 @@ contract("Escrow", function() { escrowId = receipt.events.Created.returnValues.escrowId; try { - receipt = await Escrow.methods.cancel(escrowId).send({from: accounts[1]}); // Buyer tries to cancel + receipt = await Escrow.methods.cancel(escrowId).send({from: accounts[2]}); assert.fail('should have reverted before'); } catch (error) { TestUtils.assertJump(error); } }); + + it("A seller cannot cancel an escrow marked as paid", async () => { + await SNT.methods.approve(Escrow.options.address, feeAmount).send({from: accounts[0]}); + receipt = await Escrow.methods.create_and_fund(accounts[1], ethOfferId, value, expirationTime, 123, FIAT, 140).send({from: accounts[0], value}); + created = receipt.events.Created; + escrowId = created.returnValues.escrowId; + + receipt = await Escrow.methods.pay(escrowId).send({from: accounts[1]}); + + try { + receipt = await Escrow.methods.cancel(escrowId).send({from: accounts[0]}); + assert.fail('should have reverted before'); + } catch (error) { + assert.strictEqual(error.message, "VM Exception while processing transaction: revert Only transactions in created or funded state can be canceled"); + } + }); });