# Contract Review: The LexDAO Registry

## Terminology

### Digital Covenant (DC)

A **Digital Covenant (DC)** is a signed message that conveys a ["pledge, representation, or warranty."](https://github.com/lexdao/tldr#dc) External applications can use DCs to, for example, determine whether or not a user is eligible to access the application.

### Digital Dollar Retainer (DDR)"

A **Digital Dollar Retainer (DDR)** is an [agreement between a *client* and a *provider* about some *deliverable*](https://github.com/lexdao/tldr#ddr). Each DDR contains contextual information about its deliverable and execution or disputes thereof.

## Potential Exploits

### `ScribeRole.renounceScribe` is callable by anyone

[`ScribeRole.renounceScribe`](https://github.com/lexDAO/TLDR/blob/master/lexDAOregistry.sol#L265-L267) can be called by any user, even if the user isn't a scibe. Although this doesn't cause any clear exploits, it would allow any user to trigger a `ScribeRole.ScribeRemoved` event. This could potentially pose a problem for applications that display these events.

### `lexDAOregistry.registerDDR` does not check for signatures from `client` or `provider`

[`lexDAOregistry.registerDDR`](https://github.com/lexDAO/TLDR/blob/master/lexDAOregistry.sol#L819-L855) does not check for signatures from the `client` and `provider` of the `deliverable`. As a result, any user is able to register a DDR on any other user's behalf. Although this would not make the DDR binding, this could be easily misrepresented by user interfaces.

Ideally, DDRs should only be published when all parties to the DDR have signed. Users could create and collect signatures client-side before calling `lexDAOregistry.registerDDR`. Similarly, users could send a transaction containing the *hash* of the DDR, thereby keeping secret the content of the DDR and creating a proxy for a signature on the DDR itself.

### `lexDAOregistry.resolveDDR` and `lexDAOregistry.payDDR` can be blocked by `client` or `provider`

[`lexDAOregistry.resolveDDR`](https://github.com/lexDAO/TLDR/blob/46adb4d2e3174e18a408d25637f6e2dc89167b10/lexDAOregistry.sol#L870-L884) and [`lexDAOregistry.payDDR`](https://github.com/lexDAO/TLDR/blob/46adb4d2e3174e18a408d25637f6e2dc89167b10/lexDAOregistry.sol#L887-L908) can be blocked by a malicious `client` or `provider`. Either party can achieve this by entering into the DDR with a contract address instead of with a standard wallet address. A malicious contract could be configured to automatically reject payments in order to trigger an error when `lexDAOregistry` attempts to send funds via `address.transfer`.

You could fix this issue by making use of a [withdrawal pattern](https://solidity.readthedocs.io/en/v0.4.24/common-patterns.html#withdrawal-from-contracts) for funds held by the contract. Push-payments tend to attract subtle issues, pull-payments are usually simpler and safer.

## Potential Improvements

### Plugin Logic

Core contract logic is currently baked heavily into a single primary contract, `lexDAOregistry`. Improved flexibility could potentially attract a wider audience. Established legal assocations would be unlikely to adopt `TLDR` unless their existing governance processes can be replicated.

To this end, most functions in `lexDAOregistry` could be turned into interfaces extensible by the end-user. Logic presently in `lexDAOregistry` could be pulled out into a default implementation for users looking for a prefab experience.

Some processes that should probably be customizable:
- DDR payment
- DDR dispute creation
- DDR dispute resolution
- DDR dispute resolution fee determination
- DC creation
- DC revokation
- LexScript creation
- LexScript modification

### Governance Behavior

Most governance logic in the contract should be customizable. Although this is generally covered by the `Plugin Logic` section above, it's important that this specific logic be made more flexible. Legal associations will likely want to carry out some resolution process for each dispute. Current governance logic seems too restrictive for the target audience.

### `ERC20` & `ScribeRole` Separation

`lexDAOregistry` both `ERC20` and `ScribeRole`. As a result, `lexDAOregistry` inherits significant complexity and becomes more difficult to upgrade. `lexDAOregistry` could instead reference external deployments of these contracts. This change would minimally impact existing behavior.

### String Storage

`LexScript` objects currently contain `string` fields intended to be populated with relatively large sections of text. Data storage on Ethereum can be exceedingly expensive. It may be useful to hash these strings before storing them on Ethereum. However, this would require users of the platform more carefully store the original content of the `LexScript`.