Review - Architecture Docs (#149)

* Updated documentation about the Laoland architecture, its components, tests and how to create an adapter.

Co-authored-by: Jarrel deLottinville <[email protected]>
Co-authored-by: David Roon <[email protected]>
Co-authored-by: Craig Blake <[email protected]>

CONTRIBUTING.md

@@ -1,12 +1,12 @@
-A new branch should be opened for every function being changed. 
+A new branch should be opened for every function being changed.
 
 Naming of the new branch should have the name of the function being changed
-eg. "submitVote-fix-v1"
+e.g., "submitVote-fix-v1"
 
-add comments in code and commit summary to describe changes. 
+Add comments in code and commit summary to describe changes.
 
 Once, something is working, then make a PR.
 
 In the PR, request review.
 
-If a PR passes review, then merge, delete branch. 
+If a PR passes review, then merge, delete branch.

README.md

@@ -1,59 +1,95 @@
-
 [![codecov](https://codecov.io/gh/openlawteam/laoland/branch/master/graph/badge.svg?token=XZRL9RUYZE)](https://codecov.io/gh/openlawteam/laoland/)
 
 ## Overview
 
-At the LAO, we realized that even though Moloch is very useful and powerful, it has a lot of features that we don't necessarily need. Also, there are a few features that are missing and are hard to change.
+At the LAO, we realized that even though Moloch is very useful and powerful, it has many features that are not necessary for all DAOs. There are also a few features that are missing and are difficult to add.
 
 This is why we would like to introduce a more modular approach to Moloch architecture, which will give us:
 
-- Simpler code, each part would do something more specific, this means easier to understand
-- Adaptable, we will be able to adapt each part of the DAO to the needs of the ones using it without the need to audit the entire code bade every time
-- Upgradable, it should be easier to upgrade parts once the need evolves. The best example we have in mind is voting, maybe the way of voting evolve with time and it is good to be able to upgrade that economic, we can imagine some modules being used by multiple DAOs without the need to be redeployed
+- Simpler code - each module is responsible for only one function which reduces coupling and makes the system easier to understand.
+- Adaptability - each part of the DAO can be adapted to the needs of a particular DAO without the need to audit the entire code base every time.
+- Upgradability - modules can be easily upgraded as necessary. For example, as the voting process evolves over time the module responsible for managing the voting process can be upgraded without changing any other modules or the Core Contract. Modules can also be used by multiple DAOs without the need to be redeployed.
 
-Inspired by the hexagonal architecture pattern we believe that we can have additional layers of security and break the main contract into smaller contracts. With that, we create loosely coupled modules/contracts, easier to audit, and can be easily connected to the DAO.
+Inspired by the [hexagonal architecture design pattern](<https://en.wikipedia.org/wiki/Hexagonal_architecture_(software)>) we believe that we can have additional layers of security, and break the main contract into smaller contracts. With that, we create loosely coupled modules/contracts, easier to audit, and can be easily connected to the DAO.
 
-The architecture is composed by 3 main types of components:
+### Architecture
 
-**Core Module**
-- The Core module (Registry) keeps track of the state changes of the DAO.
-- The Registry tracks all the registered Adapters, Proposals, and Bank Accounts of the DAO.
-- Only Adapters are allowed to call functions from the Registry Module.
-- The Registry does not communicate with External World directly, it needs to go through an Adapter to pull or push information.
-- The Registry uses the `onlyAdapter` modifier to functions that change the state of the Registry/DAO, in the future we may want to grant different access types based on the Adapter type. It may expose some **read-only** public functions (`external` or `public`) to facilitate queries.
+![laoland_hexagon_architecture](https://user-images.githubusercontent.com/708579/106510703-096a9880-64ae-11eb-8e48-3745e36a7b80.png)
 
-**Adapters**
-- Public/External accessible functions called from External World.
-- Adapters do not keep track of the state of the DAO, they might use storage but the ideal is that any DAO relevant state change is propagated to the Registry Core Module.
-- Adapters just execute Smart Contract logic that changes the state of the DAO by calling the Registry Core Module, they also can compose complex calls that interact with External World to pull/push additional information.
-- Each Adapter is a very specialized Smart Contract designed to do one thing very well.
-- Adapters can have public access or access limited to members of the DAO (`onlyMember` modifier).
+The main design goal is to limit access to the smart contracts according at layer boundaries. The External World (i.e. RPC clients) can access the core contracts only via Adapters, never directly. Every adapter contains all the necessary logic and data to update/change the state of the DAO in the DAORegistry Contract. The Core Contract tracks all the state changes of the DAO, and an Adapter tracks only the state changes in its own context. Extensions enhance the DAO capabilities and simplify the Core Contract code. The information always flows from the External World to the Core Contracts, never the other way around. If a Core Contract needs external info, it must be provided by an Adapter and/or an Extension instead of calling External World directly.
 
-**External World**
-- RPC clients responsible for calling the Adapters public/external functions to interact with the DAO Core Module.
+There are five main components in the Laoland architecture:
 
-![laoland_architecture](https://user-images.githubusercontent.com/708579/94478554-cddf5b00-01a9-11eb-9e80-cc3c55dea492.png)
+#### External World
 
-The main idea is to limit access to the contracts according to each layer. External World (e.g: RPC clients) can access the core module only and via Adapter, never directly. Every adapter will contain all the necessary logic and data to provide to the Core module during the calls, and Core Module will keep track of the state changes in the DAO. An initial draft of this idea was implemented in the `Financing` Adapter which allows an individual to submit a request for financing/grant. The information always flows from the External World to the Core Module, never the other way around. If a Core Module needs external info, that should be provided via an Adapter instead of calling External World directly.
+The external world is essentially anything that interacts with the DAO. An example of that are RPC clients that are responsible for calling the Adapters public/external functions to pull/push data to the DAO Core Contracts and its Extensions.
 
+#### Adapters
 
-### Usage
+Adapters are well-defined, tested and extensible smart contracts that are created with a unique purpose. One Adapter is responsible for performing one or a set of tasks in a given context. With this approach we can develop adapters targeting specific use-cases, and update the DAO configurations to use these new adapters.
 
-#### Run Tests
-This project uses truffle, to run the tests, simply run:
-> npm run test
+When a new adapter is created, one needs to submit a Managing proposal to add the new adapter to the DAO. Once the proposal passes, the new adapter is added and becomes available for use.
+
+Each adapter needs to be configured with the [Access Flags](#access-control-layer) in order to access the [Core Contracts](#core-contracts), and/or [Extensions](#extensions). Otherwise the Adapter will not able to pull/push information to/from the DAO.
+
+Adapters implemented in the Laoland project:
+
+- [Configuration](https://github.com/openlawteam/laoland/blob/master/docs/adapters/Configuration.md): manages storing and retrieving per-DAO settings required by shared adapters.
+- [Financing](https://github.com/openlawteam/laoland/blob/master/docs/adapters/Financing.md): allows individuals and/or organizations to request funds to finance their projects, and the members of the DAO have the power to vote and decide which projects should be funded.
+- [GuildKick](https://github.com/openlawteam/laoland/blob/master/docs/adapters/GuildKick.md): gives the members the freedom to choose which individuals or organizations should really be part of the DAO.
+- [Managing](https://github.com/openlawteam/laoland/blob/master/docs/adapters/Managing.md): enhances the DAO capabilities by adding/updating the DAO Adapters through a voting process.
+- [OffchainVoting](https://github.com/openlawteam/laoland/blob/master/docs/adapters/OffchainVoting.md): adds the offchain voting governance process to the DAO to support gasless voting.
+- [Onboarding](https://github.com/openlawteam/laoland/blob/master/docs/adapters/Onboarding.md): triggers the process of minting internal tokens in exchange of a specific token at a fixed price.
+- [Ragequit](https://github.com/openlawteam/laoland/blob/master/docs/adapters/Ragequit.md): gives the members the freedom to choose when it is the best time to exit the DAO for any given reason.
+- [Voting](https://github.com/openlawteam/laoland/blob/master/docs/adapters/Voting.md): adds the simple on chain voting governance process to the DAO.
+- [Withdraw](https://github.com/openlawteam/laoland/blob/master/docs/adapters/Withdraw.md): allows the members to withdraw their funds from the DAO bank.
+
+Considerations:
+
+- Adapters do not keep track of the state of the DAO. An adapter might use storage to control its own state, but ideally any DAO state change must be propagated to the DAORegistry Core Contract.
+- Adapters just execute smart contract logic that changes the state of the DAO by calling the DAORegistry. They also can compose complex calls that interact with External World, other Adapters or even Extensions, to pull/push additional information.
+- The adapter must follow the rules defined by the [Template Adapter](https://github.com/openlawteam/laoland/blob/master/docs/adapters/Template.md).
+- If you want to contribute and create an Adapter, please checkout this: [How to create an Adapter](https://github.com/openlawteam/laoland/blob/master/docs/adapters/HowToCreate.md).
+
+#### Extensions
+
+Extensions are conceived to isolate the complexity of state changes from the DAORegistry contract, and to simplify the core logic. Essentially an Extension is similar to an Adapter, but the main difference is that it is used by several adapters and by the DAORegistry - which end up enhancing the DAO capabilities and the state management without cluttering the DAO core contract.
+
+- [Bank](https://github.com/openlawteam/laoland/blob/master/docs/extensions/Bank.md): adds the banking capabilities to the DAO, and keeps track of the DAO accounts and internal token balances.
+
+#### Core Contracts
 
-#### Code Coverage
-To check the code coverage report, simply run:
-> npm run coverage
+A core contract is a contract that composes the DAO itself, and directly changes the DAO state without the need of going through an Adapter. Ideally a core contract shall never pull information directly from the external world. For that we use Adapters and Extensions, and the natural information flow is always from the external world to the core contracts.
+
+- [DaoRegistry](https://github.com/openlawteam/laoland/blob/master/docs/core/DaoRegistry.md): tracks the state changes of the DAO, only adapters with proper [Access Flags](#access-control-layer) can alter the DAO state.
+- CloneFactory: creates a clone of the DAO based on its address.
+- DaoFactory: creates, initializes, and adds adapter configurations to the new DAO, and uses the CloneFactory to reduce the DAO creation transaction costs.
+- DaoConstants: defines all the constants used by the DAO contracts, and implements some helper functions to manage the Access Flags.
+
+#### Access Control Layer
+
+The Access Control Layer (ACL) is implemented using Access Flags to indicate which permissions an adapter must have in order to access and modify the DAO state. The are 3 main categories of Access Flags:
+
+- MemberFlag: `EXISTS`, `JAILED`.
+- ProposalFlag: `EXISTS`, `SPONSORED`, `PROCESSED`.
+- AclFlag: `ADD_ADAPTER`, `REMOVE_ADAPTER`, `JAIL_MEMBER`, `UNJAIL_MEMBER`, `SUBMIT_PROPOSAL`, `SPONSOR_PROPOSAL`, `PROCESS_PROPOSAL`, `UPDATE_DELEGATE_KEY`, `SET_CONFIGURATION`, `ADD_EXTENSION`, `REMOVE_EXTENSION`, `NEW_MEMBER`.
+
+The Access Flags of each adapter must be provided to the DAOFactory when the `daoFactory.addAdapters` function is called passing the new adapters. These flags will grant the access to the DAORegistry contract, and the same process must be done to grant the access of each Adapter to each Extension (function `daoFactory.configureExtension`).
+
+The Access Flags are defined in the DAORegistry using the modifier `hasAccess`. For example, a function with the modifier `hasAccess(this, AclFlag.JAIL_MEMBER)` means the adapter calling this function needs to have the Access Flag `JAIL_MEMBER` enabled, otherwise the call will revert. In order to create an Adapter with the proper Access Flags one needs to first map out all the functions that the Adapter will be calling in the DAORegistry and Extensions, and provide these Access Flags using the DAO Factory as described above.
+
+## Usage
+
+### Run Tests
+
+This project uses truffle. To run the tests, simply run:
+
+> npm run test
 
-|Coverage Graph per Contract|
-|----------------------|
-|[![graph](https://codecov.io/gh/openlawteam/laoland/branch/master/graphs/tree.svg)](undefined)|
+### Code Format
 
+To fix the Solidity code and documentation with the linter hints, simply run:
 
-#### Code Format
-To fix the Solidity code with the linter hints, simply run:
 > npm run lint:fix
 
 ## Contribute

contracts/Migrations.sol

@@ -2,18 +2,18 @@
 pragma solidity ^0.8.0;
 
 contract Migrations {
-  address public owner;
-  uint public last_completed_migration;
+    address public owner;
+    uint256 public last_completed_migration;
 
-  constructor() {
-    owner = msg.sender;
-  }
+    constructor() {
+        owner = msg.sender;
+    }
 
-  modifier restricted() {
-    if (msg.sender == owner) _;
-  }
+    modifier restricted() {
+        if (msg.sender == owner) _;
+    }
 
-  function setCompleted(uint completed) public restricted {
-    last_completed_migration = completed;
-  }
+    function setCompleted(uint256 completed) public restricted {
+        last_completed_migration = completed;
+    }
 }

contracts/adapters/Financing.sol

@@ -92,7 +92,7 @@ contract FinancingContract is IFinancing, DaoConstants, MemberGuard {
      * @dev Only members of the DAO can sponsor a financing proposal.
      * @param dao The DAO Address.
      * @param proposalId The proposal id.
-     * @param data Additional detais about the sponsorship process.
+     * @param data Additional details about the sponsorship process.
      */
     function sponsorProposal(
         DaoRegistry dao,
@@ -123,7 +123,6 @@ contract FinancingContract is IFinancing, DaoConstants, MemberGuard {
 
     /**
      * @notice Processing a financing proposal to grant the requested funds.
-     * @dev Only members of the DAO can process a financing proposal.
      * @dev Only proposals that were not processed are accepted.
      * @dev Only proposals that were sponsored are accepted.
      * @dev Only proposals that passed can get processed and have the funds released.

contracts/adapters/GuildKick.sol

@@ -55,7 +55,7 @@ contract GuildKickContract is IGuildKick, DaoConstants, MemberGuard {
     // Keeps track of all the kicks executed per DAO.
     mapping(address => mapping(bytes32 => GuildKick)) public kicks;
 
-    // Keeps track of the latest ongoing kick proposal per DAO to ensure only 1 kick happens at time.
+    // Keeps track of the latest ongoing kick proposal per DAO to ensure only 1 kick happens at a time.
     mapping(address => bytes32) public ongoingKicks;
 
     /**
@@ -69,7 +69,7 @@ contract GuildKickContract is IGuildKick, DaoConstants, MemberGuard {
      * @notice Creates a guild kick proposal, opens it for voting, and sponsors it.
      * @dev A member can not kick himself.
      * @dev Only one kick per DAO can be executed at time.
-     * @dev Only members that have shares can be kicked out.
+     * @dev Only members that have shares or loot can be kicked out.
      * @dev Proposal ids can not be reused.
      * @param dao The dao address.
      * @param proposalId The guild kick proposal id.
@@ -82,7 +82,6 @@ contract GuildKickContract is IGuildKick, DaoConstants, MemberGuard {
         address memberToKick,
         bytes calldata data
     ) external override {
-        // Checks if the sender address is not the same as the member to kick to prevent auto kick.
         IVoting votingContract = IVoting(dao.getAdapterAddress(VOTING));
         address submittedBy =
             votingContract.getSenderAddress(
@@ -91,10 +90,21 @@ contract GuildKickContract is IGuildKick, DaoConstants, MemberGuard {
                 data,
                 msg.sender
             );
+        // Checks if the sender address is not the same as the member to kick to prevent auto kick.
         require(submittedBy != memberToKick, "you can not kick yourself");
         _submitKickProposal(dao, proposalId, memberToKick, data, submittedBy);
     }
 
+    /**
+     * @notice Converts the shares into loot to remove the voting power, and sponsors the kick proposal.
+     * @dev Only members that have shares or loot can be kicked out.
+     * @dev Proposal ids can not be reused.
+     * @param dao The dao address.
+     * @param proposalId The guild kick proposal id.
+     * @param memberToKick The member address that should be kicked out of the DAO.
+     * @param data Additional information related to the kick proposal.
+     * @param submittedBy The address of the individual that created the kick proposal.
+     */
     function _submitKickProposal(
         DaoRegistry dao,
         bytes32 proposalId,
@@ -138,7 +148,7 @@ contract GuildKickContract is IGuildKick, DaoConstants, MemberGuard {
      * @dev A kick proposal must be in progress.
      * @dev Only one kick per DAO can be executed at time.
      * @dev Only active members can be kicked out.
-     * @dev Only proposals that passed the voting can be completed.
+     * @dev Only proposals that passed the voting can be set to In Progress status.
      * @param dao The dao address.
      * @param proposalId The guild kick proposal id.
      */