From faacfab7433d50852a0cd76583c40eadba95830d Mon Sep 17 00:00:00 2001
From: ahnaguib <ahnaguib@gmail.com>
Date: Tue, 23 Apr 2019 16:04:49 -0700
Subject: [PATCH] Dev branch review

---
 contracts/MarketOracle.sol |  5 ++++-
 contracts/lib/Select.sol   |  2 +-
 test/unit/market_oracle.js | 15 ++++++++++++---
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/contracts/MarketOracle.sol b/contracts/MarketOracle.sol
index 8b4ce7c..8af4232 100644
--- a/contracts/MarketOracle.sol
+++ b/contracts/MarketOracle.sol
@@ -39,6 +39,8 @@ contract MedianOracle is Ownable, IOracle {
     // The number of seconds after which the report is deemed expired.
     uint256 public reportExpirationTimeSec;
 
+    // The number of seconds since reporting that has to pass before a report
+    // is usable.
     uint256 public reportDelaySec;
 
     uint256 public minimumProviders = 1;
@@ -50,6 +52,7 @@ contract MedianOracle is Ownable, IOracle {
     {
         reportExpirationTimeSec = reportExpirationTimeSec_;
         reportDelaySec = reportDelaySec_;
+        require(minimumProviders_ > 0);
         minimumProviders = minimumProviders_;
     }
 
@@ -189,7 +192,7 @@ contract MedianOracle is Ownable, IOracle {
      * @return The number of providers.
      */
     function providersSize()
-        public
+        external
         view
         returns (uint256)
     {
diff --git a/contracts/lib/Select.sol b/contracts/lib/Select.sol
index c9f2c02..8c843c1 100644
--- a/contracts/lib/Select.sol
+++ b/contracts/lib/Select.sol
@@ -1,4 +1,4 @@
-pragma solidity 0.4.24 ;
+pragma solidity ^0.4.24;
 import "openzeppelin-solidity/contracts/math/SafeMath.sol";
 
 library Select {
diff --git a/test/unit/market_oracle.js b/test/unit/market_oracle.js
index 7c9cc04..69edf63 100644
--- a/test/unit/market_oracle.js
+++ b/test/unit/market_oracle.js
@@ -20,6 +20,16 @@ async function setupContractsAndAccounts (accounts) {
   oracle = await MedianOracle.new(60, 10, 1);
 }
 
+contract('MedianOracle:constructor', async function (accounts) {
+  before(async function () {
+    await setupContractsAndAccounts(accounts);
+  });
+
+  it('should fail if a parameter is invalid', async function () {
+    expect(await chain.isEthException(MedianOracle.new(60, 10, 0))).to.be.true;
+  });
+});
+
 contract('MedianOracle:providersSize', async function (accounts) {
   before(async function () {
     await setupContractsAndAccounts(accounts);
@@ -61,11 +71,10 @@ contract('MedianOracle:pushReport', async function (accounts) {
     await setupContractsAndAccounts(accounts);
   });
   it('should only push from authorized source', async function () {
-    expect(
-      await chain.isEthException(oracle.pushReport(1000000000000000000, { from: A }))
-    ).to.be.true;
+    expect(await chain.isEthException(oracle.pushReport(1000000000000000000, { from: A }))).to.be.true;
     oracle.addProvider(A, { from: deployer });
     await oracle.pushReport(1000000000000000000, { from: A });
+    // should fail if reportDelaySec did not pass since the previous push
     expect(await chain.isEthException(oracle.pushReport(1000000000000000000, { from: A }))).to.be.true;
   });
 });