diff --git a/src/ArbitrationModule.sol b/src/ArbitrationModule.sol index 4e14eb5..93ca3a5 100644 --- a/src/ArbitrationModule.sol +++ b/src/ArbitrationModule.sol @@ -59,10 +59,13 @@ contract ArbitrationModule is IArbitrationModule */ //ensure that escrow is valid + //EXCEPTION: InvalidEscrow require(polyEscrow.getEscrow(escrowId).id == escrowId, "InvalidEscrow"); + //EXCEPTION: Unauthorized require (_canProposeArbitration(polyEscrow, escrowId, msg.sender), "Unauthorized"); + //EXCEPTION: InvalidEscrowState require(_escrowStateIsValid(polyEscrow, escrowId), "InvalidEscrowState"); // Check if maximum number of active arbitration cases has been reached @@ -79,12 +82,14 @@ contract ArbitrationModule is IArbitrationModule proposals[propId].status = ArbitrationStatus.ACTIVE; proposals[propId].votesAgainst = 0; proposals[propId].autoExecute = autoExecute; + proposals[propId].proposer = msg.sender; //TODO: validate the amount (should be realistic and related to amount in escrow) - //record proposer as an automatic yes vote - proposals[propId].votesFor = 1; - proposalVotes[propId][msg.sender] = true; + //record proposer as an automatic yes vote, if proposer is a voter + if (_canVoteArbitration(polyEscrow, escrowId, msg.sender)) { + _voteArbitration(polyEscrow, proposals[propId], true); + } // Increment counters proposalCount++; @@ -99,6 +104,7 @@ contract ArbitrationModule is IArbitrationModule // WHO can vote on arbitration? arbiters only //get the arbitration proposal + //EXCEPTION: InvalidProposal ArbitrationProposal storage proposal = proposals[proposalId]; require(proposal.id != bytes32(0), "InvalidProposal"); @@ -106,14 +112,17 @@ contract ArbitrationModule is IArbitrationModule bytes32 escrowId = proposal.escrowId; //ensure that escrowId is valid with escrow + //EXCEPTION: InvalidEscrow require(polyEscrow.getEscrow(escrowId).id == escrowId, "InvalidEscrow"); //TODO: ensure that escrow is in correct state to be voted on //validate rights of voter + //EXCEPTION: Unauthorized require(_canVoteArbitration(polyEscrow, escrowId, msg.sender), "Unauthorized"); //verify that the proposal is in a state in which it can be voted + //EXCEPTION: InvalidProposalState require(proposal.status == ArbitrationStatus.ACTIVE, "InvalidProposalState"); //record vote @@ -149,15 +158,25 @@ contract ArbitrationModule is IArbitrationModule } function cancelArbitration(bytes32 proposalId) external virtual { - - //TODO: WHO can cancel arbitration? - //TODO: all arbitration on an escrow should be cancelled if the seller takes any action - //get the arbitration proposal - //TODO: this code is repeated alot; can it be put into its own function + //EXCEPTION: InvalidProposal ArbitrationProposal storage proposal = proposals[proposalId]; require(proposal.id != bytes32(0), "InvalidProposal"); + //only the proposer can cancel + //EXCEPTION: Unauthorized + require(proposal.proposer == msg.sender, "Unauthorized"); + + //TODO: all arbitration on an escrow should be cancelled if the seller takes any action + + //can only cancel if status is ACTIVE + //EXCEPTION: NotCancellable + require(proposal.status == ArbitrationStatus.ACTIVE, "NotCancellable"); + + //can only cancel if no votes have been cast + //EXCEPTION: NotCancellable + require(proposal.votesFor == 0 && proposal.votesAgainst == 0, "NotCancellable"); + //TODO: can only cancel if status is ACTIVE require(proposal.status == ArbitrationStatus.ACTIVE, "InvalidProposalState"); @@ -168,10 +187,12 @@ contract ArbitrationModule is IArbitrationModule function executeArbitration(IPolyEscrow polyEscrow, bytes32 proposalId) external virtual { //get the arbitration proposal + //EXCEPTION: InvalidProposal ArbitrationProposal storage proposal = proposals[proposalId]; require(proposal.id != bytes32(0), "InvalidProposal"); //proposal must be accepted + //EXCEPTION: InvalidEscrowState require(proposal.status == ArbitrationStatus.ACCEPTED, "InvalidEscrowState"); //TODO: re-validate the amount (adjust it if necessary) @@ -180,6 +201,7 @@ contract ArbitrationModule is IArbitrationModule bytes32 escrowId = proposal.escrowId; //ensure that escrowId is valid with escrow + //EXCEPTION: InvalidEscrow require(polyEscrow.getEscrow(escrowId).id == escrowId, "InvalidEscrow"); //TODO: ensure that escrow is in correct state to have arbitration executed @@ -229,4 +251,34 @@ contract ArbitrationModule is IArbitrationModule //TODO: should also include Pending? } + + function _voteArbitration(IPolyEscrow polyEscrow, ArbitrationProposal storage proposal, bool vote) internal { + + //record vote + if (vote) { + proposal.votesFor += 1; + } else { + proposal.votesAgainst += 1; + } + proposalVotes[proposal.id][msg.sender] = vote; + + //change the status; are there enough votes to execute? + Escrow memory escrow = polyEscrow.getEscrow(proposal.escrowId); + uint256 arbiterCount = escrow.arbiters.length; + uint8 arbitersRequired = escrow.arbitersRequired; + if (proposal.votesFor >= arbitersRequired) { + proposal.status = ArbitrationStatus.ACCEPTED; + + // Auto-execute if autoExecute flag is true + if (proposal.autoExecute) { + _executeArbitration(polyEscrow, proposal); + } + } + else if (proposal.votesAgainst >= (arbiterCount - arbitersRequired)) { + proposal.status = ArbitrationStatus.REJECTED; + } + + //raise event + emit VoteRecorded(proposal.id, proposal.escrowId, msg.sender); + } } diff --git a/src/PolyEscrow.sol b/src/PolyEscrow.sol index e402088..ee5eabc 100644 --- a/src/PolyEscrow.sol +++ b/src/PolyEscrow.sol @@ -96,6 +96,7 @@ contract PolyEscrow is HasSecurityContext, Pausable, IPolyEscrow settings = systemSettings; defaultArbitrationModule = arbitrationModule; + //EXCEPTION: InvalidArbitrationModule require(address(arbitrationModule) != address(0), "InvalidArbitrationModule"); require(arbitrationModule.isArbitrationModule(), "InvalidArbitrationModule"); } @@ -124,7 +125,7 @@ contract PolyEscrow is HasSecurityContext, Pausable, IPolyEscrow //TODO: validate input more - // EXCEPTION: reject if existing escrow + // EXCEPTION: DuplicateEscrow if existing escrow require(escrows[input.id].id != input.id, "DuplicateEscrow"); // Store the escrow @@ -149,6 +150,7 @@ contract PolyEscrow is HasSecurityContext, Pausable, IPolyEscrow escrow.status = EscrowStatus.Pending; if (address(input.arbitrationModule) != address(0)) { + //EXCEPTION: InvalidArbitrationModule require(input.arbitrationModule.isArbitrationModule(), "InvalidArbitrationModule"); escrow.arbitrationModule = input.arbitrationModule; } @@ -186,20 +188,20 @@ contract PolyEscrow is HasSecurityContext, Pausable, IPolyEscrow function placePayment(PaymentInput calldata paymentInput) public payable whenNotPaused { _validatePaymentInput(paymentInput); - //EXCEPTION: reject if not existing payment + //EXCEPTION: InvalidEscrow if not existing payment require(escrows[paymentInput.escrowId].id == paymentInput.escrowId, "InvalidEscrow"); //TODO: EXCEPTION: reject if escrow not in the correct state to accept payments - //EXCEPTION: reject the wrong currency + //EXCEPTION: InvalidCurrency reject the wrong currency require (paymentInput.currency == escrows[paymentInput.escrowId].currency, "InvalidCurrency"); // Handle payment transfer if (paymentInput.currency == address(0)) { - //EXCEPTION: wrong amount rejected + //EXCEPTION: InvalidAmount amount rejected require(msg.value == paymentInput.amount, "InvalidAmount"); } else { - //EXCEPTION: failed payment + //EXCEPTION: TokenPaymentFailed failed payment require(_handleTokenInflow(paymentInput.currency, msg.sender, paymentInput.amount), "TokenPaymentFailed"); } @@ -401,6 +403,7 @@ contract PolyEscrow is HasSecurityContext, Pausable, IPolyEscrow * @param input The payment input */ function _validatePaymentInput(PaymentInput calldata input) internal pure { + //EXCEPTION: InvalidAmount require(input.amount > 0, "InvalidAmount"); } @@ -429,13 +432,18 @@ contract PolyEscrow is HasSecurityContext, Pausable, IPolyEscrow } function _validateArbiters(address[] memory arbiters, address payer, address receiver) internal pure { + //EXCEPTION: MaxArbitersExceeded require(arbiters.length <= MAX_ARBITERS, "MaxArbitersExceeded"); for (uint256 i = 0; i < arbiters.length; i++) { + //EXCEPTION: InvalidArbiter require(arbiters[i] != address(0), "InvalidArbiter"); for (uint256 j = i + 1; j < arbiters.length; j++) { + //EXCEPTION: DuplicateArbiter require(arbiters[i] != arbiters[j], "DuplicateArbiter"); } + + //EXCEPTION: InvalidArbiter require(arbiters[i] != payer, "InvalidArbiter"); require(arbiters[i] != receiver, "InvalidArbiter"); } @@ -456,11 +464,13 @@ contract PolyEscrow is HasSecurityContext, Pausable, IPolyEscrow function _refund(bytes32 escrowId, uint256 amount) internal { Escrow storage escrow = escrows[escrowId]; + //EXCEPTION: AlreadyReleased require(escrow.released == false, "AlreadyReleased"); uint256 activeAmount = _getEscrowAmountRemaining(escrow); if (amount > activeAmount) + //EXCEPTION: AmountExceeded revert("AmountExceeded"); //transfer amount back to payer @@ -475,10 +485,12 @@ contract PolyEscrow is HasSecurityContext, Pausable, IPolyEscrow function _release(bytes32 escrowId, uint256 amount) internal { Escrow storage escrow = escrows[escrowId]; + //EXCEPTION: AlreadyReleased require(escrow.released == false, "AlreadyReleased"); uint256 activeAmount = _getEscrowAmountRemaining(escrow); + //EXCEPTION: AmountExceeded if (amount > activeAmount) revert("AmountExceeded"); @@ -529,8 +541,10 @@ contract PolyEscrow is HasSecurityContext, Pausable, IPolyEscrow function executeArbitrationProposal(bytes32 escrowId, ArbitrationType proposalType, uint256 amount) external { Escrow storage escrow = escrows[escrowId]; + //EXCEPTION: InvalidEscrow require(escrow.id != bytes32(0), "InvalidEscrow"); + //EXCEPTION: Unauthorized require(msg.sender == address(escrow.arbitrationModule), "Unauthorized"); if (proposalType == ArbitrationType.REFUND) { diff --git a/src/interfaces/IArbitrationModule.sol b/src/interfaces/IArbitrationModule.sol index 5b689a2..93dcf2f 100644 --- a/src/interfaces/IArbitrationModule.sol +++ b/src/interfaces/IArbitrationModule.sol @@ -22,6 +22,7 @@ struct ArbitrationProposal { bytes32 escrowId; ArbitrationType proposalType; ArbitrationStatus status; + address proposer; uint256 amount; uint8 votesFor; uint8 votesAgainst; diff --git a/test/Arbitration.ts b/test/Arbitration.ts index 4b7c5fd..36e5012 100644 --- a/test/Arbitration.ts +++ b/test/Arbitration.ts @@ -382,6 +382,7 @@ describe('Arbitration', function () { //verify proposal expect(proposal.amount).to.equal(proposalAmount); + expect(proposal.proposer).to.equal(account.address); expect(proposal.escrowId).to.equal(escrowId); expect(proposal.proposalType).to.equal(proposalType); } @@ -708,10 +709,10 @@ describe('Arbitration', function () { proposal = await voteProposal(account, proposal.id, vote); if (vote) { - expect(proposal.votesFor).to.equal(2); + expect(proposal.votesFor).to.equal(1); expect(proposal.votesAgainst).to.equal(0); } else { - expect(proposal.votesFor).to.equal(1); + expect(proposal.votesFor).to.equal(0); expect(proposal.votesAgainst).to.equal(1); } } @@ -764,13 +765,20 @@ describe('Arbitration', function () { //vote on proposal await voteProposal(arbiter1, proposal.id, true); - //at this point, votes should be 2:0 for:against + //at this point, votes should be 1:0 for:against expect((await getProposal(proposal.id)).status).to.equal( PROPOSAL_STATUS_ACTIVE ); //vote on proposal await voteProposal(arbiter2, proposal.id, true); + //at this point, votes should be 2:0 for:against + expect((await getProposal(proposal.id)).status).to.equal( + PROPOSAL_STATUS_ACTIVE + ); + + //vote on proposal + await voteProposal(arbiter3, proposal.id, true); //at this point, votes should be 3:0 for:against, enough to win expect((await getProposal(proposal.id)).status).to.equal( PROPOSAL_STATUS_ACCEPTED @@ -935,12 +943,132 @@ describe('Arbitration', function () { }); describe('Cancelling Arbitration', function () { - describe('Happy Paths', function () {}); + async function createEscrowProposal( + escrowId: any + ): Promise { + //create the escrow + const amount = 1000000; + const isToken = true; + + //arbiters 2/3 + await createEscrow( + escrowId, + payer1, + receiver1.address, + amount, + isToken, + [arbiter1.address, arbiter2.address, arbiter3.address], + 2 + ); + + //pay into escrow + await placePayment(escrowId, payer1, amount, isToken); + + //propose arbitration as payer + const proposalType = PROPOSE_REFUND; + const proposalAmount = amount; + let proposal = await createProposal( + payer1, + escrowId, + proposalType, + proposalAmount + ); + + return proposal; + } + + describe('Happy Paths', function () { + it('proposer can cancel proposal if no votes', async function () { + const escrowId = ethers.keccak256('0x01'); + const proposal = await createEscrowProposal(escrowId); + + await arbitrationModule + .connect(payer1) + .cancelArbitration(proposal.id); + + const cancelledProposal = await getProposal(proposal.id); + expect(cancelledProposal.status == PROPOSAL_STATUS_CANCELLED); + }); + }); describe('Exceptions', function () { - it.skip('cannot cancel invalid proposal', async function () {}); + it('cannot cancel invalid proposal', async function () { + const escrowId = ethers.keccak256('0x01'); + const proposal = await createEscrowProposal(escrowId); + + await expect( + arbitrationModule + .connect(payer1) + .cancelArbitration( + proposal.id + .replace('3', '1') + .replace('2', '4') + .replace('a', 'b') + ) + ).to.be.revertedWith('InvalidProposal'); + }); + + it('cannot cancel if not the proposer', async function () { + const escrowId = ethers.keccak256('0x01'); + const proposal = await createEscrowProposal(escrowId); + + await expect( + arbitrationModule + .connect(receiver1) + .cancelArbitration(proposal.id) + ).to.be.revertedWith('Unauthorized'); + + await expect( + arbitrationModule + .connect(arbiter1) + .cancelArbitration(proposal.id) + ).to.be.revertedWith('Unauthorized'); + + await expect( + arbitrationModule + .connect(arbiter2) + .cancelArbitration(proposal.id) + ).to.be.revertedWith('Unauthorized'); + + await expect( + arbitrationModule + .connect(arbiter3) + .cancelArbitration(proposal.id) + ).to.be.revertedWith('Unauthorized'); + }); + + it('cannot cancel cancelled proposal', async function () { + const escrowId = ethers.keccak256('0x01'); + const proposal = await createEscrowProposal(escrowId); + + //cancel proposal + await arbitrationModule + .connect(payer1) + .cancelArbitration(proposal.id); - it.skip('cannot cancel inactive proposal', async function () {}); + //try to cancel again; should fail + await expect( + arbitrationModule + .connect(payer1) + .cancelArbitration(proposal.id) + ).to.be.revertedWith('NotCancellable'); + }); + + it('cannot cancel if any votes have been cast', async function () { + const escrowId = ethers.keccak256('0x01'); + const proposal = await createEscrowProposal(escrowId); + + //cast a vote + await arbitrationModule + .connect(arbiter1) + .voteArbitration(polyEscrow, proposal.id, true); + + await expect( + arbitrationModule + .connect(payer1) + .cancelArbitration(proposal.id) + ).to.be.revertedWith('NotCancellable'); + }); }); describe('Events', function () {}); diff --git a/test/util.ts b/test/util.ts index 034b4f7..d36d992 100644 --- a/test/util.ts +++ b/test/util.ts @@ -28,6 +28,7 @@ export interface IArbitrationProposal { escrowId: any; proposalType: number; status: number; + proposer: string; amount: any; votesFor: number; votesAgainst: number; @@ -63,8 +64,9 @@ export function convertProposal(rawData: any[]): IArbitrationProposal { escrowId: rawData[1], proposalType: rawData[2], status: rawData[3], - amount: rawData[4], - votesFor: rawData[5], - votesAgainst: rawData[6], + proposer: rawData[4], + amount: rawData[5], + votesFor: rawData[6], + votesAgainst: rawData[7], }; }