###### tags: `audit`
# Vault 12

## VGT.sol

1. [`442 function claimTokens(address _token, address payable _to) public onlyOwner {`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/VGT.sol#L442)

**Severity**: Minor

**Description**: public -> external

2. [`572 function currentTime() private view returns (uint256) {`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/VGT.sol#L572)

**Severity**: Minor

**Description**: Unused code

## BaseSubscription.sol

1. [`59 function subscribe(`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/BaseSubscription.sol#L59)

**Severity**: Minor

**Description**: public -> external

2. [`92 function _subscribe(uint _price) internal {}`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/BaseSubscription.sol#L92)

**Severity**: Minor

**Description**: Leave this without the implementation to make the class abstract and require implementation in subclasses.

3. [`203 function _upgradePlan(uint _price) internal {}`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/BaseSubscription.sol#L203)

**Severity**: Minor

**Description**: Leave this without the implementation to make the class abstract and require implementation in subclasses.

4. [`301 function _extendSubscription(Subscription storage subscription, uint _price) internal {}`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/BaseSubscription.sol#L301)

**Severity**: Minor

**Description**: Leave this without the implementation to make the class abstract and require implementation in subclasses.

5. [`363 function _payToOwner(uint value) internal nonReentrant {`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/BaseSubscription.sol#L363)

**Severity**: Minor

**Description**: Leave this without the implementation to make the class abstract and require implementation in subclasses.

6. [`366 function _payToUser(uint _balance) internal nonReentrant {`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/BaseSubscription.sol#L366)

**Severity**: Minor

**Description**: Leave this without the implementation to make the class abstract and require implementation in subclasses.

7. [`255 function extensionPayment(address _user, uint _price) public view returns(`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/BaseSubscription.sol#L255)

**Severity**: Critical

**Description**: Extension payment logic seems to be broken. It works only in case if extended price is the same as the old price. `newDepositValue` should be equal to `_price`. `payment` should be equal to `_price - subscription.depositValue + paidEther`. In case of extended price is lower than the old price it might be the case when leftover of user's funds is more than enough to pay for the full extension and in that case the refund of extra funds is required in `extendSubscription`.

## SubscriptionETH.sol

1. [`10 billing.incomeSpeed = billing.incomeSpeed.add(`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/SubscriptionETH.sol#L10)

**Severity**: Minor

**Description**: Updates to `billing.incomeSpeed` is always increased by `_price.mul(MULTI).div(duration)` so it can be moved to `BaseSubscription::subscribe`.

2. [Lines 40 -- 53](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/SubscriptionETH.sol#L40-L53)

**Severety**: Minor

**Description**: `billing.incomeSpeed` updating logic, `subscription` updating logic and `sortedSubscriptions` updating logic is the same for all subscriptions so it can be moved to `BaseSubscription::extendSubscription`

3. [`55 uint change = payment.sub(msg.value);`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/SubscriptionETH.sol#L55)

**Severity**: Critical

**Description**: `payment` is almost always less than `msg.value` so this statement will almost always revert txn. Should be `uint change = msg.value.sub(payment);`.

## SubscriptionVGT.sol

1. [`24 billing.incomeSpeed = billing.incomeSpeed.add(`](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/SubscriptionVGT.sol#L24)

**Severity**: Minor

**Description**: Updates to `billing.incomeSpeed` is always increased by `_price.mul(MULTI).div(duration)` so it can be moved to `BaseSupscription::subscribe`.

2. [Lines 50 -- 63](https://github.com/vault12/vault_manager/blob/d4491b0af204554a064cabeed1a853f15b0e307c/contracts/SubscriptionVGT.sol#L50-L63)

**Severety**: Minor

**Description**: `billing.incomeSpeed` updating logic, `subscription` updating logic and `sortedSubscriptions` updating logic is the same for all subscriptions so it can be moved to `BaseSubscription::extendSubscription`

## Summary

`BaseSubscription::extensionPayment` logic and therefore `BaseSubscription::extendSubscription` logic are broken and need to be rewritten. I recommend to implement `extendSubscription` logic like `upgradePlan` logic was implemented as the cases are similar.

For the rest except for some refactoring issues the contract is well written.