minor doc updates: include Best Practices for Feature Development. (#58)

docs/advanced/mtx.rst

@@ -100,6 +100,7 @@ This call will revert in the following scenarios:
 - The mtx has expired.
 - The Ethereum transaction's gas price (``tx.gasprice``) is outside of the range ``[mtx.minGasPrice..mtx.maxGasPrice]``
 - The ETH sent with the mtx is less than ``mtx.value``
+- The allowance/balance of ``signer`` is insufficient to pay ``feeAmount`` of ``feeToken`` to the ``sender`` (if specified)
 - The signature is invalid.
 - The mtx was already executed
 - The underlying function is not supported by meta-transactions (see list above).
@@ -122,7 +123,7 @@ batchExecuteMetaTransactions
         payable
         returns (bytes[] memory returnResults);
 
-A `MetaTransactionExecuted <../basics/events.html#metatransactionexecuted>`_ event is emitted for each mtx on succes. The ``returnResult`` contains the raw return data for the executed function This call will revert if the one of the ``mtxs`` reverts.
+A `MetaTransactionExecuted <../basics/events.html#metatransactionexecuted>`_ event is emitted for each mtx on succes. The ``returnResult`` contains the raw return data for the executed function This call will revert if the one of the ``mtxs`` reverts. Any exceess Ether will be refunded to the ``msg.sender``.
 
 
 getMetaTransactionExecutedBlock

docs/advanced/uniswap.rst

@@ -24,6 +24,8 @@ The 0x Protocol is equipped with a highly optimized `UniswapV2 Router <https://u
 
 This function sells ``sellAmount`` of ``tokens[0]`` for at least ``minBuyAmount`` of ``tokens[-1]``. The ``tokens`` array defines how to route the trade between Uniswap pools. This function does not emit any events, although Uniswap pools will emit their own events. This function reverts if amount bought from Uniswap is less than ``minBuyAmount``, or if Uniswap reverts.
 
+See the source code for our router `here <https://github.com/0xProject/protocol/blob/development/contracts/zero-ex/contracts/src/features/UniswapFeature.sol>`_.
+
 See the official `Uniswap V2 Documentation <https://uniswap.org/docs/v2/>`_ for information on events/reverts/allowances.
 
 .. note::

docs/architecture/features.rst

@@ -49,4 +49,29 @@ The only requirement is that the Feature implements the interface in `IFeature <
 
         /// @dev The version of this feature set.
         function FEATURE_VERSION() external view returns (uint256 version);
-    }
+    }
+
+
+Best Practices
+================
+
+We use this checklist to review the safety of new Features.
+
+.. code-block::
+
+    - [ ] Feature has updated version information.
+    - [ ] implements IFeature interface.
+    - [ ] Feature contracts are stateless (including inherited contracts).
+    - [ ] onlySelf feature functions are prefixed with _.
+    - [ ] Feature functions are added to full_migration_tests.
+    - [ ] No delegatecalls from inside a feature. Call other features through the router.
+    - [ ] No self-destruct in features (except BootstrapFeature).
+    - [ ] No intentionally persistent (non-atomic) balances on the Exchange Proxy.
+    - [ ] No direct access to another feature’s storage bucket without strong justification.
+    - [ ] No executing arbitrary calldata from the context of the Exchange Proxy.
+    - [ ] No external calls to arbitrary contracts from within the Exchange Proxy.
+    - [ ] Features use unique StorageIds. 
+    - [ ] Document functions with execution contexts outside of the Exchange Proxy.
+    - [ ] Document feature dependencies in checklist doc.
+    - [ ] Document reentrant functions in checklist doc.
+    - [ ] Document temporary balances.

docs/architecture/proxy.rst

@@ -246,4 +246,12 @@ Although the proxy will not have access to the V3 asset proxies initially, early
 
 **Balances**
 
-Inevitably, there will be features that will cause the Exchange Proxy to hold temporary balances (e.g., payable functions). Thus, it’s a good idea that no feature should cause the Exchange Proxy to hold a permanent balance of tokens or ether, since these balances can easily get mixed up with temporary balances.
+Inevitably, there will be features that will cause the Exchange Proxy to hold temporary balances (e.g., payable functions). Thus, it’s a good idea that no feature should cause the Exchange Proxy to hold a permanent balance of tokens or ether, since these balances can easily get mixed up with temporary balances.
+
+**Reentrancy**
+
+Functions can be re-entered by default; those secured by the ``nonReentrant`` modifier cannot be re-entered.
+
+**Colliding Function Selectors**
+
+We manually ensure that function selectors do not collide during PR's. See the `Feature Checklist <./features.html#best-practices>`_ for a complete list of our best practices on Feature Development.