Increase code coverage

* Added .solcover.js
  - skip lab folders that aren't tested yet

* In deploy-example.js,
  - export prepareInitializerArgs for use in tests

* In faq.md,
  - fix typo

* In FismoAccess.sol,
  - remove FismoStore import and use, since FismoView.getStore method is inherited

* In FismoClone,
  - remove FismoStore import and use, since FismoView.getStore method is inherited
  - in init method, revert with ALREADY_INITIALIZED if owner address is already set

* In FismoConstants.sol,
  - Added ALREADY_INITIALIZED, STATE_EXISTS
  - updated text of INITIALIZER_REVERTED and GUARD_REVERTED

* In FismoOperate.sol,
  - use getState rather than getStateIndex and direct mapping access

* In FismoTools.sol,
   - refactor/renamed enforceHasContractCode to requireContractCode

* In FismoUpdate.sol,
  - in installAndInitializeMachine,
    - refactor/renamed enforceHasContractCode to requireContractCode
  - in addState,
    - pass false instead of index to storeState
  - in updateState,
    - remove fetch of state index
    - pass true instead of index to storeState
  - in addTransition,
    - remove fetch of machine
    - remove fetch of state index
    - pass true instead of index to storeState
  - in storeState,
    - change _index arg to _shouldExist
    - pass _shouldExist instead of index to getState

* In FismoView.sol
  - in getState,
    - added bool _shouldExist arg
    - after fetching revert if state existence doesn't match the _shouldExist

* In IFismoUpdate.sol and IFismoUpdate.md,
  - updated doc

* In LockableDoorGuards.sol,
  - imported FismoTools and FismoStore
  - extended FismoTools
  - in initialize,
    - require _keyToken to not be zero address, and revert with no reason (for test)
    - require contract code, reverting with CODELESS_INITIALIZER (for test)
  - in LockableDoor_Locked_Exit,
    - require user not be the contract owner, reverting with no reason (for test)

* In revert-reasons.js,
  - added ALREADY_INTIALIZED
  - updated text of INITIALIZER_REVERTED and GUARD_REVERTED

* In FismoTest.js,
  - import prepareInitializerArgs and deployTokens
  - in both IFismoClone methods contexts
    - added init() context
      - "Should revert if not called by cloneFismo on an instance it created"
  - in IFismoUpdate methods context
    - added installAndInitializeMachine() context
      - "Should install a valid guarded Machine and initialize it"
      - "Should revert with initializer's revert reason if it reverts with one"
      - "Should revert with generic reason if initializer reverts without one"
      - Should revert if operator address is zero address"
      - "Should revert if machine id is invalid"
      - "Should revert if machine already exists"
      - "Should revert if a state id in a state is invalid"
      - "Should revert if an action id in a transition is invalid"
      -"Should revert if a target state id in a transition is invalid"
  - in IFismoView methods context
      - added getOwner() context
        - "Should return the owner of the Fismo instance"

* In LockableDoorTest.js,
  - in invokeAction() context,
    - added test "Should revert with a generic reason if guard reverts without one"

* In README.md,
  - added link to FSM wikipedia article

.solcover.js

@@ -0,0 +1,3 @@
+module.exports = {
+    skipFiles: ['lab/NightClub','lab/StopWatch','lab/Tokens']
+};

README.md

@@ -9,18 +9,17 @@ There are standards for tokens that allow us to represent things like currency,
 Each user's position on their journey through a state machine is recorded, and can be publicly queried by anyone. Progress can be controlled by the tokens a user holds. Likewise, tokens could be transferred to a user when they or take some action or arrive at some waypoint.
 
 * 💥 Cheaply clone Fismo on Ethereum or deploy to any EVM
-* 💥 Configure and install a virtually unlimited number of FSMs
+* 💥 Configure and install a virtually unlimited number of machines
 * 💥 Deploy custom logic to be triggered by any state transition
 * 💥 Deploy custom logic for controlling access to your machines
 * 💥 Use off-chain metadata to describe states in any medium
 
