feat: return proposalId and verify

thomas-waite · thomas-waite · commit cc267656b69c · 2022-06-12T22:44:08.000+01:00
diff --git a/contracts/pods/PodExecutor.sol b/contracts/pods/PodExecutor.sol
@@ -20,7 +20,7 @@ contract PodExecutor is CoreRef {
         bytes calldata data,
         bytes32 predecessor,
         bytes32 salt
-    ) public payable whenNotPaused {
+    ) public payable whenNotPaused returns (bytes32) {
         bytes32 proposalId = TimelockController(payable(timelock)).hashOperation(
             target,
             value,
@@ -30,6 +30,7 @@ contract PodExecutor is CoreRef {
         );
         TimelockController(payable(timelock)).execute(target, value, data, predecessor, salt);
         emit ExecuteTransaction(timelock, proposalId);
+        return proposalId;
     }
 
     /// @notice Execute a transaction which contains a set of actions which were batch scheduled on a timelock
@@ -40,7 +41,7 @@ contract PodExecutor is CoreRef {
         bytes[] calldata payloads,
         bytes32 predecessor,
         bytes32 salt
-    ) external payable whenNotPaused {
+    ) external payable whenNotPaused returns (bytes32) {
         bytes32 proposalId = TimelockController(payable(timelock)).hashOperationBatch(
             targets,
             values,
@@ -50,5 +51,6 @@ contract PodExecutor is CoreRef {
         );
         TimelockController(payable(timelock)).executeBatch(targets, values, payloads, predecessor, salt);
         emit ExecuteTransaction(timelock, proposalId);
+        return proposalId;
     }
 }
diff --git a/contracts/test/integration/governance/PodOperation.t.sol b/contracts/test/integration/governance/PodOperation.t.sol
@@ -127,7 +127,7 @@ contract PodOperationIntegrationTest is DSTest {
         values[1] = 0;
 
         bytes32 predecessor = bytes32(0);
-        bytes32 salt = bytes32(1);
+        bytes32 salt = bytes32(uint256(1));
 
         vm.prank(safe);
         timelockContract.scheduleBatch(targets, values, payloads, predecessor, salt, podConfig.minDelay);
diff --git a/contracts/test/unit/governance/PodExecutor.t.sol b/contracts/test/unit/governance/PodExecutor.t.sol
@@ -33,7 +33,7 @@ function dummyBatchProposals(address[] memory ethReceivers, uint256[] memory amo
     payloads[1] = abi.encodePacked(bytes4(keccak256(bytes("transfer(uint256)"))), amounts[1]);
 
     bytes32 predecessor = bytes32(0);
-    bytes32 salt = bytes32(0);
+    bytes32 salt = bytes32(uint256(1));
     uint256 delay = 1;
     return (targets, values, payloads, predecessor, salt, delay);
 }
@@ -142,10 +142,8 @@ contract PodExecutorTest is DSTest {
         assertGt(timelock.getTimestamp(proposalId), 0);
 
         // 3. Execute proposal through PodExecutor
-        podExecutor.execute(address(timelock), target, value, payload, predecessor, salt);
-
-        // TODO: Verify proposalId is emitted in the event as expected
-        // TODO: (maybe) Verify CallExecuted event is emitted
+        bytes32 executedProposalId = podExecutor.execute(address(timelock), target, value, payload, predecessor, salt);
+        assertEq(executedProposalId, proposalId);
 
         // Verfiy state
         assertTrue(timelock.isOperation(proposalId));
@@ -200,10 +198,15 @@ contract PodExecutorTest is DSTest {
         assertGt(timelock.getTimestamp(proposalId), 0);
 
         // 3. Execute proposal through PodExecutor
-        podExecutor.executeBatch(address(timelock), targets, values, payloads, predecessor, salt);
-
-        // TODO: Verify proposalId is emitted in the event as expected
-        // TODO: (maybe) Verify CallExecuted event is emitted
+        bytes32 executedProposalId = podExecutor.executeBatch(
+            address(timelock),
+            targets,
+            values,
+            payloads,
+            predecessor,
+            salt
+        );
+        assertEq(executedProposalId, proposalId);
 
         // Verfiy state
         assertTrue(timelock.isOperation(proposalId));
diff --git a/proposals/description/pod_exec_v2.ts b/proposals/description/pod_exec_v2.ts
diff --git a/test/integration/tests/podExecutor.ts b/test/integration/tests/podExecutor.ts
@@ -33,7 +33,7 @@ const dummyBatchProposal = (dai: ERC20, receiverA: string, receiverB: string, am
   };
 };
 
-describe('Tribal Council', function () {
+describe('Pod executor', function () {
   let contracts: NamedContracts;
   let contractAddresses: NamedAddresses;
   let e2eCoord: TestEndtoEndCoordinator;
