# Token Subscription (MetaTx) Community Audit

This audit was performed upon the commit [9ff9deb](https://github.com/austintgriffith/tokensubscription.com/tree/9ff9deb10430aacb5bce44a34e16a9c446fb3f4d) by Dean Eigenmann, it was done as part of the [ZK Labs community audit initiative](https://blog.zklabs.io/zk-labs-community-audits-initiative-for-non-ico-projects-ea7830d082f8
).

The code is kept simple & concise, there is no unnecessary complexity found in the smart contract.

**Mentioned issue fixes and suggestions were implemented as of commit [13ced40](https://github.com/austintgriffith/tokensubscription.com/tree/13ced407c2709e99fbe7838a84a9d53f855b40bc).**

## Contract Intent

The Token Subscription (MetaTx) project enables applications to create a trustless subscription model without their users needing to constantly and manually transfer tokens. It consists of one contract, the Subscription contract, which is created with the parameters for a subscription including destination address, token address, token amount, and period of recurrence. The publisher then supplies a link to the subscriber that is presented with the terms of the subscription to sign an off-chain meta transaction that is replayed on the defined period. The subscriber controls the flow of the tokens (starting, stopping, pausing, etc) using the ERC20 standard ```approve()``` function.

More information can be found through the [website](https://tokensubscription.com/).

## Tests

There does not seem to be any tests covering the smart contracts, these types of tests should be created.

## Subscription.sol

- ```ECDSA``` is used for all ```bytes32``` variables.
- ```SafeMath``` is used for all ```uint256``` variables.

### Suggestions

- Move mapping on ```L72``` to there where rest of the variables are defined.
- Move ```constructor``` to the rest of the function definitions.
- Stick to a consistent code style.
- Make all functions that need not be called by the contract itself ```external```.
- Add a ```nonce``` field to subscription signatures. Making it possible to reopen cancelled subscriptions.

### Constructor

This function sets various fields:
- requiredToAddress
- requiredTokenAddress
- requiredTokenAmount
- requiredPeriodSeconds
- requiredGasPrice
- author

### isSubscriptionActive

This function returns whether the ```nextValidTimestamp``` for a given subscription with a ```gracePeriod``` added to it is smaller than or equal to the current ```timestamp```.

### getSubscriptionHash

This function packs the passed variables using the ```abi.encodePacked``` function and then proceeds to hashing those bytes using the ```keccak256``` function.

### getSubscriptionSigner

This function recovers the signer of a given message using the ```recover``` function found in the ```ECDSA``` library.

### isSubscriptionReady

This function checks multiple things, such as:
- That the signer of the message is equal to the ```from``` address.
- That the ```from``` address is not equal to the ```to``` address.
- That the current time is greater than or equal to the ```nextValidTimestamp``` for the given subscription.
- Ensures that the token allowance is greater than the ```tokenAmount``` added with the ```gasPrice```.
- Ensures that the token balance is greater than the ```tokenAmount``` added with the ```gasPrice```.

### cancelSubscription

First the subscription hash is generated by the passed variables using the ```getSubscriptionHash``` function. The ```signer``` is then recovered using the ```recover``` function found in the ```ECDSA``` library. It is then ensured that the ```signer``` is equal to the value of the passed ```from``` variable. Finally the ```nextValidTimestamp``` for the given subscription is set to ```2**256 - 1```.

#### Suggestions
- Instead of doing the whole ```recover``` call, the ```getSubscriptionSigner``` function can be called as it has been in other functions.

### executeSubscription

Initially the subscription hash is generated using the ```getSubscriptionHash``` function. Then the signer is recovered using the ```getSubscriptionSigner``` function. Next it is ensured that the ```signer``` is equal to the passed ```from``` address. It is then ensured that the current timestamp is greater than or equal to the value of the ```nextValidTimestamp``` for the given subscription.

Several checks are then run:
- It is ensured that the ```requiredToAddress``` is either equal to ```0x0``` or the passed ```to``` variable.
- It is ensured that the ```requiredTokenAddress``` is either equal to ```0x0``` or the passed ```tokenAddress``` variable.
- It is ensured that the ```requiredTokenAmount``` is either equal to ```0``` or the passed ```tokenAmount``` variable.
- It is ensured that the ```requiredPeriodSeconds``` is either equal to ```0``` or the passed ```periodSeconds``` variable.
- It is ensured that the ```requiredGasPrice``` is either equal to ```0``` or the passed ```gasPrice``` variable.

The value of the ```nextValidTimestamp``` for the given subscription is then set to the current timestamp added to the ```periodSeconds``` variable.

Tokens are then transfered from the ```from``` address to the passed ```to``` address using the ```transferFrom``` function. It is ensured that tokens were transfered successfully by adding the balance of the ```to``` address before the transfer to the transfered amount and ensuring that said value is equal to the new balance of the ```to``` address. The ```ExecuteSubscription``` event is then emitted.

Finally is the value of ```gasPrice``` is greater than zero, the value is transfered to the function caller from the ```from``` address.

#### Issues

- The ```require``` on ```L233-236``` will not work for every token. See [here](https://medium.com/@chris_77367/explaining-unexpected-reverts-starting-with-solidity-0-4-22-3ada6e82308c).
- Should this function not call ```isSubscriptionReady```?