chore: priority queue fail with message when empty

To make the failure more gracefull and easier to understand. issue: #391
omgnetwork · Oct 30, 2019 · 6a17399 · 6a17399
1 parent d0a062e
commit 6a17399
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 5 deletions.
diff --git a/plasma_framework/contracts/src/framework/utils/PriorityQueue.sol b/plasma_framework/contracts/src/framework/utils/PriorityQueue.sol
@@ -43,7 +43,7 @@ contract PriorityQueue is OnlyFromAddress {
     }
 
     /**
-     * @notice Inserts an element into the queue by the owner
+     * @notice Inserts an element into the queue by the framework
      * @dev Does not perform deduplication
      */
     function insert(uint256 _element) external onlyFrom(framework) {
@@ -53,10 +53,12 @@ contract PriorityQueue is OnlyFromAddress {
     }
 
     /**
-     * @notice Deletes the smallest element from the queue
+     * @notice Deletes the smallest element from the queue by the framework
+     * @dev Fails when queue is empty
      * @return The smallest element in the priority queue
      */
     function delMin() external onlyFrom(framework) returns (uint256) {
+        require(queue.currentSize > 0, "Queue is empty");
         uint256 retVal = queue.heapList[1];
         queue.heapList[1] = queue.heapList[queue.currentSize];
         delete queue.heapList[queue.currentSize];
@@ -72,6 +74,7 @@ contract PriorityQueue is OnlyFromAddress {
      * @return The smallest element in the priority queue
      */
     function getMin() external view returns (uint256) {
+        require(queue.currentSize > 0, "Queue is empty");
         return queue.heapList[1];
     }
 

diff --git a/plasma_framework/test/src/framework/utils/PriorityQueue.test.js b/plasma_framework/test/src/framework/utils/PriorityQueue.test.js
@@ -17,9 +17,10 @@ contract('PriorityQueue', ([_, nonOwner]) => {
         });
 
         it('fails when empty', async () => {
-            const errorMsg = 'Returned error: VM Exception while processing transaction: invalid opcode';
-            await this.priorityQueue.getMin()
-                .catch(err => expect(err.message).to.equal(errorMsg));
+            await expectRevert(
+                this.priorityQueue.getMin(),
+                'Queue is empty',
+            );
         });
     });
 
@@ -128,5 +129,12 @@ contract('PriorityQueue', ([_, nonOwner]) => {
                 'Caller address is unauthorized',
             );
         });
+
+        it('should fail when queue is empty', async () => {
+            await expectRevert(
+                this.priorityQueue.delMin(),
+                'Queue is empty',
+            );
+        });
     });
 });