Get rid of unnecessary looping in getNumberMediaByAddress

avoiding_common_mistakes.md

@@ -10,6 +10,8 @@ I have mitigated against this risk by:
 
 * Following Solidity coding standards and general coding best practices.
 * Avoiding overly complex rules.
+* Writing unit tests that cover all functions of my contract, including the happy path, and situations where exceptions will/can occur.
+* Leveraging tools like Solidity Coverage to enesure I have tested relevant scenarios of my contract.
 
 ## 2. Poison Data
 Contracts that accept user input that is stored or exposed to other users are vulnerable to being supplied with unanticipated input that causes problems for the contract or for other users of the contract. 
@@ -36,7 +38,7 @@ I have mitigated against this risk by:
 
 * Limiting the length of user-supplied data such as the name, description, and tags for a media asset.
 
-## 6. Other Vulnerabilities
+## 6. Block Timestamps/Hashes
 Even without serious collusion, Ethereum miners have some limited ability to influence block timestamps and which transactions are chosen in a block.
 
 We have mitigated against this risk by:
@@ -57,3 +59,31 @@ The blockchain is immutable in the sense that a once approved transaction is goi
 I have added a tool called Solidity-Coverage in order to generate a report to understand how extensive my test coverage is for the MediaGallery contract.
 
 The report for my test coverage can be found here: [Solidity Coverage Report](http://htmlpreview.github.io/?https://github.com/gjackson12/media_gallery_ethereum/blob/master/coverage/contracts/index.html)
+
+## 9. Race Conditions
+One of the major dangers of calling external contracts is that they can take over the control flow, and make changes to your data that the calling function wasn't expecting. This class of bug can take many forms, and both of the major bugs that led to the DAO's collapse were bugs of this sort.
+
+I miitgated the risks of race conditions by:
+
+* Not allowing more than one function at one time to change a shared state variable.
+* No external contracts are calling functions on the MediaGallery contract.
+
+## 10. Integer Overflow and Underflow
+
+### 1.0 Overflow
+
+If a balance reaches the maximum uint value (2^256) it will circle back to zero. In order to mitigate this from occuring I have limited the integers a user can impact to just the following: 
+
+* Total number of media assets.
+* Unique identifier for a media asset.
+
+### 2.0 Underflow
+
+I don't see a scenario where this would be a risk given the MediaGallery contract's use of integers.
+
+## 11 Gas Limits
+
+I have mitigated gas limit issues to the best of my ability by: 
+
+* Not looping over arrays of an un-determined length.
+* I validate the length of data inputted by users.

contracts/MediaGallery.sol

@@ -1,4 +1,4 @@
-pragma solidity ^0.4.24;
+pragma solidity 0.4.24;
 
 // import Open Zeppelin contract for Owner function authorization
 import "../node_modules/openzeppelin-solidity/contracts/ownership/Ownable.sol";
@@ -14,7 +14,7 @@ import "../node_modules/openzeppelin-solidity/contracts/ownership/Ownable.sol";
 */
 contract MediaGallery is Ownable {
 
-    bool public isStopped;      //State variabe used to stop/start the contract
+    bool public isStopped;      //State variable used to stop/start the contract
     uint public mediaCounter;   //A count of the total media assets added to the contract
     uint public maximumNameLength = 125; //The maximum allowed length of a media asset name
     uint public maximumDescLength = 250; //The maximum allowed length of a media asset description
@@ -132,19 +132,11 @@ contract MediaGallery is Ownable {
       * @param _user Address to retrieve media identifiers for.
       * @return mediaAssetIds An array of media identifiers for provided address.
     */
-    function getMediaByAddress(address _user) public view returns (uint[]) {
+    function getNumberMediaByAddress(address _user) public view returns (uint) {
         //Check to see if the address has provided any media assets yet
         require(mediaDatabase[_user].length > 0, "No media found for this user");
 
-        uint[] memory mediaAssetIds = new uint[](mediaDatabase[_user].length);
-
-        uint numberOfMediaAssets = 0;
-
-        for(uint i = 0; i < mediaDatabase[_user].length;  i++) {
-            mediaAssetIds[numberOfMediaAssets] = mediaDatabase[_user][i].id;
-            numberOfMediaAssets++;
-        }
-        return mediaAssetIds;
+        return mediaDatabase[_user].length;
     }
 
     /** 

coverage.json

@@ -1 +1 @@
-{"contracts/MediaGallery.sol":{"l":{"53":7,"57":6,"62":1,"67":1,"87":6,"88":5,"89":4,"90":3,"92":2,"101":2,"103":2,"104":2,"105":2,"106":2,"107":2,"108":2,"109":2,"110":2,"111":2,"114":2,"117":2,"127":2,"137":2,"139":1,"141":1,"143":1,"144":1,"145":1,"147":1,"156":6,"157":6,"158":6,"159":1,"162":5,"171":5,"172":5,"173":5,"174":1,"177":4,"186":4,"187":4,"188":4,"189":1,"192":3,"201":3,"202":3,"203":3,"204":1,"207":2},"path":"/Users/grahamjackson/Projects/media_gallery/contracts/MediaGallery.sol","s":{"1":7,"2":1,"3":1,"4":6,"5":5,"6":4,"7":3,"8":2,"9":2,"10":2,"11":2,"12":2,"13":2,"14":2,"15":2,"16":2,"17":2,"18":2,"19":2,"20":1,"21":1,"22":1,"23":1,"24":1,"25":6,"26":6,"27":6,"28":1,"29":5,"30":5,"31":5,"32":5,"33":1,"34":4,"35":4,"36":4,"37":4,"38":1,"39":3,"40":3,"41":3,"42":3,"43":1,"44":2},"b":{"1":[6,1],"2":[5,1],"3":[4,1],"4":[3,1],"5":[2,1],"6":[1,1],"7":[1,5],"8":[1,4],"9":[1,3],"10":[1,2]},"f":{"1":7,"2":1,"3":1,"4":6,"5":2,"6":6,"7":5,"8":4,"9":3},"fnMap":{"1":{"name":"stoppedInEmergency","line":52,"loc":{"start":{"line":52,"column":4},"end":{"line":52,"column":29}}},"2":{"name":"stopContract","line":61,"loc":{"start":{"line":61,"column":4},"end":{"line":61,"column":42}}},"3":{"name":"resumeContract","line":66,"loc":{"start":{"line":66,"column":4},"end":{"line":66,"column":44}}},"4":{"name":"addMedia","line":79,"loc":{"start":{"line":79,"column":4},"end":{"line":86,"column":38}}},"5":{"name":"getMediaByAddress","line":135,"loc":{"start":{"line":135,"column":4},"end":{"line":135,"column":72}}},"6":{"name":"validateName","line":155,"loc":{"start":{"line":155,"column":4},"end":{"line":155,"column":64}}},"7":{"name":"validateDesc","line":170,"loc":{"start":{"line":170,"column":4},"end":{"line":170,"column":71}}},"8":{"name":"validateTags","line":185,"loc":{"start":{"line":185,"column":4},"end":{"line":185,"column":64}}},"9":{"name":"validateHashLength","line":200,"loc":{"start":{"line":200,"column":4},"end":{"line":200,"column":70}}}},"statementMap":{"1":{"start":{"line":53,"column":8},"end":{"line":53,"column":2693}},"2":{"start":{"line":62,"column":8},"end":{"line":62,"column":23}},"3":{"start":{"line":67,"column":8},"end":{"line":67,"column":24}},"4":{"start":{"line":87,"column":8},"end":{"line":87,"column":58}},"5":{"start":{"line":88,"column":8},"end":{"line":88,"column":72}},"6":{"start":{"line":89,"column":8},"end":{"line":89,"column":58}},"7":{"start":{"line":90,"column":8},"end":{"line":90,"column":69}},"8":{"start":{"line":101,"column":8},"end":{"line":101,"column":34}},"9":{"start":{"line":103,"column":8},"end":{"line":103,"column":37}},"10":{"start":{"line":104,"column":8},"end":{"line":104,"column":32}},"11":{"start":{"line":105,"column":8},"end":{"line":105,"column":46}},"12":{"start":{"line":106,"column":8},"end":{"line":106,"column":39}},"13":{"start":{"line":107,"column":8},"end":{"line":107,"column":37}},"14":{"start":{"line":108,"column":8},"end":{"line":108,"column":42}},"15":{"start":{"line":109,"column":8},"end":{"line":109,"column":32}},"16":{"start":{"line":110,"column":8},"end":{"line":110,"column":42}},"17":{"start":{"line":111,"column":8},"end":{"line":111,"column":42}},"18":{"start":{"line":117,"column":8},"end":{"line":117,"column":5020}},"19":{"start":{"line":137,"column":8},"end":{"line":137,"column":79}},"20":{"start":{"line":139,"column":8},"end":{"line":139,"column":76}},"21":{"start":{"line":141,"column":8},"end":{"line":141,"column":35}},"22":{"start":{"line":143,"column":8},"end":{"line":143,"column":5877}},"23":{"start":{"line":144,"column":12},"end":{"line":144,"column":74}},"24":{"start":{"line":147,"column":8},"end":{"line":147,"column":28}},"25":{"start":{"line":156,"column":8},"end":{"line":156,"column":44}},"26":{"start":{"line":157,"column":8},"end":{"line":157,"column":42}},"27":{"start":{"line":158,"column":8},"end":{"line":158,"column":6425}},"28":{"start":{"line":159,"column":12},"end":{"line":159,"column":24}},"29":{"start":{"line":162,"column":8},"end":{"line":162,"column":19}},"30":{"start":{"line":171,"column":8},"end":{"line":171,"column":51}},"31":{"start":{"line":172,"column":8},"end":{"line":172,"column":42}},"32":{"start":{"line":173,"column":8},"end":{"line":173,"column":6896}},"33":{"start":{"line":174,"column":12},"end":{"line":174,"column":24}},"34":{"start":{"line":177,"column":8},"end":{"line":177,"column":19}},"35":{"start":{"line":186,"column":8},"end":{"line":186,"column":44}},"36":{"start":{"line":187,"column":8},"end":{"line":187,"column":41}},"37":{"start":{"line":188,"column":8},"end":{"line":188,"column":7331}},"38":{"start":{"line":189,"column":12},"end":{"line":189,"column":24}},"39":{"start":{"line":192,"column":8},"end":{"line":192,"column":19}},"40":{"start":{"line":201,"column":8},"end":{"line":201,"column":44}},"41":{"start":{"line":202,"column":8},"end":{"line":202,"column":41}},"42":{"start":{"line":203,"column":8},"end":{"line":203,"column":7776}},"43":{"start":{"line":204,"column":12},"end":{"line":204,"column":24}},"44":{"start":{"line":207,"column":8},"end":{"line":207,"column":19}}},"branchMap":{"1":{"line":53,"type":"if","locations":[{"start":{"line":53,"column":8},"end":{"line":53,"column":8}},{"start":{"line":53,"column":8},"end":{"line":53,"column":8}}]},"2":{"line":87,"type":"if","locations":[{"start":{"line":87,"column":8},"end":{"line":87,"column":8}},{"start":{"line":87,"column":8},"end":{"line":87,"column":8}}]},"3":{"line":88,"type":"if","locations":[{"start":{"line":88,"column":8},"end":{"line":88,"column":8}},{"start":{"line":88,"column":8},"end":{"line":88,"column":8}}]},"4":{"line":89,"type":"if","locations":[{"start":{"line":89,"column":8},"end":{"line":89,"column":8}},{"start":{"line":89,"column":8},"end":{"line":89,"column":8}}]},"5":{"line":90,"type":"if","locations":[{"start":{"line":90,"column":8},"end":{"line":90,"column":8}},{"start":{"line":90,"column":8},"end":{"line":90,"column":8}}]},"6":{"line":137,"type":"if","locations":[{"start":{"line":137,"column":8},"end":{"line":137,"column":8}},{"start":{"line":137,"column":8},"end":{"line":137,"column":8}}]},"7":{"line":158,"type":"if","locations":[{"start":{"line":158,"column":8},"end":{"line":158,"column":8}},{"start":{"line":158,"column":8},"end":{"line":158,"column":8}}]},"8":{"line":173,"type":"if","locations":[{"start":{"line":173,"column":8},"end":{"line":173,"column":8}},{"start":{"line":173,"column":8},"end":{"line":173,"column":8}}]},"9":{"line":188,"type":"if","locations":[{"start":{"line":188,"column":8},"end":{"line":188,"column":8}},{"start":{"line":188,"column":8},"end":{"line":188,"column":8}}]},"10":{"line":203,"type":"if","locations":[{"start":{"line":203,"column":8},"end":{"line":203,"column":8}},{"start":{"line":203,"column":8},"end":{"line":203,"column":8}}]}}}}
+{"contracts/MediaGallery.sol":{"l":{"53":7,"57":6,"62":1,"67":1,"87":6,"88":5,"89":4,"90":3,"92":2,"101":2,"103":2,"104":2,"105":2,"106":2,"107":2,"108":2,"109":2,"110":2,"111":2,"114":2,"117":2,"127":2,"137":2,"139":1,"148":6,"149":6,"150":6,"151":1,"154":5,"163":5,"164":5,"165":5,"166":1,"169":4,"178":4,"179":4,"180":4,"181":1,"184":3,"193":3,"194":3,"195":3,"196":1,"199":2},"path":"/Users/grahamjackson/Projects/media_gallery/contracts/MediaGallery.sol","s":{"1":7,"2":1,"3":1,"4":6,"5":5,"6":4,"7":3,"8":2,"9":2,"10":2,"11":2,"12":2,"13":2,"14":2,"15":2,"16":2,"17":2,"18":2,"19":2,"20":1,"21":6,"22":6,"23":6,"24":1,"25":5,"26":5,"27":5,"28":5,"29":1,"30":4,"31":4,"32":4,"33":4,"34":1,"35":3,"36":3,"37":3,"38":3,"39":1,"40":2},"b":{"1":[6,1],"2":[5,1],"3":[4,1],"4":[3,1],"5":[2,1],"6":[1,1],"7":[1,5],"8":[1,4],"9":[1,3],"10":[1,2]},"f":{"1":7,"2":1,"3":1,"4":6,"5":2,"6":6,"7":5,"8":4,"9":3},"fnMap":{"1":{"name":"stoppedInEmergency","line":52,"loc":{"start":{"line":52,"column":4},"end":{"line":52,"column":29}}},"2":{"name":"stopContract","line":61,"loc":{"start":{"line":61,"column":4},"end":{"line":61,"column":42}}},"3":{"name":"resumeContract","line":66,"loc":{"start":{"line":66,"column":4},"end":{"line":66,"column":44}}},"4":{"name":"addMedia","line":79,"loc":{"start":{"line":79,"column":4},"end":{"line":86,"column":38}}},"5":{"name":"getNumberMediaByAddress","line":135,"loc":{"start":{"line":135,"column":4},"end":{"line":135,"column":76}}},"6":{"name":"validateName","line":147,"loc":{"start":{"line":147,"column":4},"end":{"line":147,"column":64}}},"7":{"name":"validateDesc","line":162,"loc":{"start":{"line":162,"column":4},"end":{"line":162,"column":71}}},"8":{"name":"validateTags","line":177,"loc":{"start":{"line":177,"column":4},"end":{"line":177,"column":64}}},"9":{"name":"validateHashLength","line":192,"loc":{"start":{"line":192,"column":4},"end":{"line":192,"column":70}}}},"statementMap":{"1":{"start":{"line":53,"column":8},"end":{"line":53,"column":2692}},"2":{"start":{"line":62,"column":8},"end":{"line":62,"column":23}},"3":{"start":{"line":67,"column":8},"end":{"line":67,"column":24}},"4":{"start":{"line":87,"column":8},"end":{"line":87,"column":58}},"5":{"start":{"line":88,"column":8},"end":{"line":88,"column":72}},"6":{"start":{"line":89,"column":8},"end":{"line":89,"column":58}},"7":{"start":{"line":90,"column":8},"end":{"line":90,"column":69}},"8":{"start":{"line":101,"column":8},"end":{"line":101,"column":34}},"9":{"start":{"line":103,"column":8},"end":{"line":103,"column":37}},"10":{"start":{"line":104,"column":8},"end":{"line":104,"column":32}},"11":{"start":{"line":105,"column":8},"end":{"line":105,"column":46}},"12":{"start":{"line":106,"column":8},"end":{"line":106,"column":39}},"13":{"start":{"line":107,"column":8},"end":{"line":107,"column":37}},"14":{"start":{"line":108,"column":8},"end":{"line":108,"column":42}},"15":{"start":{"line":109,"column":8},"end":{"line":109,"column":32}},"16":{"start":{"line":110,"column":8},"end":{"line":110,"column":42}},"17":{"start":{"line":111,"column":8},"end":{"line":111,"column":42}},"18":{"start":{"line":117,"column":8},"end":{"line":117,"column":5019}},"19":{"start":{"line":137,"column":8},"end":{"line":137,"column":79}},"20":{"start":{"line":139,"column":8},"end":{"line":139,"column":42}},"21":{"start":{"line":148,"column":8},"end":{"line":148,"column":44}},"22":{"start":{"line":149,"column":8},"end":{"line":149,"column":42}},"23":{"start":{"line":150,"column":8},"end":{"line":150,"column":6136}},"24":{"start":{"line":151,"column":12},"end":{"line":151,"column":24}},"25":{"start":{"line":154,"column":8},"end":{"line":154,"column":19}},"26":{"start":{"line":163,"column":8},"end":{"line":163,"column":51}},"27":{"start":{"line":164,"column":8},"end":{"line":164,"column":42}},"28":{"start":{"line":165,"column":8},"end":{"line":165,"column":6607}},"29":{"start":{"line":166,"column":12},"end":{"line":166,"column":24}},"30":{"start":{"line":169,"column":8},"end":{"line":169,"column":19}},"31":{"start":{"line":178,"column":8},"end":{"line":178,"column":44}},"32":{"start":{"line":179,"column":8},"end":{"line":179,"column":41}},"33":{"start":{"line":180,"column":8},"end":{"line":180,"column":7042}},"34":{"start":{"line":181,"column":12},"end":{"line":181,"column":24}},"35":{"start":{"line":184,"column":8},"end":{"line":184,"column":19}},"36":{"start":{"line":193,"column":8},"end":{"line":193,"column":44}},"37":{"start":{"line":194,"column":8},"end":{"line":194,"column":41}},"38":{"start":{"line":195,"column":8},"end":{"line":195,"column":7487}},"39":{"start":{"line":196,"column":12},"end":{"line":196,"column":24}},"40":{"start":{"line":199,"column":8},"end":{"line":199,"column":19}}},"branchMap":{"1":{"line":53,"type":"if","locations":[{"start":{"line":53,"column":8},"end":{"line":53,"column":8}},{"start":{"line":53,"column":8},"end":{"line":53,"column":8}}]},"2":{"line":87,"type":"if","locations":[{"start":{"line":87,"column":8},"end":{"line":87,"column":8}},{"start":{"line":87,"column":8},"end":{"line":87,"column":8}}]},"3":{"line":88,"type":"if","locations":[{"start":{"line":88,"column":8},"end":{"line":88,"column":8}},{"start":{"line":88,"column":8},"end":{"line":88,"column":8}}]},"4":{"line":89,"type":"if","locations":[{"start":{"line":89,"column":8},"end":{"line":89,"column":8}},{"start":{"line":89,"column":8},"end":{"line":89,"column":8}}]},"5":{"line":90,"type":"if","locations":[{"start":{"line":90,"column":8},"end":{"line":90,"column":8}},{"start":{"line":90,"column":8},"end":{"line":90,"column":8}}]},"6":{"line":137,"type":"if","locations":[{"start":{"line":137,"column":8},"end":{"line":137,"column":8}},{"start":{"line":137,"column":8},"end":{"line":137,"column":8}}]},"7":{"line":150,"type":"if","locations":[{"start":{"line":150,"column":8},"end":{"line":150,"column":8}},{"start":{"line":150,"column":8},"end":{"line":150,"column":8}}]},"8":{"line":165,"type":"if","locations":[{"start":{"line":165,"column":8},"end":{"line":165,"column":8}},{"start":{"line":165,"column":8},"end":{"line":165,"column":8}}]},"9":{"line":180,"type":"if","locations":[{"start":{"line":180,"column":8},"end":{"line":180,"column":8}},{"start":{"line":180,"column":8},"end":{"line":180,"column":8}}]},"10":{"line":195,"type":"if","locations":[{"start":{"line":195,"column":8},"end":{"line":195,"column":8}},{"start":{"line":195,"column":8},"end":{"line":195,"column":8}}]}}}}