Contract-level introductions are missing

[fulldecent]

Contract-level documentation is user-visible (because people read Etherscan) and NatSpec specifies that it can be shown to users. Every contract should explain what it does cleary.

For contracts that are deployed (i.e. not abstracts and mix-ins) the top-level documentation should be spectacular.

Recommendation: update contract so that when the compiled contract is opened in Etherscan it will look like a million-dollar effort went into it.

Here is some artwork to get you started:

                  ┌───────────────────────┐                                  
            ┌─────│        isOpen         │─────┬───────────────┐            
            ▼     └───────────────────────┘     │               │            
┌───────────────────────┐                       │               │            
│  onlyStakeDeposited   │───────────────────────────────────────┤            
└───────────────────────┘           ┌───────────────────────┐   │            
            │                       │ onlyPaymentDeposited  │───┤            
            │                       └───────────────────────┘   │            
            └─────────────────┐                 │               │            
                              ├─────────────────┘               │            
                              ▼                                 ▼            
                  ┌───────────────────────┐         ┌───────────────────────┐
                  │      isDeposited      │────────▶│      isCancelled      │
                  └───────────────────────┘         └───────────────────────┘
                              │                                              
                              ▼                                              
                  ┌───────────────────────┐                                  
                  │      isFinalized      │                                  
                  └───────────────────────┘                                  

And if you want to restrict to ASCII:

                  +-----------------------+                                  
            +-----|        isOpen         |-----+---------------+            
            v     +-----------------------+     |               |            
+-----------------------+                       |               |            
|  onlyStakeDeposited   |---------------------------------------+            
+-----------------------+           +-----------------------+   |            
            |                       | onlyPaymentDeposited  |---+            
            |                       +-----------------------+   |            
            +-----------------+                 |               |            
                              +-----------------+               |            
                              v                                 v            
                  +-----------------------+         +-----------------------+
                  |      isDeposited      |-------->|      isCancelled      |
                  +-----------------------+         +-----------------------+
                              |                                              
                              v                                              
                  +-----------------------+                                  
                  |      isFinalized      |                                  
                  +-----------------------+                                  

References:

• Su Squares introduction: https://github.com/su-squares/ethereum-contract/blob/master/contracts/ALLINONE.sol

• Function-level documentation is incomplete

  NatSpec documentation is provided for most functions, that’s great!

  The @notice tag for functions is user-visible documentation. So it should be included for every public function. In the future, wallets and Etherscan will show these notes to end-users when transactions are being signed.

  Recommendation: achieve 100% public function-level NatSpec coverage (including public variables which are implicitly functions).

  Additional notes: as developers we often think “this function name will be obvious to all users”. But the function isOpen() refers to only a specific state of the CountdownGriefingEscrow contract. It is very reasonable for someone to look at that and think that all non-terminal states would count as true for isOpen()

  References:

  • One missing documentation:

    erasure-protocol/contracts/escrows/CountdownGriefingEscrow.sol

    Line 446 in 4a3d98c

    function isOpen() public view returns (bool validity) {

[thegostep]

@fulldecent On the topic of state machines, what are your thoughts on including them as a url?

erasure-protocol/contracts/escrows/CountdownGriefingEscrow.sol

Line 18 in 4a3d98c

/// @dev State Machine: https://www.lucidchart.com/publicSegments/view/839ccf53-cdc9-4528-8d5e-8ce53df6f647/image.png

Obviously this relies on lucidchart to host -- perhaps solvable by adding a resource directory to this repo.

[fulldecent]

Could be a problem since smart contracts are forever.

If you will link to this repo make sure you have a specific file branch or commit hash so they see the correct file.

EIP-820 has got some nice ASCII art in it. Maybe I was pushing too hard for super-cute file headers!