Thales Parlay Market AMM Smart Contract Audit
# 1. Introduction
iosiro was commissioned by Thales to conduct a smart contract audit of their Parlay Market AMM. The audit was performed by 2 auditors between 6 and 17 February, using 10 resource days.
This report is organized into the following sections.
- **[Section 2 - Executive summary:](#section-2)** A high-level description of the findings of the audit.
- **[Section 3 - Audit details:](#section-3)** A description of the scope and methodology of the audit.
- **[Section 4 - Design specification:](#section-4)** An outline of the intended functionality of the smart contracts.
- **[Section 5 - Detailed findings:](#section-5)** Detailed descriptions of the findings of the audit.
The information in this report should be used to understand the smart contracts’ risk exposure better and as a guide to improving the security posture of the smart contracts by remediating the issues identified. The results of this audit reflect the in-scope source code reviewed at the time of the audit.
The purpose of this audit was to achieve the following:
- Identify potential security flaws.
- Ensure that the smart contracts function according to the documentation provided.
Assessing the off-chain functionality associated with the contracts, for example, backend web application code, was outside of the scope of this audit.
Due to the unregulated nature and ease of transfer of cryptocurrencies, operations that store or interact with these assets are considered high risk from cyber attacks. As such, the highest level of security should be observed when interacting with these assets. This requires a forward-thinking approach, which takes into account the new and experimental nature of blockchain technologies. Strategies that should be used to encourage secure code development include:
- Security should be integrated into the development lifecycle, and the level of perceived security should not be limited to a single code audit.
- Defensive programming should be employed to account for unforeseen circumstances.
- Current best practices should be followed where possible.
# 2. Executive summary
This report presents the findings of an audit performed by iosiro on the Thales Parlay Market AMM contracts.
#### Audit findings
During the audit, a single low-risk issue and two informational issues were identified. In addition, iosiro made several suggestions for improving the clarity and gas efficiency of the code.
The low risk issue prevented parlay markets from expiring within an acceptable period of time, and the informational issues pertained to missing validation of a configurable parameter, and the use of `selfdestruct()` which is likely to be deprecated in the near future. Thales resolved these issues prior to the final delivery of this report.
#### Recommendations
The following suggestions are provided as possible improvements to the codebase:
- Remove unused variables and arguments to reduce code size, with some minor benefits to gas consumption.
- Improve readability by adding comments that document function arguments.
- Reduce the number and sizes of events by only emitting the necessary information, reducing code complexity and gas costs.
# 3. Audit details
## 3.1 Scope
The source code considered in-scope for the assessment is described below. Code from all other files was considered to be out-of-scope. Out-of-scope code that interacts with in-scope code was assumed to function as intended and not introduce any functional or security vulnerabilities for the purposes of this audit.
### 3.1.1 Smart contracts
- **Project name: Parlay Markets AMM**
- **Initial commit:** https://github.com/thales-markets/contracts/commit/5096547deee735670e3761052412297e97a62573
- **Final commit:** https://github.com/thales-markets/contracts/commit/a1ead400e8e50cf73419b05f1a10d7acfdaa93b4
- **Files:** `ParlayMarket.sol`, `ParlayMarketData.sol`, `ParlayMarketMasterCopy.sol`, `ParlayMarketsAMM.sol`, `ParlayVerifier.sol`
## 3.2 Methodology
The audit was conducted using a variety of techniques described below.
### 3.2.1 Code review
The source code was manually inspected to identify potential security flaws. Code review is a useful approach for detecting security flaws, discrepancies between the specification and implementation, design improvements, and high-risk areas of the system.
### 3.2.2 Dynamic analysis
The contracts were compiled, deployed, and tested in a test environment, both manually and through the test suite provided. Manual analysis was used to confirm that the code was functional and discover security issues that could be exploited. The coverage report of the provided tests as on the final day of the audit is given below.
| File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Lines |
| --- | --- | --- | --- | --- | --- |
| SportMarkets/Parlay/ | 100 | 69.47 | 96.59 | 93.3 | |
| ParlayMarket.sol | 100 | 78.41 | 95.45 | 97.35 | 117,145,344,359 |
| ParlayMarketData.sol | 100 | 64.71 | 89.47 | 86.24 | 38-55, 171-178, 235-237 |
| ParlayMarketMastercopy.sol | 100 | 100 | 100 | 100 | |
| ParlayMarketsAMM.sol | 100 | 58.82 | 100 | 95.78 | 531, 536, 537, 539 |
| ParlayVerifier.sol | 100 | 72.22 | 100 | 90.91 | 108, 109, 113 114, 123, 124 |
The majority of functionality was covered by tests with only minor coverage gaps.
### 3.2.3 Automated analysis
Tools were used to automatically detect the presence of several types of security vulnerabilities, including reentrancy, timestamp dependency bugs, and transaction-ordering dependency bugs. Static analysis results were reviewed manually and any false positives were removed. Any true positive results are included in this report.
Static analysis tools commonly used include Slither, Securify, and MythX. Tools such as the Remix IDE, compilation output, and linters could also be used to identify potential areas of concern.
## 3.3 Risk ratings
Each issue identified during the audit has been assigned a risk rating. The rating is determined based on the criteria outlined below.
- **High risk**: The issue could result in a loss of funds for the contract owner or system users.
- **Medium risk**: The issue resulted in the code specification being implemented incorrectly.
- **Low risk**: A best practice or design issue that could affect the security of the contract.
- **Informational**: A lapse in best practice or a suboptimal design pattern that has a minimal risk of affecting the security of the contract.
- **Closed**: The issue was identified during the audit and has since been satisfactorily addressed, removing the risk it posed.
# 4. Design specification
The following section outlines the intended functionality of the system at a high level. This specification is based on the implementation in the codebase. Any perceived points of conflict should be highlighted with the auditing team to determine the source of the discrepancy.
## Overview
The Parlay Markets AMM is an automated market maker that utilises Thales’ existing sports markets to create parlays. A parlay combines sports markets a user wishes to participate in, lowering the overall cost of the position while offering significantly higher gains, but with higher risk of failure. If the outcome of a single market within the parlay is against the position, then the entire parlay is considered lost.
In order to limit the risk the AMM is exposed to, the AMM tracks its spending against a given sports market combination and ensures that it does not spend more than a configured amount against that combination. It also features a configurable parameter that limits the odds against which the AMM will take a position.
The AMM also allows users to buy parlays using alternative collateral, such as Dai, USDC and USDT, by exchanging the alternative collateral to sUSD via a curve pool.
## Contracts
A summary of the different contracts are provided below:
- `ParlayMarketsAMM.sol`: The main entry point of the AMM, which interacts with the sports AMM to buy positions, creates parlay markets, and allows users to exercise their parlay positions. The AMM contract is also responsible for limiting the risk exposure of the system, by tracking how much the AMM has already spent on a given combination of sports markets and limiting the odds against which the market will participate.
- `ParlayVerifier.sol`: A utilities contract that is used by `ParlayMarketsAMM.sol` to price parlays and sort sports market combinations to be able to identify unique markets and determine the AMM’s risk exposure.
- `ParlayMarketData.sol`: Aggregates the data from the parlay AMM into various views.
- `ParlayMarket.sol`: For each parlay, an instance of this contract is deployed, and represents a single position. The contract tracks the status of a position, and allows the AMM to determine whether a parlay has already been lost, or if it has potentially won.
# 5. Detailed findings
The following section details the findings of the audit.
## 5.1 High risk
No high-risk issues were identified during this audit.
### 5.2 Medium risk
No medium-risk issues were identified during this audit.
## 5.3 Low risk
All identified low-risk issues were remediated.
## 5.4 Informational
### 5.4.1 Design comments
Actions to improve the functionality and readability of the codebase are outlined below.
#### Unused variables and function parameters
The following variables and function parameters are not used and could be removed to reduce contract size and gas usage:
1. `ParlayMarketsAMM.sol#434`: `initialQuote` is passed into `_buyQuoteFromParlay` but not used.
2. `ParlayVerifier.sol#20-25`: `_positions`, `_totalSUSDToPay` and `_parlayAMM` are passed into this function but not used.
##### Update
Removing these variables will require UI changes, and are thus planned for a future release, [per PR comments](https://github.com/thales-markets/contracts/pull/287).
#### Naming suggestion
`ParlayMarket.sol#103`: The function `isAnySportMarketResolved` could be more accurately named `isAnySportMarketResolvable`. The variable `isResolved` should also be renamed to `isResolvable`.
##### Update
The name change will require UI changes, and is thus planned for a future release, [per PR comments](https://github.com/thales-markets/contracts/pull/287).
#### Spelling errors
The following spelling errors were found in the codebase:
1. `ParlayMarketsAMM.sol#218`: The function parameter `differentRecepient` should be `differentRecipient`.
2. `ParlayMarket.sol#201`: `exerciseWiningSportMarkets` should be `exerciseWinningSportMarkets`
3. `ParlayMarket.sol#250`: `exercizable` should be `exercisable`
4. `ParlayMarketsAMM.sol#655`: `address refferer` should be `address referrer`.
##### Update
The spelling errors will be addressed in a future release, [per PR comments](https://github.com/thales-markets/contracts/pull/287).
## 5.5 Closed
### 5.5.1 Incorrect expiration calculation in `ParlayMarket.sol` (low risk)
[*ParlayMarket.sol#L82*](https://github.com/thales-markets/contracts/blob/5096547deee735670e3761052412297e97a62573/contracts/SportMarkets/Parlay/ParlayMarket.sol#L82)
#### Description
The initializer of the `ParlayMarket` contract calculates the expiration of the market by adding the `_expiryDuration` argument to `block.timestamp` (`expiry = block.timestamp + _expiryDuration;`). However, `block.timestamp` is also added to `_expiryDuration` when it’s passed to the initializer by the `ParlayMarketsAMM` contract (`(block.timestamp + sportManager.expiryDuration())`). As a result, `block.timestamp` is added to the expiry duration twice. Some parlay markets on-chain have been observed to only expire in 2076.
#### Recommendation
The addition of `block.timestamp` to the expiry duration should be removed from the initializer of the `ParlayMarket`.
#### Update
This issue was addressed in [f6321b6](https://github.com/thales-markets/contracts/commit/f6321b6df286fbc39525182bd573ccf6b8775627#diff-05366875a4f987b8ddd1770c7e979582865c6dbeb08f25c2d049aa77963769f5R80) by removing the addition of `block.timestamp` from the expiry duration in the `ParlayMarket` contract initializer.
### 5.5.2 Missing validation of parlay size in `setParameters()` (informational)
[*ParlayMarketsAMM.sol#L548*](https://github.com/thales-markets/contracts/blob/5096547deee735670e3761052412297e97a62573/contracts/SportMarkets/Parlay/ParlayMarketsAMM.sol#L548)
#### Description
The current system design would only ever allow a maximum parlay size of 8 markets. However, the `setParameters()` function in the `ParlayMarketsAMM.sol` contract allows the owner to set the maximum parlay size to any value. This could introduce unwanted behaviour if the maximum was ever set to exceed 8.
#### Recommendation
The `setParameters()` function should include a `require` statement that ensures the new parlay size does not exceed 8.
#### Update
A `require` statement was added to the `setParameters()` function in [f6321b6](https://github.com/thales-markets/contracts/commit/f6321b6df286fbc39525182bd573ccf6b8775627#diff-3098c37a5a056f9e67eda68c25023a3ab7510f642ce001b5064b292a905fa51cR558), ensuring the new maximum parlay size does not exceed 8.
### 5.5.3 Use of `selfdestruct()` (informational)
[*ParlayMarket.sol#L371*](https://github.com/thales-markets/contracts/blob/5096547deee735670e3761052412297e97a62573/contracts/SportMarkets/Parlay/ParlayMarket.sol#L371)
#### Description
`ParlayMarket` includes functionality that will cause the contract to self destruct on expiry. As per [EIP-6049](https://eips.ethereum.org/EIPS/eip-6049), the `SELFDESTRUCT` opcode will be removed from the EVM in the future, and should not be relied upon to maintain its existing functionality.
#### Recommendation
Ideally, any functionality that relies on `selfdestruct()` should be reworked to ensure compatibility with EIP-6049.
#### Update
The call to `selfdestruct()` was commented out in [f6321b6](https://github.com/thales-markets/contracts/commit/f6321b6df286fbc39525182bd573ccf6b8775627#diff-05366875a4f987b8ddd1770c7e979582865c6dbeb08f25c2d049aa77963769f5R369).
### 5.5.4 Design comments
Actions to improve the functionality and readability of the codebase are outlined below.
#### Simplify `riskPerGameCombination` mapping
*ParlayMarketsAMM.sol*
The `riskPerGameCombination` mapping in `ParlayMarketsAMM.sol` could be simplified by changing it to a mapping of `bytes32 => uint`, where the key is the `keccak256` hash of the addresses in the combination. This would improve readability, and also allow parlay market sizes to be dynamically sized in the future, as the current implementation would restrict parlay sizes to a maximum of 8, regardless of the value of `parlaySize`. An example implementation to calculate the combination key is shown below. Additionally, as above, the sorting logic could be moved to this function, helping reduce code size.
***It should be noted that this does change the way `riskPerGameCombination` is represented in storage, and an immediate change would render the existing `riskPerGameCombination` information unusable.***
```solidity
function calculateCombinationKey(address[] memory _sportsMarkets) public view returns (bytes32) {
address[] memory sortedAddresses = new address[](_sportMarkets.length);
sortedAddresses = parlayVerifier.sort(_sportsMarkets);
return keccak256(abi.encodePacked(sortedAddresses));
}
```
##### Update
Implemented in [05c978c](https://github.com/thales-markets/contracts/commit/05c978ca994efb70c45b2a71cc663f1e2cdd0efc#diff-deafcb0e40a52799ed2b1988c800f9e1dcfc6ae7a668a499662d76cc22806eacR227).
#### Simplify `_verifyMarket` function
*ParlayVerifier.sol*
The `_verifyMarket` function, called by `verifyMarkets`, performs the gas-intensive operation of recalculating the `keccack256` hash of team IDs for each sport market. This could be simplified by caching all hashes that have been calculated and performing a lookup against the cached hashes. An example implementation is provided below, which was observed to reduce the gas cost of buying a parlay containing four markets by approximately 343 110.
```solidity
function _getGameIds(
ITherundownConsumer consumer,
address sportMarket
) internal view returns (bytes32 home, bytes32 away) {
ITherundownConsumer.GameCreate memory game = consumer.getGameCreatedById(
consumer.gameIdPerMarket(sportMarket)
);
home = keccak256(abi.encodePacked(game.homeTeam));
away = keccak256(abi.encodePacked(game.awayTeam));
}
function verifyMarkets(
address[] memory _sportMarkets,
uint[] memory _positions,
uint _totalSUSDToPay,
ISportsAMM _sportsAMM,
address _parlayAMM
) external view returns (bool eligible) {
ITherundownConsumer consumer = ITherundownConsumer(_sportsAMM.theRundownConsumer());
eligible = true;
bytes32[] memory cachedTeams = new bytes32[](_sportMarkets.length * 2);
uint lastCachedIdx = 0;
bytes32 gameIdHome;
bytes32 gameIdAway;
uint8 motoCounter = 0;
for (uint i = 0; i < _sportMarkets.length; i++) {
address sportMarket = _sportMarkets[i];
(gameIdHome, gameIdAway) = _getGameIds(consumer, sportMarket);
// check if game IDs already exist
for (uint j = 0; j < lastCachedIdx; j++) {
if (cachedTeams[j] == gameIdHome || cachedTeams[j] == gameIdAway) {
revert("SameTeamOnParlay");
}
}
cachedTeams[lastCachedIdx++] = gameIdHome;
cachedTeams[lastCachedIdx++] = gameIdAway;
uint marketTag = ISportPositionalMarket(sportMarket).tags(0);
if (marketTag == 9100 || marketTag == 9101) {
if (motoCounter > 0) {
eligible = false;
break;
}
motoCounter++;
}
}
}
```
##### Update
Implemented per the recommendation in [f6321b6](https://github.com/thales-markets/contracts/commit/f6321b6df286fbc39525182bd573ccf6b8775627#diff-deafcb0e40a52799ed2b1988c800f9e1dcfc6ae7a668a499662d76cc22806eacR46-R65).
#### Add `onlyKnownMarket` modifier
*ParlayMarketsAMM.sol*
Various instances of the code `_knownMarkets.contains(_parlayMarket)` exist in the `ParlayMarketsAMM.sol` contract. In most cases, the functionality requires that the provided `_parlayMarket` actually exists, and could therefore benefit by adding a modifier that ensures exactly this to improve readability and code reuse. An example implementation would look as follows:
```solidity
modifier onlyKnownMarkets(address _parlayMarket) {
require(_knownMarkets.contains(_parlayMarket), "Unknown parlay market");
_;
}
```
##### Update
Modifier added in [aeb2f44](https://github.com/thales-markets/contracts/commit/aeb2f44721cc1e9853eb940e0e933e63d335622a#diff-3098c37a5a056f9e67eda68c25023a3ab7510f642ce001b5064b292a905fa51cR622).
#### Gas optimization suggestions
The following suggestions are provided as suggestions for improving the contracts’ gas efficiency:
1. `ParlayMarketsAMM.sol`: The function `_buyFromParlay`emits two events with several shared parameters: `NewParlayMarket` and `ParlayMarketCreated`. These could be consolidated into a single event. Additionally, some parameters that are accessible from `ParlayMarket` view functions could be removed from the event.
2. `ParlayVerifier.sol`: In the function `_applySkewImpactBatch`, the variable `totalAmount` is defined and used but not returned. It could be removed from the function.
3. `ParlayMarket.sol`: Instead of fetching a reference to the `sUSD` contract from `ParlayMarketsAMM` every time a transfer is made, `sUSD` could be set in `ParlayMarket.initialize`. While this would increase the gas cost of `ParlayMarketAmm.buyFromParlay`, it would decrease the gas cost of `exerciseParlays`, `exerciseSportMarketsInParlays`, `exerciseParlay` and `exerciseSportMarketInParlay` by greater amounts. However, it would prevent the system from updating the `sUSD` address used by active `ParlayMarket` contracts.
4. `SafeMathUpgradeable` could be removed from `ParlayMarketsAMM` as it uses Solidity 0.8.0.
5. Instances of `type(uint256).max` in `ParlayMarketsAMM.sol` could be replaced by the `MAX_APPROVAL` constant.
6. The `_sportMarkets` array must be sorted before being passed to `ParlayMarketsAMM._storeRisk` or `ParlayVerifier._checkRisk` for these functions to work correctly. This is handled correctly in the current codebase, but could conceivably be omitted in future alterations or extensions. Moving the sorting logic inside these functions would mitigate this risk and improve encapsulation.
7. `ParlayVerifier.sol#198`: The function `obtainSportsAMMPosition` could be written more clearly as below:
```solidity
function obtainSportsAMMPosition(uint _position) public pure returns (ISportsAMM.Position) {
if (_position == 0) {
return ISportsAMM.Position.Home;
} else if (_position == 1) {
return ISportsAMM.Position.Away;
}
return ISportsAMM.Position.Draw;
}
```
##### Update
1. The two events remained in place to preserve the existing parameters without causing a stack too deep compile error.
2. The variable was removed in [f6321b6](https://github.com/thales-markets/contracts/commit/f6321b6df286fbc39525182bd573ccf6b8775627#diff-deafcb0e40a52799ed2b1988c800f9e1dcfc6ae7a668a499662d76cc22806eacL198).
3. This change was not implemented to avoid increasing the gas cost of the `buyFromParlay` function.
4. `SafeMathUpgradeable` was removed in [77a063d](https://github.com/thales-markets/contracts/commit/77a063def32be4994dcfa6ee587aa6d0510ba03f).
5. Implemented in [77a063d](https://github.com/thales-markets/contracts/commit/77a063def32be4994dcfa6ee587aa6d0510ba03f).
6. Implemented in [05c978c](https://github.com/thales-markets/contracts/commit/05c978ca994efb70c45b2a71cc663f1e2cdd0efc).
7. Implemented in [77a063d](https://github.com/thales-markets/contracts/commit/77a063def32be4994dcfa6ee587aa6d0510ba03f#diff-deafcb0e40a52799ed2b1988c800f9e1dcfc6ae7a668a499662d76cc22806eacR218).
#### Missing events
*ParlayMarketsAMM.sol*
Events are not emitted for the setter functions `setParlayMarketMastercopies`, `setParameters` and `setCurveSUSD` .
##### Update
Events added in [aeb2f44](https://github.com/thales-markets/contracts/commit/05c978ca994efb70c45b2a71cc663f1e2cdd0efc).
#### Unused variables and events
The following variables and events are not used and could be removed to reduce contract size and gas usage:
1. `ParlayMarketsAMM.sol#456`: `buyAMMQuote` is declared but not used.
2. `ParlayMarketsAMM.sol#627`: The event `SetSUSD` is unused.
##### Update
1. Removed in [77a063d](https://github.com/thales-markets/contracts/commit/77a063def32be4994dcfa6ee587aa6d0510ba03f#diff-3098c37a5a056f9e67eda68c25023a3ab7510f642ce001b5064b292a905fa51cL465).
2. Removed in [77a063d](https://github.com/thales-markets/contracts/commit/77a063def32be4994dcfa6ee587aa6d0510ba03f#diff-3098c37a5a056f9e67eda68c25023a3ab7510f642ce001b5064b292a905fa51cL637).
