Dev branch review

ampleforth · Apr 23, 2019 · faacfab · faacfab
1 parent e5efb21
commit faacfab
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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;
   });
 });