combine vote and execute, modify the test files based on the change

contracts/Bridge.sol

@@ -64,7 +64,7 @@ contract Bridge is Pausable, AccessControl, SafeMath {
         bytes32 dataHash
     );
     event FailedHandlerExecution(
-        string reason
+        bytes lowLevelData
     );
 
     bytes32 public constant RELAYER_ROLE = keccak256("RELAYER_ROLE");
@@ -363,6 +363,7 @@ contract Bridge is Pausable, AccessControl, SafeMath {
 
         if (proposal._status == ProposalStatus.Passed) {
             executeProposal(domainID, depositNonce, data, resourceID, true);
+            return;
         } else {
             require(uint(proposal._status) <= 1, "proposal already executed/cancelled");
             require(!_hasVoted(proposal, msg.sender), "relayer already voted");
@@ -450,16 +451,16 @@ contract Bridge is Pausable, AccessControl, SafeMath {
 
         require(proposal._status == ProposalStatus.Passed, "Proposal must have Passed status");
 
+        proposal._status = ProposalStatus.Executed;
         IDepositExecute depositHandler = IDepositExecute(handler);
 
-        if (revertOnFail == true) {
-            proposal._status = ProposalStatus.Executed;
+        if (revertOnFail) {
             depositHandler.executeProposal(resourceID, data);
         } else {
             try depositHandler.executeProposal(resourceID, data) {
-                proposal._status = ProposalStatus.Executed;
-            } catch Error(string memory reason) {
-                emit FailedHandlerExecution(reason);
+            } catch (bytes memory lowLevelData) {
+                proposal._status = ProposalStatus.Passed;
+                emit FailedHandlerExecution(lowLevelData);
                 return;
             }
         }

test/contractBridge/cancelDepositProposal.js

@@ -22,8 +22,6 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)
     const relayer1Bit = 1 << 0;
     const relayer2Bit = 1 << 1;
     const relayer3Bit = 1 << 2;
-    const relayer4Bit = 1 << 3;
-    const depositerAddress = accounts[4];
     const destinationChainRecipientAddress = accounts[4];
     const depositAmount = 10;
     const expectedDepositNonce = 1;

test/contractBridge/executeWithFailedHandler.js

@@ -10,12 +10,10 @@ const ERC20HandlerContract = artifacts.require("HandlerRevert");
 contract('Bridge - [execute - FailedHandlerExecution]', async accounts => {
     const relayerThreshold = 2;
     const domainID = 1;
-
     const depositerAddress = accounts[1];
     const recipientAddress = accounts[2];
     const relayer1Address = accounts[3];
     const relayer2Address = accounts[4];
-    const relayer3Address = accounts[5];
 
     const initialTokenAmount = 100;
     const depositAmount = 10;
@@ -29,9 +27,14 @@ contract('Bridge - [execute - FailedHandlerExecution]', async accounts => {
     let depositData;
     let depositProposalData;
     let depositProposalDataHash;
-    let initialResourceIDs;
-    let initialContractAddresses;
-    let burnableContractAddresses;
+
+    const STATUS = {
+        Inactive : '0',
+        Active : '1',
+        Passed : '2',
+        Executed : '3',
+        Cancelled : '4'
+    }
 
     beforeEach(async () => {
         await Promise.all([
@@ -41,10 +44,6 @@ contract('Bridge - [execute - FailedHandlerExecution]', async accounts => {
 
         resourceID = Helpers.createResourceID(ERC20MintableInstance.address, domainID);
 
-        initialResourceIDs = [resourceID];
-        initialContractAddresses = [ERC20MintableInstance.address];
-        burnableContractAddresses = [];
-
         ERC20HandlerInstance = await ERC20HandlerContract.new(BridgeInstance.address);
 
         await Promise.all([
@@ -78,6 +77,9 @@ contract('Bridge - [execute - FailedHandlerExecution]', async accounts => {
             { from: relayer2Address }
         ));
 
+        const depositProposalBeforeFailedExecute = await BridgeInstance.getProposal(
+            domainID, expectedDepositNonce, depositProposalDataHash);
+
         await TruffleAssert.reverts(BridgeInstance.executeProposal(
             domainID,
             expectedDepositNonce,
@@ -89,8 +91,8 @@ contract('Bridge - [execute - FailedHandlerExecution]', async accounts => {
 
         const depositProposalAfterFailedExecute = await BridgeInstance.getProposal(
             domainID, expectedDepositNonce, depositProposalDataHash);
-        
-        assert.strictEqual(depositProposalAfterFailedExecute._status, '2');
+
+        assert.deepInclude(Object.assign({}, depositProposalBeforeFailedExecute), depositProposalAfterFailedExecute);
     });
 
     it("Should not revert even though handler execute is reverted if the proposal's status is changed to Passed during voting. FailedHandlerExecution event should be emitted with expected values. Proposal status still stays on Passed", async () => {        
@@ -111,13 +113,41 @@ contract('Bridge - [execute - FailedHandlerExecution]', async accounts => {
             { from: relayer2Address }
         );
 
-        TruffleAssert.eventEmitted(voteWithExecuteTx, 'FailedHandlerExecution', (event) => {
-            return event.reason.toString() === "Something bad happened"
+        TruffleAssert.eventEmitted(voteWithExecuteTx, 'FailedHandlerExecution', (event) => {   
+            return Ethers.utils.parseBytes32String("0x"+event.lowLevelData.slice(-64)) === "Something bad happened"
         });
-
+        
         const depositProposalAfterFailedExecute = await BridgeInstance.getProposal(
             domainID, expectedDepositNonce, depositProposalDataHash);
 
-        assert.strictEqual(depositProposalAfterFailedExecute._status, '2');        
+        assert.strictEqual(depositProposalAfterFailedExecute._status, STATUS.Passed);
+    });
+
+    it("Vote proposal should be reverted if handler execution is reverted and proposal status was on Passed for vote", async () => {
+
+        TruffleAssert.passes(await BridgeInstance.voteProposal(
+            domainID,
+            expectedDepositNonce,
+            resourceID,
+            depositProposalData,
+            { from: relayer1Address }
+        ));
+
+        // After this vote, automatically executes the proposqal but handler execute is reverted. So proposal still stays on Passed after this vote.
+        TruffleAssert.passes(await BridgeInstance.voteProposal(
+            domainID,
+            expectedDepositNonce,
+            resourceID,
+            depositProposalData,
+            { from: relayer2Address }
+        ));
+
+        await TruffleAssert.reverts(BridgeInstance.voteProposal(
+            domainID,
+            expectedDepositNonce,
+            resourceID,
+            depositProposalData,
+            { from: relayer2Address }
+        ), 'Something bad happened');
     });
 });

test/contractBridge/voteDepositProposal.js

@@ -22,7 +22,6 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)
     const relayer1Bit = 1 << 0;
     const relayer2Bit = 1 << 1;
     const relayer3Bit = 1 << 2;
-    const relayer4Bit = 1 << 3;
     const depositerAddress = accounts[4];
     const destinationChainRecipientAddress = accounts[4];
     const depositAmount = 10;
@@ -31,6 +30,14 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)
     const expectedFinalizedEventStatus = 2;
     const expectedExecutedEventStatus = 3;
 
+    const STATUS = {
+        Inactive : '0',
+        Active : '1',
+        Passed : '2',
+        Executed : '3',
+        Cancelled : '4'
+    }
+
     let BridgeInstance;
     let DestinationERC20MintableInstance;
     let DestinationERC20HandlerInstance;
@@ -98,29 +105,31 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)
         assert.deepInclude(Object.assign({}, depositProposal), expectedDepositProposal);
     });
 
-    it('should revert because depositerAddress is not a relayer', async () => {
-        await TruffleAssert.reverts(vote(depositerAddress));
-    });
-
-    it("depositProposal shouldn't be voted on if it has a Passed status", async () => {
+    it("depositProposal should be automatically executed after the vote if proposal staus is changed to Passed during the vote", async () => {
         await TruffleAssert.passes(vote(relayer1Address));
 
         await TruffleAssert.passes(vote(relayer2Address));
 
-        await TruffleAssert.passes(vote(relayer3Address));
+        await TruffleAssert.passes(vote(relayer3Address)); // After this vote, automatically executes the proposal.
 
-        await TruffleAssert.reverts(vote(relayer4Address), 'proposal already executed/cancelled.');
+        const depositProposalAfterThirdVoteWithExecute = await BridgeInstance.getProposal(
+            originDomainID, expectedDepositNonce, depositDataHash);
+
+        assert.strictEqual(depositProposalAfterThirdVoteWithExecute._status, STATUS.Executed); // Executed
+    });
+
+    it('should revert because depositerAddress is not a relayer', async () => {
+        await TruffleAssert.reverts(vote(depositerAddress));
     });
 
-    it("depositProposal shouldn't be voted on if it has a Transferred status", async () => {
+    it("depositProposal shouldn't be voted on if it has a Passed status", async () => {
         await TruffleAssert.passes(vote(relayer1Address));
 
         await TruffleAssert.passes(vote(relayer2Address));
 
-        await TruffleAssert.passes(vote(relayer3Address)); // After this vote, automatically executes the proposal.
+        await TruffleAssert.passes(vote(relayer3Address));
 
         await TruffleAssert.reverts(vote(relayer4Address), 'proposal already executed/cancelled.');
-
     });
 
     it("relayer shouldn't be able to vote on a depositProposal more than once", async () => {
@@ -146,23 +155,23 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)
             originDomainID, expectedDepositNonce, depositDataHash);
         assert.equal(depositProposalAfterFirstVote._yesVotesTotal, 1);
         assert.equal(depositProposalAfterFirstVote._yesVotes, relayer1Bit);
-        assert.strictEqual(depositProposalAfterFirstVote._status, '1');
+        assert.strictEqual(depositProposalAfterFirstVote._status, STATUS.Active);
 
         await TruffleAssert.passes(vote(relayer2Address));
 
         const depositProposalAfterSecondVote = await BridgeInstance.getProposal(
             originDomainID, expectedDepositNonce, depositDataHash);
         assert.equal(depositProposalAfterSecondVote._yesVotesTotal, 2);
         assert.equal(depositProposalAfterSecondVote._yesVotes, relayer1Bit + relayer2Bit);
-        assert.strictEqual(depositProposalAfterSecondVote._status, '1');
+        assert.strictEqual(depositProposalAfterSecondVote._status, STATUS.Active);
 
         await TruffleAssert.passes(vote(relayer3Address)); // After this vote, automatically executes the proposal.
 
         const depositProposalAfterThirdVote = await BridgeInstance.getProposal(
             originDomainID, expectedDepositNonce, depositDataHash);
         assert.equal(depositProposalAfterThirdVote._yesVotesTotal, 3);
         assert.equal(depositProposalAfterThirdVote._yesVotes, relayer1Bit + relayer2Bit + relayer3Bit);
-        assert.strictEqual(depositProposalAfterThirdVote._status, '3'); // Executed
+        assert.strictEqual(depositProposalAfterThirdVote._status, STATUS.Executed); // Executed
     });
 
     it("Relayer's address should be marked as voted for proposal", async () => {

test/e2e/erc721/differentChainsMock.js

@@ -28,7 +28,6 @@ contract('E2E ERC721 - Two EVM Chains', async accounts => {
     let OriginERC721HandlerInstance;
     let originDepositData;
     let originDepositProposalData;
-    let originDepositProposalDataHash;
     let originResourceID;
     let originBurnableContractAddresses;
 
@@ -77,7 +76,6 @@ contract('E2E ERC721 - Two EVM Chains', async accounts => {
 
         originDepositData = Helpers.createERCDepositData(tokenID, 20, recipientAddress);
         originDepositProposalData = Helpers.createERC721DepositProposalData(tokenID, 20, recipientAddress, 32, 0);
-        originDepositProposalDataHash = Ethers.utils.keccak256(DestinationERC721HandlerInstance.address + originDepositProposalData.substr(2));
 
         destinationDepositData = Helpers.createERCDepositData(tokenID, 20, depositerAddress);
         destinationDepositProposalData = Helpers.createERC721DepositProposalData(tokenID, 20, depositerAddress, 32, 0)