# Informal Review of Portals Multicall and Router
## Scope of Review
### Files Reviewed
- RouterBase.sol
- PortalsRouter.sol
- PortalsMulticall.sol
### Commit hash
`6c94653685f945f2a7e979c3a2eaf15279a98d50`

## PortalsRouter.sol
### Potential Concerns
#### Order fee token and amount are user configurable (Line 124)
##### Considerations
- There are no checks to make sure a fee token is present, or that the fee token amount entered is valid
- It may be worth it to store fee token and amount in a storage variable, updatable by owner

### Permit owner and and signature owner can be different addresses (Line 82)
#### Considerations
- A malicious solver could potentially use the permit message and a signature message from another user
- In practice there is probably no harm here, but, to err on the side of caution it would probably be better to utilize the same sender for each payload

### Gas optimizations
#### Emitting a Portal event may be unnecessary (Line 129)

##### Considerations
- If this event is not necessary it may be worth removing it since it is called on every _execute
- Emitting this event takes up about 6k gas
- The consumer of this event is not clear
- Indexers such as the graph are capable of indexing events based on method selector calls (events are not necessary here)
- If a primary objective is to perform swaps for as cheaply as possible (as the landscape is competitive) it's worth Saving as much gas as possible

##### Fetching collector balance when fee is zero uses extra gas (Line 109)

##### Considerations
- In order to properly determine a collector has accrued the correct fee amount, two external balanceOf calls are made
- If order.fee is expected to be zero somewhat regularly it is better to add conditional logic and skip the two external balanceOf calls when they aren't necessary to save gas

## RouterBase.sol
### Potential Concerns
#### It's possible to brick collector and multicall address (Lines 245 and 256)
#### Considerations
- It may be worth implementing a propose/accept flow for admin functions

#### Gas Optimizations
##### The _permit method could be separated to save gas (Line 190)
##### Considerations
- By splitting up conditional logic into separate functions a small amount of gas can be saved on every permit call
- The amount of gas saved is probably not worth the increase in code complexity, but this is something to consider

## PortalsMulticall.sol
## Potential Concerns
### User has the ability to directly manipulate memory (Line 67)
#### Considerations
- On line 74 the user is able to manipulate memory pointers by specifying different numbers for `amountIndex`
- Since Multicall.sol does not have any allowance for end users (only router does) this is probably ok, but it should be considered that user may be able to edit memory to cause unexpected results

## General Gas Optimizations
### Inconsistent use of internal and private methods
#### Consideration
- Utilizing `private`, `internal`, `public` and `external` methods result in different gas costs
- It's possible to save a small amount of gas by performing tests for to determine which method is most gas efficient for the given version of Solidity
- Gas usage for method types will vary widely based on Solidity versions

## General Considerations
### Additional audits necessary
- Due to usage of low level assembly and complex permit and signature patterns additional audits are recommended
- The biggest risk here is probably someone utilizing permit or signatures to somehow steal from users who have approved the router contract (assuming infinite approval on the router)

## Similar work
[Andre Cronje's Metawallet](https://gist.github.com/andrecronje/6f6098e37ff28c95dfca2b6d1790f088#file-metawallet-sol-L95)