-
 ### Status 
 ### [![Node.js CI](https://github.com/cliffhall/Fismo/actions/workflows/node.js.yml/badge.svg)](https://github.com/cliffhall/Fismo/actions/workflows/node.js.yml) 🔬 ![85%](https://progress-bar.dev/85/?title=Progress&width=120&color=000000)
 
 Done or in progress are:
 - ✅ Science! a working [Deterministic Selector Proxy](docs/about.md#experimentdeterministicselectorproxy) implementation
-- ✅ A robust Finite State Machine protocol
+- ✅ A robust [Finite State Machine](https://en.wikipedia.org/wiki/Finite-state_machine) protocol
 - ✅ Minimal clones for cheap deployments ($40 vs $2000)!!!
 - ✅ Initialization and access of machine-specific storage slots
 - ✅ Clear and complete interface documentation and inline code comments

contracts/components/FismoAccess.sol

@@ -2,7 +2,6 @@
 pragma solidity ^0.8.0;
 
 import { FismoView } from "./FismoView.sol";
-import { FismoStore } from "../domain/FismoStore.sol";
 
 /**
  * @title FismoControl
@@ -14,7 +13,7 @@ import { FismoStore } from "../domain/FismoStore.sol";
 contract FismoAccess is FismoView {
 
     modifier onlyOwner() {
-        require(msg.sender == FismoStore.getStore().owner, ONLY_OWNER);
+        require(msg.sender == getStore().owner, ONLY_OWNER);
         _;
     }
 

contracts/components/FismoClone.sol

@@ -29,13 +29,12 @@ contract FismoClone is IFismoClone, FismoUpdate  {
     external
     override
     {
-        address owner = FismoStore.getStore().owner;
-        require(owner == address(0), INVALID_ADDRESS);
+        address owner = getStore().owner;
+        require(owner == address(0), ALREADY_INITIALIZED);
         setOwner(_owner);
         setIsFismo(false);
     }
 
-
     function cloneFismo()
     external
     override

contracts/components/FismoOperate.sol

@@ -65,11 +65,8 @@ contract FismoOperate is IFismoOperate, FismoUpdate  {
         // Make sure transition was found
         require(valid == true, NO_SUCH_ACTION);
 
-        // Get the next state's index in the machine's states array
-        index = getStateIndex(_machineId, transition.targetStateId);
-
         // Get the next state
-        State storage nextState = machine.states[index];
+        State storage nextState = getState(_machineId, transition.targetStateId, true);
 
         // Create the action response
         response.machineName = machine.name;

contracts/components/FismoTools.sol

@@ -96,14 +96,14 @@ contract FismoTools is FismoTypes, FismoConstants {
     }
 
     /**
-     * @notice make sure the given address has code
+     * @notice Verify an address is a contract and not an EOA
      *
      * Reverts if address has no contract code
      *
      * @param _contract - the contract to check
      * @param _errorMessage - the revert reason to throw
      */
-    function enforceHasContractCode(address _contract, string memory _errorMessage) internal view {
+    function requireContractCode(address _contract, string memory _errorMessage) internal view {
         uint256 contractSize;
         assembly {
             contractSize := extcodesize(_contract)

contracts/components/FismoUpdate.sol

@@ -73,6 +73,7 @@ contract FismoUpdate is IFismoUpdate, FismoAccess {
      * - Operator address is zero
      * - Machine id is not valid for Machine name
      * - Machine already exists
+     * - Initializer has no code
      * - Initializer call reverts
      *
      * @param _machine - the machine definition to install
@@ -89,7 +90,7 @@ contract FismoUpdate is IFismoUpdate, FismoAccess {
     onlyOwner
     {
         // Make sure this is actually a contract
-        enforceHasContractCode(_initializer, CODELESS_INITIALIZER);
+        requireContractCode(_initializer, CODELESS_INITIALIZER);
 
         // Add the new machine to Fismo's storage
         addMachine(_machine);
@@ -149,7 +150,7 @@ contract FismoUpdate is IFismoUpdate, FismoAccess {
         mapStateIndex(_machineId, _state.id, index);
 
         // Store the new state in the machine's states array
-        storeState(machine, _state, index);
+        storeState(machine, _state, false);
 
         // Alert listeners to change of state
         emit StateAdded(_machineId, _state.id, _state.name);
@@ -188,12 +189,8 @@ contract FismoUpdate is IFismoUpdate, FismoAccess {
         // Get the machine
         Machine storage machine = getMachine(_machineId);
 
-        // Make sure state exists
-        uint256 index = getStateIndex(_machineId, _state.id);
-        require(machine.states[index].id == _state.id, NO_SUCH_STATE);
-
         // Overwrite the state in the machine's states array
-        storeState(machine, _state, index);
+        storeState(machine, _state, true);
 
         // Alert listeners to change of state
         emit StateUpdated(_machineId, _state.id, _state.name);
@@ -230,20 +227,14 @@ contract FismoUpdate is IFismoUpdate, FismoAccess {
         // Make sure target state id is valid
         require(_transition.targetStateId == nameToId(_transition.targetStateName), INVALID_TARGET_ID);
 
-        // Get the machine
-        Machine storage machine = getMachine(_machineId);
-
-        // Get the state's index in the machine's states array
-        uint256 index = getStateIndex(_machineId, _stateId);
-
         // Get the target state
-        State storage state = machine.states[index];
+        State storage state = getState(_machineId, _stateId, true);
 
         // Zero init a new transitions array element in storage
         state.transitions.push();
 
         // Get the new transition's storage index in the state's transitions array
-        index = state.transitions.length - 1;
+        uint256 index = state.transitions.length - 1;
 
         // Overwrite the state in the machine's states array
         Transition storage transition = state.transitions[index];
@@ -324,19 +315,19 @@ contract FismoUpdate is IFismoUpdate, FismoAccess {
      *
      * @param _machine - the machine's storage location
      * @param _state - the state's storage location
-     * @param _index - the state's index within the machine's states array
+     * @param _shouldExist - true if the state should exist
      */
-    function storeState(Machine storage _machine, State memory _state, uint256 _index)
+    function storeState(Machine storage _machine, State memory _state, bool _shouldExist)
     internal
     {
         // Overwrite the state in the machine's states array
-        State storage state = _machine.states[_index];
+        State storage state = getState(_machine.id, _state.id, _shouldExist);
         state.id = _state.id;
         state.name = _state.name;
         state.exitGuarded = _state.exitGuarded;
         state.enterGuarded = _state.enterGuarded;
         if (_state.exitGuarded || _state.enterGuarded) {
-            enforceHasContractCode(_state.guardLogic, CODELESS_GUARD);
+            requireContractCode(_state.guardLogic, CODELESS_GUARD);
             state.guardLogic = _state.guardLogic;
         }
 

contracts/components/FismoView.sol

@@ -173,13 +173,14 @@ contract FismoView is IFismoView, FismoTools {
      * @notice Get a state by Machine id and State id.
      *
      * Reverts if
-     * - State does not exist
+     * - _verify is true and State does not exist
      *
      * @param _machineId - the id of the machine
      * @param _stateId - the id of the state
+     * @param _shouldExist - true if the state should already exist
      * @return state - the state definition
      */
-    function getState(bytes4 _machineId, bytes4 _stateId)
+    function getState(bytes4 _machineId, bytes4 _stateId, bool _shouldExist)
     internal
     view
     returns (State storage state) {
@@ -193,8 +194,12 @@ contract FismoView is IFismoView, FismoTools {
         // Get the state
         state = machine.states[index];
 
-        // Make sure state exists
-        require(state.id == _stateId, NO_SUCH_STATE);
+        // Verify expected existence or non-existence of State
+        if (_shouldExist) {
+            require(state.id == _stateId, NO_SUCH_STATE);
+        } else {
+            require(state.id == 0, STATE_EXISTS);
+        }
     }
 
     /**

contracts/domain/FismoConstants.sol

@@ -11,27 +11,29 @@ contract FismoConstants {
     // Revert Reasons
     string internal constant MULTIPLICITY = "Can't clone a clone";
 
-    string internal constant INVALID_ADDRESS = "Invalid address";
+    string internal constant ALREADY_INITIALIZED = "Already initialized";
 
     string internal constant ONLY_OWNER = "Only owner may call";
     string internal constant ONLY_OPERATOR = "Only operator may call";
 
     string internal constant MACHINE_EXISTS = "Machine already exists";
+    string internal constant STATE_EXISTS = "State already exists";
 
     string internal constant NO_SUCH_GUARD = "No such guard";
     string internal constant NO_SUCH_MACHINE = "No such machine";
     string internal constant NO_SUCH_STATE = "No such state";
     string internal constant NO_SUCH_ACTION = "No such action";
 
+    string internal constant INVALID_ADDRESS = "Invalid address";
     string internal constant INVALID_OPERATOR_ADDR = "Invalid operator address";
     string internal constant INVALID_MACHINE_ID = "Invalid machine id";
     string internal constant INVALID_STATE_ID = "Invalid state id";
     string internal constant INVALID_ACTION_ID = "Invalid action id";
     string internal constant INVALID_TARGET_ID = "Invalid target state id";
 
     string internal constant CODELESS_INITIALIZER = "Initializer address not a contract";
-    string internal constant INITIALIZER_REVERTED = "Initializer function reverted";
+    string internal constant INITIALIZER_REVERTED = "Initializer function reverted, no reason given";
     string internal constant CODELESS_GUARD = "Guard address not a contract";
-    string internal constant GUARD_REVERTED = "Guard function reverted";
+    string internal constant GUARD_REVERTED = "Guard function reverted, no reason given";
 
 }

contracts/interfaces/IFismoUpdate.sol

@@ -98,6 +98,7 @@ interface IFismoUpdate {
      * - Operator address is zero
      * - Machine id is not valid for Machine name
      * - Machine already exists
+     * - Initializer has no code
      * - Initializer call reverts
      *
      * @param _machine - the machine definition to install

contracts/lab/LockableDoor/LockableDoorGuards.sol

@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: BSD-3-Clause
 pragma solidity ^0.8.0;
 
+import "../../components/FismoTools.sol";
+import "../../domain/FismoStore.sol";
+
 /**
  * @notice KeyToken is the Fismo ERC-20, which we only check for a balance of
  */
@@ -11,10 +14,13 @@ interface KeyToken {
 /**
  * @notice Transition guard functions
  *
+ * Note:
+ * Using FismoTools here to test for code at key token address
+ *
  * - Machine: LockableDoor
  * - Guards: Locked/Exit
  */
-contract LockableDoorGuards {
+contract LockableDoorGuards is FismoTools {
 
     // -------------------------------------------------------------------------
     // MACHINE STORAGE
@@ -51,6 +57,14 @@ contract LockableDoorGuards {
     function initialize(address _keyToken)
     external
     {
+        // Make sure _keyToken isn't the zero address
+        // Note: specifically testing a revert with no reason here
+        require(_keyToken != address(0));
+
+        // Make sure _keyToken address has code
+        // Note: specifically testing a revert with a reason here
+        requireContractCode(_keyToken, CODELESS_INITIALIZER);
+
         // Initialize market config params
         lockableDoorSlot().keyToken = KeyToken(_keyToken);
     }
@@ -66,6 +80,10 @@ contract LockableDoorGuards {
     view
     returns(string memory)
     {
+        // Make sure _user isn't the owner address
+        // Note: specifically testing a revert with no reason here
+        require(_user != FismoStore.getStore().owner);
+
         // User must have key to unlock door
         bool hasKey = lockableDoorSlot().keyToken.balanceOf(_user) > 0;
         require(hasKey, "User must hold key to unlock.");

docs/api/IFismoUpdate.md

@@ -179,6 +179,7 @@ Install a Fismo Machine and initialize it.
 - Operator address is zero
 - Machine id is not valid for Machine name
 - Machine already exists
+- Initializer has no code
 - Initializer call reverts
 
 **Signature**

docs/faq.md

@@ -37,7 +37,7 @@ For instance, even though multiple people may interact with an FSM-based auction
 the auction that's being tracked (pending, open, closed). What the users are allowed to do is based on the state of 
 the machine. The user has no "state" to speak of.
 
-Fismo flips all this on it's head. It tracks the state of _each user in every machine they interact with_.
+Fismo flips all this on its head. It tracks the state of _each user in every machine they interact with_.
 
 A Machine is a combination of static configuration and guard logic that governs the current state of a user on that machine.
 

scripts/config/revert-reasons.js

@@ -10,6 +10,8 @@ exports.RevertReasons = {
     // --------------------------------------------------------
     MULTIPLICITY: "Can't clone a clone",
 
+    ALREADY_INITIALIZED: "Already initialized",
+
     ONLY_OWNER: "Only owner may call",
     ONLY_OPERATOR: "Only operator may call",
 
@@ -28,10 +30,10 @@ exports.RevertReasons = {
     INVALID_TARGET_ID: "Invalid target state id",
 
     CODELESS_INITIALIZER: "Initializer address not a contract",
-    INITIALIZER_REVERTED: "Initializer function reverted",
+    INITIALIZER_REVERTED: "Initializer function reverted, no reason given",
 
     CODELESS_GUARD: "Guard address not a contract",
-    GUARD_REVERTED: "Guard function reverted",
+    GUARD_REVERTED: "Guard function reverted, no reason given",
 
     // --------------------------------------------------------
     // LOCKABLE DOOR EXAMPLE

scripts/deploy/deploy-example.js

@@ -126,4 +126,5 @@ function prepareInitializerArgs(example, guards, tokens) {
 
 }
 
-exports.deployExample = deployExample;
+exports.deployExample = deployExample;
+exports.prepareInitializerArgs = prepareInitializerArgs;

test/lab/LockableDoorTest.js

@@ -193,6 +193,48 @@ describe("Lockable Door Machine", function() {
                         .to.revertedWith(RevertReasons.NO_SUCH_ACTION);
                 });
 
+                it("Should revert with a generic reason if guard reverts without one", async function () {
+
+                    // ---------------------------------------------------------------------------------------------
+                    // LOCK THE DOOR FIRST
+                    // ---------------------------------------------------------------------------------------------
+
+                    // Initial state is "Closed", action is "Lock", target state is "Locked"
+                    action = "Lock";
+                    actionId = nameToId(action);
+                    priorState = "Closed";
+                    nextState = "Locked";
+
+                    // Invoke the action via the Operator
+                    await operator.connect(deployer).invokeAction(machine.id, actionId);
+
+                    // ---------------------------------------------------------------------------------------------
+                    // GIVE USER THE KEY
+                    // ---------------------------------------------------------------------------------------------
+
+                    // One key
+                    amountToMint = "1";
+
+                    // Mint the key token and check for the event
+                    await expect(keyToken.connect(deployer).mintSample(deployer.address, amountToMint))
+                        .to.emit(keyToken, 'Transfer')
+                        .withArgs(ethers.constants.AddressZero, deployer.address, amountToMint);
+
+                    // ---------------------------------------------------------------------------------------------
+                    // NOW ATTEMPT TO UNLOCK THE DOOR
+                    // ---------------------------------------------------------------------------------------------
+
+                    // Current state is "Locked", action is "Unlock", target state is "Closed"
+                    action = "Unlock";
+                    actionId = nameToId(action);
+
+                    // Attempt to invoke the action via the Operator, checking for the generic revert reason
+                    // Note: This guard will revert with no reason if the owner attempts to use it. (Contrived)
+                    await expect(operator.connect(deployer).invokeAction(machine.id, actionId))
+                        .to.revertedWith(RevertReasons.GUARD_REVERTED);
+
+                });
+
                 it("Should revert if user doesn't have key for action 'Unlock'", async function () {
 
                     // ---------------------------------------------------------------------------------------------