Salty.IO - niroh's results

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/ManagedWallet.sol#L42 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/ManagedWallet.sol#L59

Vulnerability details

Impact

The managedWallet contract implements the following wallet update process:

1. Current main wallet proposes a new pair of wallets (newMainWallet and newConfirmationWallet)
2. current ConfirmationWallet can either accept or reject the proposal by sending an amount >= 0.05 ether (accept) or < 0.05 (reject).
3. If the proposal is accepted by the current ConfirmationWallet, the new proposed mainWallet can apply the change after a 30 days lock period elapses.

The problem is in the scenario where the current ConfirmationWallet rejects the proposal. In this case the only action taken is that activeTimelock is set to type(uint256).max. The MainWallet and ConfirmationWallets are not reset, which means new proposals can not be submitted (because of the following check:
require( proposedMainWallet == address(0), "Cannot overwrite non-zero proposed mainWallet." );)
in proposeWallets.

This leaves the system is a state where the current wallets can not be replaced unless the original proposal is accepted. The only way out of this deadlock is if both the current ConfirmationWallet owner and the new (main and confirmation) wallets owners all agree to facilitate a transition to an alternative wallet pair by having the current ConfirmationWallet accept the proposed pair, and the proposed pair agreeing to propose and accept an alternative pair. This of course entails trusting the proposed pair which is unacceptable (given that the current confirmationWallet rejected them).

managedTeamWallet is an immutable in the ExchangeConfig which means there is no other way to change it. Inability to replace it may put funds at risk in situations such as: There's a risk that private key for the managedTeamWallet were exposed, or the managedTeamWallet address is a multisig wallet whose security may be compromised (for example following an upgrade) etc.

Proof of Concept

The following POC shows how after a proposal is rejected all new proposals revert because of the issue described is this finding.

How to Run:

1. Copy the function below to the src/root_tests/ManagedWallet.t.sol file.
2. run with COVERAGE="yes" NETWORK="sep" forge test -vv --rpc-url "YOUR_RPC_URL" --match-test "testNoNewProposalAfterRejection"

function testNoNewProposalAfterRejection() public {
        ManagedWallet managedWallet = new ManagedWallet(alice, address(this));

        // Propose new wallets
        address newMainWallet = address(0x3333);
        address newConfirmationWallet = address(0x4444);
        vm.prank(alice);
        managedWallet.proposeWallets(newMainWallet, newConfirmationWallet);

        //fast forward a bit for realistic simulation
         vm.warp(block.timestamp + TIMELOCK_DURATION / 30);

        // Confirmation wallet sends less than 0.05 ether to reject
        uint256 amountToSend = 0.000001 ether;
        vm.prank(address(this));
        vm.deal(address(this), amountToSend);
        (bool success,) = address(managedWallet).call{value: amountToSend}("");
        assertTrue(success, "Rejection of proposal failed");

         //fast forward some more (doesn't matter how much)
         vm.warp(block.timestamp + TIMELOCK_DURATION );

        //Try to propose a different pair of main/confirmation wallets.
        newMainWallet = address(0x6666);
        newConfirmationWallet = address(0x7777);
        vm.prank(alice);
        vm.expectRevert("Cannot overwrite non-zero proposed mainWallet.");
        managedWallet.proposeWallets(newMainWallet, newConfirmationWallet);

        //fast forward for a long time
         vm.warp(block.timestamp + TIMELOCK_DURATION*1000 );

        //still cant propose new wallets. We're stuck until current confirmation wallet accepts 
        vm.prank(alice);
        vm.expectRevert("Cannot overwrite non-zero proposed mainWallet.");
        managedWallet.proposeWallets(newMainWallet, newConfirmationWallet);
    }

Tools Used

Foundry

Recommended Mitigation Steps

When a proposal is rejected, reset the proposal mechanism the same way it is done in the changeWallets function:

// Reset
activeTimelock = type(uint256).max;
proposedMainWallet = address(0);
proposedConfirmationWallet = address(0);

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/PriceAggregator.sol#L142 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/PriceAggregator.sol#L183 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/PriceAggregator.sol#L195

Vulnerability details

Impact

1. The price feed aggregator relies on three feeds (Chainlink feeds, Uniswap 30 minute TWAP and Salty spot price) to report pricing. The aggregation logic is to average the closest two feed prices, and if the price difference between them exceeds maximumPriceFeedPercentDifferenceTimes1000 (default value: 3%), the aggregator reverts. Since liquidation and borrowing operations rely on price feed reporting, these will revert as well when the minimum price disparity between the three oracles exceeds maximumPriceFeedPercentDifferenceTimes1000.

2. In times of price volatility for the reported pairs (ETH/USD, BTC/USD) the three oracles used are likely to diverge by more than the 3% limit. To understand why, consider the following traits of each of the three feed sources:

• Uniswap 30 minutes TWAP reports the time weighed average over the last 30 minutes.
• Chainlink uses multiple sources (onchain Dexes, Cexes) and reports immediately on price changes larger than 0.5% (the Chainlink update trigger setting for the two feeds used.)
• Salty reports the immediate price taken from the relevant pools on Salty at the time (block) of price aggragation.
• When volatility is high, the 30 minutes TWAP is likely to diverge from spot oracles such as Chainlink feeds and Salty's pools. If for example the market price shifts drastically within 5 minutes, Uniswap's TWAP will only weigh the change at 20% and the previous price at 80%, while Chainlink and Salty will report the most recent price. In such conditions, a 3% price difference is likely to happen.
• Since Salty's liquidity is likely to be much smaller than the Uniswap pools used (Weth/Wbtc and Weth/Usdc) its reported price is likely to diverge from Uniswap's price, especially when sharp price shifts occur. This is because large pools such as Uniswaps are likely to be arbitraged sooner (to match centralized exchanges where price discovery typically happens) and smaller pools take longer to catch up to the new price.

3. The result is that is times of high volatility, Salty's price feed is likely to enter a state of >3% disparity between the feeds, effetively blocking liquidations at the time they are most crucial.
4. The maximumPriceFeedPercentDifferenceTimes1000 parameter can be extended up to 7% through a DAO vote, but given that every 0.5% increase requires a vote duration of 10 days at least, the DAO is not likely to react it time to adjust for market volatility.

Proof of Concept

Tools Used

Foundry

Recommended Mitigation Steps

The common behavior for DeFi price feed aggregators is to fallback to the most trustworthy oracle rather than abort. Typically, Chainlink is selected as the preferred fallback due to the diversity of its sources and reputation of stability. It is recommended to follow this principle and fallback to Chainlink's price instead of reverting when prices diverge by more than the max.

Assessed type

Other

Low Findings

[L-01] Pre-Approval of collateralAndLiquidity on SALT by the DAO puts the DAOs SALT reserves at risk

Github Links

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L90

Description

1. The DAO contract pre-approves the collateralAndLiquidity contract on SALT/USDS and DAI to the maximal amount to save the gas of individual approve calls whenever formPOL is called during an Upkeep.
2. While USDS and DAI are not kept long-term by the DAO, Salt rewards accumulate in the DAO contract from several sources (such as POL rewards, vesting wallet emissions etc.) and form a substantial asset of the DAO.
3. A call to ERC20::approve takes around 45,000 gas units which is less than 2% of the ~2.3M gas units of an Upkeep call (according to Salty's documentation)
4. The pre-approval exposes all of the DAOs SALT reserves to exfiltration through potential exploits of the collateralAndLiquidity contract.
5. Given that SALT is the main token for value accrual in the DAO, the risk is not justified by the marginal gas savings and specific, per-upkeep approvals should be used.

Recommended Mitigation Steps

Remove the max approvals from the Dao constructor and add an exact amount approval in

[L-02] proposeWebsiteUpdate enables creating multiple (contradicting) proposals simultaneously

Github Links

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L249C11-L249C31 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L102

Description

1. The Proposal contract prevents a proposal from being opened if an open proposal exists with the same name. For example, while a ballot to change priceFeed1 is open, there can be no additional ballot to change priceFeed1 (even if the new ballot offers a different feed address). This is ensured bacause the ballot name for setContract is:

string memory ballotName = string.concat("setContract:", contractName );

where contractName is either priceFeed1,2,3 or accessManager.

2. In proposeWebsiteUpdate however, the ballot name is defined as

string memory ballotName = string.concat("setURL:", newWebsiteURL );

Meaning only WebsiteUpdate ballots who propose the same URL are blocked from being opened in parallel. Additional WebsiteUpdate proposals can be opened with different URLs while previous proposals are still open.

3. This is inconsistent with the behaviour of other proposal types (such as setContract) and results in unexpected behaviour for voters (i.e. the last executed proposal URL is set, regardless of its vote-count compared to other suggested URLs that were open in parallel)

Recommended Mitigation Steps

Change the ballot name of proposeWebsiteUpdate to a fixed string such as "setURL" so that only one updateWebsite proposal can be live simultaneously.

[L-03] the Upkeep function _formPOL trades equal ammounts of weth to SALT and USDS instead of using the ratio of SALT/USDS in the pool

Github Links

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/Upkeep.sol#L129 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/Upkeep.sol#L166

Description

1. The _formPOL function in Upkeep swaps WETH to the two tokens to be added as POL in equal amounts. The function is called once for USDS/DAI and once for SALT/DAI
2. While using equal amounts may make sense for USDS/DAI (likely to be traded at close to 1:1), it does not for SALT/DAI whose trading ratio may be very different.
3. When the POL is formed in DAO.formPOL the depositLiquidityAndIncreaseShare function is called with useZapping set to true. This means that the amounts of SALT/DAI (likely to be extremely imbalanced compared to the pool ratio) will cause a large zap swap to match the pool SALT/DAI ratio.
4. The impact is not only substantial waste of gas, but also a potentially large slippage in the price of SALT/DAI caused by the zap swap, especially if this pool's liquidity is relatively low (Excessive slippage due to zapping swaps is shown with a POC in another finding I submitted).

Recommended Mitigation Steps

instead of trading equal amounts in _formPOL, get the current reserve ratio from the pool of the relevant tokens and trade the pro-rata amount of WETH for each.

[L-04] Emissions mechanism creates an unpredictable emissions rate that depends on Upkeep calls rate

Github Links

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/rewards/Emissions.sol#L55

Description

1. The current Emissions mechanism is activated from the Upkeep function and calculates the current emission as a percentage of the remaining funds to emit. The percentage is calculated as the pro-rata of a constant (default: 0.5% per week) based on the time since last call. When emissions are calculated this way, the more frequent the Upkeep calls, the slower the emission rate. For example: If Upkeep is called once a week over a month, the total emissions in that month are: $\sum_{k=1}^w totalfunds * weeklyrate^k$ $w=4$ If upkeep is called every hour in the same time period, total emissions are: $\sum_{k=1}^h totalfunds * (weeklyrate/h)^k$ $h=4*7*24=672$ The effect is the reverse of compounded interest where the more frequent the emissions the less funds are emitted in a given period. (The effect is accentuated by the double aplication of emissions rate using this calculation method: once in Emissions.sol and once in RewardsEmitter.sol)

2. The rate of Upkeep calls depends on market conditions. For example: in high volatility periods the arbitrage profit rate is likely to be higher than normal. This will create a more frequent incentive to call upkeep (the reward for which is 5% of arbitrage profits since the last call).

3. This creates an undesired uncertainty for both LPs and stakers regarding the rate at which they'll receive emissions, contrary to the common vesting schedules on most DeFi protocols. This in turn creates a disincentive to become a Salty LP/staker.

Recommended Mitigation Steps

A similar emissions pattern can be achieved without dependency on Upkeep call rate, by using a function of the form $A = \frac{-a}{b} \cdot (e^{-bx2} - e^{-bx1})$ $where\;\;a=initialAmount\;\;b=constant\;\;x1=lastUpdateSecondsSinceStart\;\;x2=currentSecondsSinceStart$ (this is the integral between x1,x2 of $f(x) = a \cdot e^{-bx}$ - a function whose form is similar to the one used in the current mechanism)

[L-05] priceFeedModificationCooldown treats all price feeds (1,2,3) as one, and consequently might delay a feed replacement unnessecarily

Github Links

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/PriceAggregator.sol#L50

Description

1. According to the code comments, the priceFeedModificationCooldown (default of 35 days) is meant to "Allows time to evaluate the performance of the recently updated PriceFeed before further updates are made" however there are three price feeds defined on the aggregator, and the cooldown period treats them as one (meaning if priceFeed1 was recently updated but priceFeed2 was not, pricefeed2 would still be blocked from changing for the duration of the cooldown starting at the latest update of priceFeed1).
2. The implication is that if a price feed that has been active for some time (longer than the cooldown period) proves to be unreliable, replacing it to improve price accuracy might be delayed unnecessarily because of a recent change in another price feed. Since price feeds affect the efficiency of liquidations and collateral management, this in turn might negatively effect the system's financial stability.

Recommended Mitigation Steps

Either apply the cooldown separately per feed, or remove the cooldown altogether (keeping in mind that price feed changes might be required with urgency for example if a price feed is permanently down or compromised.)

[L-06] priceFeedModificationCooldown is not applied to the initial feeds that are set in the system by the deployer

Github Links

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/PriceAggregator.sol#L37

Description

1. The cooldown period used to allow for feed evaluation before it is replaced (35 days by default) is only applied when setPriceFeed is called. However, the initial feeds that are set in the system by the deployer and are activated once the boorstrapBallot is finalized, are set through a different function setInitialFeeds and do not activate the cooldown period.
2. Following the same logic that necessitates the cooldown for feeds set by the DAO, initial feeds set by the deployer require the same evaluation time and should have the cooldown period applied to them as well

Recommended Mitigation Steps

Add a bootstrap function to PriceAggregator, callable by BootstrapBallot only, that will initiate the priceFeedModificationCooldownExpiration variable. BootstrapBallot can call this function during finalizeBallot.

QA Findings

[Q-01] - Inaccurate comment in Pools.sol line 292

Github Links

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L292

Description

The comment reads: "Deposit the arbitrage profit for the DAO - later to be divided between the DAO, SALT stakers and liquidity providers in DAO.performUpkeep" however there is no DAO.performUpkeep function and the distribution happens in other contracts

Recommended Mitigation Steps

Change comment to "Deposit the arbitrage profit for the DAO - later to be divided between the DAO, SALT stakers and liquidity providers at part of the Upkeep process"

Gas Optimizations

[G-01] Arbitrage Function can save gas by returning the expected arb profit of each iteration and exiting if the last incremental improvement is below some threshold

Github Links

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/arbitrage/ArbitrageSearch.sol#L63 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/arbitrage/ArbitrageSearch.sol#L120

Description

1. The _bisectionSearch function in ArbitrageSearch searches for the optimal input amount in terms of arbitrage profit. It iterates 8 times, calling _rightMoreProfitable each time to get an indication of weather it should iterate to the left or right half of the current range.
2. _rightMoreProfitable calculates the arb profit at two adgacent points to produce an answer.
3. In many cases, the iteration reaches very small incremental improvements after a small number of iterations (i.e. when trades or profit range are small).
4. Running the full 8 iterations has a significant effect on swap gas costs which overweighs miniscule improvements of arbitrage profits
5. Since _rightMoreProfitable calculates the profit at the current point anyway, the current profit can be returned to _bisectionSearch, and an exit condition added to the loop that exits if the last incremental improvement is smaller than a certain threshold (i.e. 0.000001 WETH or 0.2 cents at current prices)

[G-02] Dao members poolsConfig, stakingConfig, rewardsConfig, stableConfig, daoConfig and priceAggregator should be defined in the Parameters parent contract, saving an unnessecary transfer of all of them with each call to _executeParameterChange.

Github Links

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Parameters.sol#L57

Description

Since the Parameters contract is only used as a parent contract for Dao it would save gas to define the state variables mentioned above in Parameters. This would save the added gas cost of pushing these 6 addresses to the stack (roughly 60x6 = 360 gas units) with each call to _executeParameterChange.

Overview

Salty is a decentralized exchange with a unique value proposition of zero fees trading. Zero fees are made possible by an Automatic Atomic Arbitrage (AAA) mechanism whose generated profits are used to incentivise liquidity providers. In addition to the exchange, Salty also offers a dollar pegged over collateralized stable token (USDS) that uses the liquidity of the Salty WBTC/WETH pool as collateral. Salty also includes an exchange token (Salt) that is the main form of rewards on the platform, and whose staked token (XSalt) grants voting rights on the Salty DAO.

Salty is unique in the DeFi exchange landscape in its no-compromises full decentralization policy that applies from day one of the platforms launch. All settings and parameters underlying the protocol can only be changed through a DAO vote. There are no trusted admins or team safety switches. The only exception is an offchain external verifier in charge of authorizing exchange access based on a geolocation whitelist, however this function is also under the control of the DAO in that the DAO can vote to replace the AccessControl contract and with it the entire ACL mechanism.

Scope

ArbitrageSearch.sol

Implements an internal arbitrage search for Salty. For each pool trade, defines a fixed potential path (always with three legs that start and end with Weth) and runs an 8 step iterative binary search for the optimal input amount (if any) to maximize arbitrage profit on the path.

Dao.sol

Represents the Dao controlling Salty. Holds DAO funds and in charge with finalizing voted actions.

DaoConfig.sol

Holds values of core system parameters such as voting quorum, staking rewards etc and enables the DAO to change these parameters.

Proposals.sol

Manages Dao proposals/Ballots: enables opening ballots, voting on ballots and reporting if a ballot can be finalized or not

Parameters.sol

Defines the different system parameters that can be tweaked by the dao and channels change requests to the right config contracts

Airdrop.sol

Implements the airdrop mechanism activated when the bootstrap ballot is accepted. Allows BootstrapBallot to authorize wallets that voted and are eligible for the airdrop, enables claiming when bootstrap ballot is accepted. Claiming users receive an equal share of XSalt of the airdrop allocation (5M Salt)

BootstrapBallot.sol

Manages the initial vote to launch the system/DAO

InitialDistribution.sol

Defines and performs the initial allocation of Salt then the system is launched.

PoolUtils.sol

Helper functions for the pools contract

PoolsConfig.sol

Parameters (configurable by the DAO) related to Salty pools

PoolStats.sol

Registry of arbitrage profits per pool for arbitrage rewards distribution

Pools.sol

Implements Salty exchange with pools that implement swaps with atomic internal arbitrage

PoolMath.sol

Helper functions for pools related math

CoreChainlinkFeed.sol

Feed implementation encaplusationg Chainlink price feeds (BTC/USD, ETH/USD) used by PriceAggregator

CoreUniswapFeed.sol

Feed implementation encaplusationg UniswapV3 TWAP prices of last 30 minutes (BTC/USD, ETH/USD) used by PriceAggregator

PriceAggregator.sol

PriceFeed implementation aggregating results from three sources (initially UniswapV3 TWAP, Chainlink and Salty but can be replaced by DAO). Used for collateral evaluation in USDS borrowing and liquidations

CoreSaltyFeed.sol

Feed implementation encapsulating Salty spot prices(BTC/USD, ETH/USD) used by PriceAggregator

RewardsConfig.sol

Parameters (configurable by the DAO) related to reward distributions

Emissions.sol

A contract that receives an initial amount of Salt at launch and releases the funds gradually over time (exponential decay pattern) to further emitters and eventually to staking and LP rewards contracts.

RewardsEmitter.sol

Emmits Salt rewards acording to a configurable schedule (exponential decay pattern) to a StakingRewards target (either CollateralAndLiquidity or Staking)

SaltRewards.sol

Receives emissions from Emissions.sol and distributed between the staking RewardsEmitter and the liquidity RewardsEmitter according to a configurable ratio parameter.

USDS.sol

A stable token that can be borrowed against Weth/WBTC pool liquidity. Initial collateralization ratio of 200% and liquidation (min) ratio of 110%

StableConfig.sol

Parameters (configurable by the DAO) related to the stable token USDS

CollateralAndLiquidity.sol

Derives from Liquidity, manages collateral balances for USDS borrowing and liquidity providing balances for Salty pools.

Liquidizer.sol

Manages liquidations of USDS CDPs

StakingConfig.sol

Parameters (configurable by the DAO) related to the Salt staking mechanism in Salty

Liquidity.sol

Derives from StakingRewards (base of CollateralAndLiquidity). Implements the part related to pools liquidity providing.

StakingRewards.sol

Base contract used for both pools LP rewards and staking rewards. Registers user rewards shares and enables rewards claiming.

Staking.sol

Derives from StakingRewards, implements Salt staking and rewards distribution.

ManagedWallet.sol

A contract that represents a wallet that can be changed subject to the approval of a confirmation wallet. The main wallet can propose a change (of both itself and the confirmation wallet). The confirmation wallet can either accept the change (in which case the proposed new main wallet can finalize the change after a wait period of 30 days form acceptance) or reject the change.

AccessManager.sol

Grants wallet access to add liquidity/collateral and borrow USDS. Based on an offchain verifier that checks for whitelisted geo location. Salt.sol 16 @openzeppelin/* SigningTools.sol 20 - Upkeep.sol 169 @openzeppelin/*

ExchangeConfig.sol

Serves as a registry of core protocol addresses such as tokens (salt, dai, wbtc, weth etc.) accessManager, DAO, upkeep, airdrop and teamWallet. DAO contrtolled. Of these, only the accessManager can be updated after the first initialization.

Approach Taken in evaluating the codebase

In analysing the codebase we took the following steps:

1. Architecture Review
Reading the provided documentation to understand the architecture and main functionality of Salty.

2. Compiling code and running provided tests

3. Detailed Code Review
A thorough code review of the contracts in scope.

4. Security Analysis
Defining the main attack surfaces of the codebase i.e. DAO voting manipulation, pool liquidity funds exfiltration, Rewards Distribution Attacks, external manipulation (front/back running trader and LPs), price manipulation of the Salty price feed etc.

5. Additional Testing
Augmenting the existing test scheme with new tests derived from the potential attack surfaces outlined in the security analysis conducted in the previous step.

Mechanism Review

Below is a description of the main mechanisms at the core of Salty's protocol:

Lanuch and Rewards Distribution

Salty's launch is orchestrated using a set of contracts (Airdrop, BootsrapBallot and InitialDistribution) with full decentralization in mind. The process starts with a vote on weather or not to launch the protocol, the duration of which is set by the platform deployer. Voters are authorized by an offchain entity that provides them with a signature to be verified before voting (authorization is based on geolocation and having twitted about the airdrop). An authorized voter gets approved for an airdrop of Salt that takes place at launch. If the vote is successful, the system is kickstarted with an initial distribution of Salt that cascade to different actors through various scheduling and reward distribution mechanisms. The diagram below shows the flow of rewards in Salty at launch and during its ongoing operation.

Automatic Atomic Arbitrage

As mentioned above, Salty is able to offer zero-fee trade by capruting arbitrage opportunities atomically and using the gains to reward liquidity providers. Automatic Atomic Arbitrage (AAA) is implemented based on the following components: A. New tokens on Salty can only be added to two predefined pools: NewToken/Weth and NewToken/WBTC. This, coupled with the core pools that are pre-added to Salty at the time of deployment enables the system to map a single arbitrage path for any trade, that start and ends with Weth. B. As part of each trade transaction, the exchange attempts to find a profitable arbitrage on the mapped path by running an 8 step binary search for an optimal input amount that generates maximum profit. If such a trade exists it is carried out, and the profit is accredited to the pools involved in the path for the purpose of AAA reward distribution.

DAO Voting

As mentioned, Salty imposes strict DAO control on all system configurable components. The DAO voting system is based on XSalt (staked Salt) voting power and enable several predefined types of proposals, namely parameter changes, token whitelisting and approval proposals (such as replacing price feed sources, adding/removing approved countries etc). The voting mechanism includes several mechanisms to prevent hasty/malicious changes:

1. parameters can only be changed incrementally (in fixed steps) and between pre-set min/max values.
2. The default voting period is 10 days (though this can be changed by the DAO down to 3 days)
3. Some sensitive changes (such at contract replacements) require an additional vote (confirmation) once the original proposal is approved. When a proposal quorum is reached and it's minimal duration elapses, anyone can call a finalize function which executes the proposal if the vote is successful, and removes it from the open ballot queue.

Access Control

Access control to add liquidity and to stake Salt is restricted through an external offchain verifier that verifies an address meets the geo restrictions set in the DAO. Once the verification signature is validated the address is marked as whitelisted for future access. If the DAO decides to remove a previously approved country from the list, a new version of verification is formed and users need to re-verify in order to interact with the access-restricted actions.

Stable Token and Liquidations

Salty's stable token (USDS) can be minted based on provided collateral in the form of WBTC/WETH pool liquidity. The collateral value is determined through the system's price feed (see next item) and affect the borrowing limit (requires 200% collateralization) and liquidations (enabled below 110% collateralization). When a user CDP can be liquidated anyone can call a liquidate function and receive a reward of 5% of the liquidated collateral value. The remaining collateral is sent to the Liquidizer contract to be converted to USDS, and the forgone amount of USDS is marked for burning. In each Upkeep cycle the liquidizer tries to burn all USDS marked for burning, and complements missing UDSD (due to undercollateralized liquidations) from the protocol owned liquidity.

Price Feeds

To evaluate collateral for USDS borrowing and liquidations, Salty uses a price aggregator for BTC/USD, ETH/UDS that relies on three external sources (UniswapV3 TWAP, Chainlink and Salty's own pools). The price aggregator averages the two closest prices of the three and reverts if all three reported prices diverge from each other by more than a predefined max difference (3% by default). All three price sources can be replaced by the DAO.

Upkeep

An upkeep transaction that can be called by anyone in the driving engine behind many of Salty's inner mechanics. It is in charge initiating liquidates USDS burns, distributing Weth AAA profits between POL, LPs, stakers and the function caller, distributing POL rewards, releasing scheduled rewards to LPs/stakers and more. Upkeep calls are incentivised by 5% of accumulated AAA Weth profits since the last call.

Systemic Risks

Salty's system architecture facilitates many of Salty's features and unique advantages. However, as with any DeFi protocol, it also entails several risks:

Slow response time to sudden market changes or security threats

As mentioned above, Salty's voting system takes a cautious approach with mechanisms such as long minimal ballot periods, incremental and limited parameter changes, confirmation ballots in some cases etc. This approah however also poses a risk in cases where a more swift response is required. For example:

1. Changing the Upkeep reward percent to match sudden market condition changes where arbitrage profits are unable to cover the upkeep gas cost. As an example, currently increasing the rate to 10% takes 5 voting cycles with a minimal total duration of 50 days. Leaving the upkeep process (which is crucial for Satly's system health) negatively incentivized for that long can be problematic.
2. Changing a disfunctional/compromized price feed that disrupts the correct functioning of USDS borrowing and liquidations. (Which would take atleast 20 days to perform with default params)
3. Unwhitelisting a token that turns out to be rogue and exploitative towards users.

Exposure to rogue tokens

While rogue tokens (fee-on-transfer/reentrancy attacks and other variants) are a problem that affects most dexes, Salty's exposure to this risk is slightly elevated for two reasons:

1. Salty pool collateral is unsegregated opening the door for rogue token exploits that affect pools other than the ones hosting the rogue token.
2. The AAA mechanism potentially couples every trade with an arbitrage trade involving other pools. This too raises the risk for cross-pool exploits.

While tokens need to pass a DAO vote to be whitelisted, some rogue tokens are hard to identify even for experts and the task of filtering them out might prove to be beyond the capabilities of a DAO vote.

Dependancy on SALT token value

Salty's intricate incentive system is at the core of the protocol, especially given that no initial liquidity is provided through investors or founders. In this context it is important to note that all rewards in Salty are awarded in the exchange token SALT (while many dexes incentivize LPs with fees paid in the provided liquidity tokens). This creates a strong dependency on the value of SALT, raising the risk of death-by-lack-of-incentive in the case of an extreme devaluation of Salt.

high levels of inter-dependencies

Many of Salty's inner mechanics are intertwined in a way that can cause a cascading effect from a disruption in one service to the others. Some examples:

1. The WBTC/WETH pool is used both as collateral for the USDS token and as a participant in almost all arbitrage paths. In the event of a "bank run" on USDS causing large liquidity withdrawals from this pool, arbitrage profits will also be affected (since low liquidity reduces the potential arbitrage profit).
2. Salty relies on its on pools for various maintenance tasks. For example: Weth AAA profits are traded for Salt/Usds/Dai before they are distributed or added as POL, POL is liquidated and Weth/WBTC/Dai are traded by the liquidizer for USDS to cover for USDS burning deficiencies. This creates the risk of a problem in one of these pools (such as liquidity shortage or price divergence from market prices) causing disruptions in the above mechanisms and causing a snowball effect in the system.

Lack of contract upgradeability options

In the event that a critical vulnerability is discovered after launch, the protocol currently does not include any mechanism to upgrade contracts (other than the limited option to replace the price-feeds and AccessControl with a DAO vote). This creates the risk of future exploits necessitating a full system relaunch as a remedy.

Centralization Risks

Due to its fully decentralized approach, Salty does not present many centralization risks. The only potential centralization risk is around the external access control authority

Access Control Validator Risk

Salty's system is deployed with a trusted external validator that is in charge of verifying users geolocation and provides signatures that enable users to add liquidity/stake of Salty. While the AccessControl contract can be replaced by the DAO, the process for doing takes a minimum of 20 days which gives ample time for a malicious verifier to perform various exploits such as DOSing the system by blocking everyone.

Recommendations

Below is a list of recommendations, some of which are general architecture/structure recommendations and some are meant to address one or more of the risks mentioned above.

Adding a limited fast-track for changes to address urgent scenarios

Given the relative slow reaction time of the dao to different situations, salty could add a fast-track for changes, limited to specific situations that call for immediate reaction. This can be achieved with a trusted multisig address that is allowed to open fast-response ballots (i.e. shorter duration, smaller quorum) or even perform certain actions directly (such as unwhitelist tokens or change upkeep reward ratio). Alternatively (and keeping in line with Salty's decentralization), certain emergency actions can be defined that can be voted on with a much shorter minimum duration coupled with a larger quorum to enable the DAO to act on emergencies

Adding an upgradability mechanism controlled by the DAO

To address the current lack of upgradability mechanics, it is recommended to include such a mechanism, controlled by a new type of DAO proposal (possibly requiring a very high quorum as a security measure) to enable remediation of potential exploits or bugs that might be discovered in the future.

Adding a standard Access Control mechanism

Currently Salty relies on hard-coded checks to limit access to protected functionality. This might prove to be too limited a mechanism, when in the future access control requirements might become more elaborate (for example if implementing the above recommendations). A standard ACL mechanism such as OpenZeppelin can be more flexible (i.e. enabling the DAO to dynamically grant certain privileges to specific actors) and more robust, and help prevent future ACL vulnerabilities.

Time spent:

50 hours