veRWA - RED-LOTUS-REACH's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/VotingEscrow.sol#L326-L349 https://github.com/code-423n4/2023-08-verwa/blob/main/src/VotingEscrow.sol#L356-L387

Vulnerability details

Impact

• Users can lock their CANTO tokens in the VotingEscrow.sol contract for a duration of 5 years. This lock grants them voting power corresponding to the locked tokens.
• During the token locking process, the user's voting power is automatically assigned. Alternatively, they can opt to delegate their voting power using the VotingEscrow::delegate() function. It's important to note that a user can only delegate their voting power to another user who has a longer lock duration.
• An issue arises when the delegatee's lock period ends, and they withdraw their funds from the contract. In such a scenario, the user who originally delegated their voting power to the delegatee becomes unable to withdraw their tokens. These tokens become permanently locked within the VotingEscrow contract.

Proof of Concept

• Let's illustrate this through a scenario:

  1. User A locks 50 CANTO tokens and gains corresponding voting power.
  2. User Z locks 10 CANTO tokens.
  3. User A delegates their voting power to User Z.
  4. After 5 years, once the lock duration ends, User Z withdraws all initially deposited CANTO tokens.
  5. At this point, if User A attempts to withdraw their CANTO tokens, they encounter a problem. Their voting power is not self-delegated.
  6. Even if User A tries to delegate their voting power back to themselves, it's impossible due to the requirement of a longer lock period for delegation. This creates an effectively repeating delegation cycle.
  7. The only viable option for User A is to create a new lock with a new account, let's say User D, and then delegate their voting power to this new account.
  8. As a result, the funds owned by User A remain perpetually locked within the VotingEscrow contract.

• Copy the following code to 2023-08-verwa/src/test/PoC.t.sol and run forge test --mc "PoC" -vvvv

// SPDX-License-Identifier: AGPL-3.0-or-later
pragma solidity ^0.8.16;
import "forge-std/Test.sol";
import "../VotingEscrow.sol";

contract PoC is Test {
    VotingEscrow public ve;

    address public constant userA = address(10001);
    address public constant userD = address(10002);
    address public constant userZ = address(10004);

    uint256 public constant LOCK_AMT_A = 50 ether;
    uint256 public constant LOCK_AMT_Z = 10 ether;
    uint256 public constant LOCK_AMT_D = 1 ether;

    uint256 public constant LOCK_DUR = 1825 days;

    function setUp() public {
        ve = new VotingEscrow("Voting Escrow", "VE");
        vm.deal(userA, 100 ether);
        vm.deal(userD, 100 ether);
        vm.deal(userZ, 100 ether);
    }

    function test_WithdrawDelegtedFail() public {
        //1. User Z locks 30 for 5 years
        vm.prank(userZ);
        ve.createLock{value: LOCK_AMT_Z}(LOCK_AMT_Z);

        //2. User A locks and delegates 50 to User Z
        vm.prank(userA);
        ve.createLock{value: LOCK_AMT_A}(LOCK_AMT_A);
        vm.prank(userA);
        ve.delegate(userZ);

        //3. After 5 years, User Z withdraws first
        vm.warp(LOCK_DUR + 1);
        vm.prank(userZ);
        ve.withdraw();

        //4. User A tries to withdraw, but fails!
        vm.prank(userA);
        vm.expectRevert();
        ve.withdraw();

        vm.prank(userD);
        ve.createLock{value: LOCK_AMT_D}(LOCK_AMT_D);

        //5. User A tries to delegate to User D
        vm.prank(userA);
        ve.delegate(userD);

        //6. User A then again tries to withdraw
        vm.prank(userA);
        vm.expectRevert("Lock delegated");
        ve.withdraw();

        //⚠️ User A can't withdraw! CANTO is LOCKED ⚠️
    }
}

Tools Used

Manual Review and Foundry

Recommended Mitigation Steps

• Add a condition in delegate() that a if a user's lock time is over then they are allowed to delegate the voting power to themselves so that they can withdraw their funds.

Assessed type

Invalid Validation

Low Risk and Noncritical Issues

• Low Risk and Noncritical Issues
• Casting between types makes values negative for huge input values
• bias and slope can be made negative with huge deposits thus manipulating total voting power
• Inconsistent Lock Durations
• Unnecessary Modifiers Cost Gas
• Many cases of precision loss from multiply-after-divide
• Inconsistent Use of NatSpec
• Unnecessary or Uncomplemented receive function
• Old version of OpenZeppelin Contracts used
• Error Messages don’t match intent
• Function Names don’t match intent
• Dead Code
  • Unused assignment
  • Unused Enums
• TODOs included in code/comments
• Reference to non-existent documentation

Casting between types makes values negative for huge input values

Invariants to the VotingEscrow contract can be broken. While an enormous quantity is necessary, greater than the total supply of $CANTO, it’s worth mentioning due to the fact that invariants in the system can be broken, if only in testing environments.

Invariants affected in VotingEscrow.sol:

• LockedBalance.delegated must always be > 0
  • can be made negative
• LockedBalance.amount must always be > 0
  • can be made negative
• Point.slope must always be ≥ 0
  • can be made negative
• Point.bias must always be ≥ 0
  • can be made negative

Cases in VotingEscrow.sol where casting creates vulnerable attack surfaces:

• From increaseAmount in VotingLock.sol, line 317:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L317

newLocked.delegated += int128(int256(_value));

• From increaseAmount in VotingLock.sol, line 305:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L305

newLocked.delegated += int128(int256(_value));

• From createLock in VotingLock.sol, line 276:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L276

locked_.amount += int128(int256(_value));

• From createLock in VotingLock.sol, line 278:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L278

locked_.delegated += int128(int256(_value));

• From increaseAmount in VotingLock.sol, line 300:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L300

newLocked.amount += int128(int256(_value));

• From increaseAmount in VotingLock.sol, line 317:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L317

newLocked.delegated += int128(int256(_value));

Risk

Debugging and auditing this code is made more complex due to the inclusion of this casting, which puts the protocol more at risk than had they been unused.

It's not fully clear why the types for amount and delegated of the LockedBalance struct are of type int128. This could introduce unforseen edge cases due to the copious casting. All assignments from function arguments are of type uint256 (coming from createLock() and increaseAmount()), making the usage more peculiar and inconsistent. Similarly, int128 is used for bias and slope of the Point struct, and in all places where they are used for calculations, they must be checked for being negative and are subsequently reset to 0 in the case of being negative. The suspicion is that since the origin of the ve system came from the original Curve contracts, the implicit assumptions of that system were kept wholesale.

Mitigation

Use unsigned integers for all financial calculations so that all casting is removed and remove the subsequent unnecessary "resets" to 0. Instead, for places where the "resets" are used, define pre-conditions that ensure negative-valued outcomes are not permissible in calls to createLock() and increaseLock().

bias and slope can be made negative with huge deposits thus manipulating total voting power

Similar to the previous “Casting between types makes values negative for huge input values”, huge inputs can allow for attacks on the protocol, if only in a testing environment.

In VotingEscrow.sol the code in question is from _checkpoint(), lines 129 - 136:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L129-L136

// When a huge deposit is made, the casting of _value in createLock can allow delegated to overflow into negative a value...
if (_oldLocked.end > block.timestamp && _oldLocked.delegated > 0) {
    // ...these can become negative as a result
    userOldPoint.slope = _oldLocked.delegated / int128(int256(LOCKTIME));
    userOldPoint.bias = userOldPoint.slope * int128(int256(_oldLocked.end - block.timestamp));
}
if (_newLocked.end > block.timestamp && _newLocked.delegated > 0) {
    // ...these can become negative as a result
    userNewPoint.slope = _newLocked.delegated / int128(int256(LOCKTIME));
    userNewPoint.bias = userNewPoint.slope * int128(int256(_newLocked.end - block.timestamp));
}

Risk

Control over the sign of bias and slope allows for the control of decay of voting power, because at any point they become < 0, they are reset to 0, giving the theoretical attacker the ability to temporarily stem the decay of their voting power.

Mitigation

Similar to “Casting between types makes values negative for huge input values”, bypass the need to cast (and automatically hard-lock values to 0) by using unsigned types with guards for relevant pre-conditions that ensure negative-valued outcomes are not permissible in calls to createLock() and increaseLock().

Inconsistent Lock Durations

Locks for veRWA are set for 5 years, given no further actions on a lock are performed. This 5 year period is an important duration, because it also determines the calculations for the decay in voting power a lock has. In **VotingEscrow.sol**, the 5 year lock period is measured as 1825 days.

Places where the 1785 day measurement is used:

• in _checkpoint(), line 185

for (uint256 i = 0; i < 255; i++) {
	iterativeTime = iterativeTime + WEEK; 

	...

	if (iterativeTime == block.timestamp) {
	    lastPoint.blk = block.number;
	    break;
	} ...
}

• in _supplyAt(), line 538

function _supplyAt(..., uint256 _t) ... {
	for (uint256 i = 0; i < 255; i++) {
		iterativeTime = iterativeTime + WEEK; 
	
		...
	
		if (iterativeTime == _t) {
		    break;
		}
	}
}

Risk

There is a possibility that the protocol faces a case where locked funds are untouched and no new funds are locked across a 5 year period. In this “last users standing” scenario, the protocol theoretically is still responsible for calculating the final voting weights, allowing investors to maximize the utility of their locked funds regardless of the relevancy of the protocol.

Surprisingly, the maximum span the protocol can actually measure is less than 5 years, it is 255 weeks, which equates to 1785 days. For those final 40 days and afterwards, for example, weights will be miscalculated, and, especially depending on the size of the funds locked, returns on those funds or supply totals will be miscalculated.

Mitigation

Increase the number of iterations to fully calculate weights through the 5 year period.

Unnecessary Modifiers Cost Gas

Reentrancy is a common root-cause for exploits in smart contracts. To avoid the specific case of basic reentrancy, a nonReentrant modifier is often used. In the case of a handful of functions in VotingEscrow.sol, this modifier is unnecessary, because the cause of reentrancy - calling into external contracts - is also missing. While they are not a risk, they do cause extra computation, and thus raise gas costs, which may be of some notice especially during high network congestion.

Locations where they exist:

• Line 268:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L268

function createLock(uint256 _value) external payable nonReentrant {

• Line 288:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L288

function increaseAmount(uint256 _value) external payable nonReentrant {

• Line 326:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L326C5-L326

function withdraw() external nonReentrant {

• Line 356:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L356

function delegate(address _addr) external nonReentrant {

Risk

No identified risk, since no external contract calls are made in these functions. However there is the cost of computation from running the modifiers during each call, thus increasing gas costs.

Mitigation

Remove the modifiers from the function definitions.

Many cases of precision loss from multiply-after-divide

While there wasn’t enough time to explore possible attack vectors for these cases, it’s apparent that there is definite loss in precision while calculating important values in the system.

• From _checkpoint() in VotingEscrow.sol, lines 129 - 132:

https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/VotingEscrow.sol#L129-L132

if (_oldLocked.end > block.timestamp && _oldLocked.delegated > 0) {
    userOldPoint.slope = _oldLocked.delegated / int128(int256(LOCKTIME));
    userOldPoint.bias = userOldPoint.slope * int128(int256(_oldLocked.end - block.timestamp));
}

• From _checkpoint() in VotingEscrow.sol, lines 133 - 136:

https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/VotingEscrow.sol#L133-L136

if (_newLocked.end > block.timestamp && _newLocked.delegated > 0) {
	userNewPoint.slope = _newLocked.delegated / int128(int256(LOCKTIME));
	userNewPoint.bias = userNewPoint.slope * int128(int256(_newLocked.end - block.timestamp));
}

• From _checkpoint() in VotingEscrow.sol, lines 176 - 218

https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/VotingEscrow.sol#L176-L218

uint256 blockSlope = 0; // dblock/dt
if (block.timestamp > lastPoint.ts) {
    blockSlope = (MULTIPLIER * (block.number - lastPoint.blk)) / (block.timestamp - lastPoint.ts);
}

// Go over weeks to fill history and calculate what the current point is
uint256 iterativeTime = _floorToWeek(lastCheckpoint);
for (uint256 i = 0; i < 255; i++) {
     ...

    // PRECISION LOSS HERE: blockslope was originally calculated w/ a division, now it's multiplied further
    lastPoint.blk = initialLastPoint.blk + (blockSlope * (iterativeTime - initialLastPoint.ts)) / MULTIPLIER;

     ...
}

• From _checkpoint_lender() in LendingLedger.sol, line 60:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/LendingLedger.sol#L60

uint256 currEpoch = (block.timestamp / WEEK) * WEEK;

Risk

Precision loss in critical calculations can make the risk/reward for investors less appealing. In critical cases there could be possible losses involved when these discrepancies are exploited.

Mitigation

Reorder operations to ensure division happens before multiplication in the listed instances.

Inconsistent Use of NatSpec

NatSpec is a boon to all Solidity developers. Not only does it provide a structure for developers to document their code within the code itself, it encourages the practice of documenting code. When future developers read code documented with NatSpec, they are able to increase their capacity to understand, upgrade, and fix code. Without code documented with NatSpec, that capacity is hindered.

The veRWA codebase does have a high level of documentation with NatSpec. However there are numerous instances of functions missing NatSpec.

Risks

1. Lack of understanding of code without adequate documentation.
2. Difficulty in understanding, upgrading, and fixing code without documentation.

Recommendation

1. Practice consistent use of NatSpec on all parts of the project codebase.
2. Include the need for NatSpec comments for code reviews to pass.

Unnecessary or Uncomplemented receive function

Most functions that handle accounting for funds in smart contracts typically have a complementary function to provide the opposite operation. In the case of LendingLedger.sol, there is the receive function, however there is no related accounting in that function, or a way to recover funds sent to that contract.

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/LendingLedger.sol#L209

receive() external payable {} // No way to get funds out, no accounting for balance in contract

Risk

Any CANTO deposited in the contract will be locked there because there is no method to transfer it back to the depositor.

Mitigation

Remove the receive function.

Old version of OpenZeppelin Contracts used

Using old versions of 3rd-party libraries in the build process can introduce vulnerabilities to the protocol code.

• Latest is 4.9.3 (as of July 28, 2023), version used in project is 4.9.2

Risks

1. Older versions of OpenZeppelin contracts might not have fixes for known security vulnerabilities.
2. Older versions might contain features or functionality that have been deprecated in recent versions.
3. Newer versions often come with new features, optimizations, and improvements that are not available in older versions.

Recommendation

Review the latest version of the OpenZeppelin contracts and upgrade

Error Messages don’t match intent

Meaningful error messages give better handles to test against. While building a test suite, edge cases can become numerous, and providing descriptive error messages allows not just for the project developers to write better tests, it helps developers of 3rd-party integrations and forks (other protocols, token projects, etc) to understand the intentions and reasoning of the parent project.

In the veRWA codebase, there are places where the error cases are materially different from the guards they are included with.

• From VotingEscrow.sol, line 573:

https://github.com/code-423n4/2023-08-verwa/blob/9a2e7be003bc1a77b3b87db31f3d5a1bcb48ed32/src/VotingEscrow.sol#L573

require(_blockNumber <= block.number, "Only past block number"); //Should be “Only before block number”

Risk

1. Project developers get the wrong understanding of why a case is erroneous.
2. Writing tests for specific error types becomes difficult because of a lack of unique errors provided by the executed code.
3. 3rd-party developers (forks, integrations) can have difficulties understanding developer intentions.

Recommendation

Use more descriptive handler names and error messages and limit reusing error messages for categorically-similar-yet-different errors.

Function Names don’t match intent

totalSupply, totalSupplyAt, supplyAt in VotingEscrow.sol refer to voting power, not locked supply. This causes confusion to developers or auditors onboarding to the codebase.

Dead Code

In many cases, dead code can create attack surfaces for malicious exploits. In the case for VotingEscrow.sol, there are instances of dead and unused code, though the risks are much lower.

Unused assignment

• From _checkpoint() in VotingEscrow.sol, line 206:

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L206

lastCheckpoint = iterativeTime; // <= this is never used

Unused Enums

Ex. VotingEscrow.sol, line 62 and 64

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L62

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L64

enum LockAction {
		...
    INCREASE_TIME, // <= never used
    QUIT, // <= never used
		...
}

Risk

While no attack vectors were identified from these instances of dead code, there is a definite increase in gas usage during execution and deployment.

Mitigation

Remove the instances of dead code from the codebase.

TODOs included in code/comments

Ex. LendingLedger.sol, line 48 and GaugeController.sol line 59

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/LendingLedger.sol#L48

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/GaugeController.sol#L59

governance = _governance; // TODO: Maybe change to Oracle

Reference to non-existent documentation

The references to IVotingEscrow are misleading because there is no IVotingEscrow anywhere in the codebase. This makes it difficult to trust comments in the codebase, let alone find the referenced documentation that may enlighten the developer or auditor.

Examples:

• https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L267
• https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L286
• https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L325
• https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L355
• https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L472https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L486

Table Of Contents

1. Architectural Review & Feedback

Architectural Review & Feedback

GaugeController.sol

As part of the RED-LOTUS team’s audit approach, we identify previous audit findings from original implementation of the protocol code, in the case Curve’s GaugeController.vy. The following High finding within the 2020 Trail of Bits audit of Curve DAO (https://solodit.xyz/issues/several-loops-are-not-executable-due-to-gas-limitation-difficulty-high-trailofbits-curve-dao-pdf) was examined to see if it was applicable to the veRWA codebase.

While GaugeController is based on Curve Finance’s protocol, it can be observed that the main modifications from the Curve implementation are that gauge types have been removed, different whitelisting of gauge addresses is implemented due to the removed types and specific gauges have been removed also. Furthermore, the Governance role / address plays a more crucial and centralized role in administration of the protocol.

Curve concepts for Canto veRWA

To understand Canto’s veRWA implementation of GaugeController, we first need to understand the concepts of gauges, weights, bias, check-ins and slopes pioneered originally within Curve and then forked, modified and implemented in veRWA.

Gauges

Within Canto’s veRWA and as aforementioned, pioneered by Curve, a gauge is a mechanism used to incentivize liquidity providers and govern the distribution of rewards within the protocol. Let's break down the concept of gauges:

1. Liquidity Provision: Canto is an example of a decentralized exchanges that specialize in stablecoin trading. Users can provide liquidity by depositing their stablecoins into liquidity pools. These pools enable efficient trading with low slippage.
2. Gauge Weight: Each liquidity pool in veRWA has an associated gauge weight. The gauge weight determines the amount of rewards that liquidity providers receive for their participation in the pool. Higher gauge weights indicate higher rewards.
3. Voting Power: Holding veCANTO tokens grants voting power within the veRWA governance system.
4. Gauge Voting: Canto allows the community to vote on changes to the gauge weights. Liquidity providers can participate in these votes to influence the allocation of rewards and incentivize specific pools, handled by vote_for_gauge_weights()

function vote_for_gauge_weights(address _gauge_addr, uint256 _user_weight) external 

Overall, gauges in veRWA play a crucial role in incentivizing liquidity provision and governing the distribution of rewards. They allow liquidity providers to earn CANTO tokens based on their participation in the protocol and provide a mechanism for the community to influence the allocation of rewards through voting.

The addition and removal of gauges within the protocol are handled by:

function add_gauge(address _gauge) external onlyGovernance {

function remove_gauge(address _gauge) external onlyGovernance {

Weights

Weights refer to the relative importance or allocation of different assets within a liquidity pool. These weights determine the pricing and trading dynamics of the pool.

In more detail:

1. Asset Weights: Each asset within a veRWA liquidity pool is assigned a weight, which represents its proportionate share or allocation within the pool. The weights are determined by the protocol developers and can vary for different pools.
2. Pricing Formula: veRWA uses a specific pricing formula, known as the "StableSwap" algorithm, to determine the exchange rates between assets in the pool. The formula takes into account the relative weights of the assets and the total value of each asset in the pool.
3. Impact on Pricing: The weights assigned to the assets directly influence the pricing curve of the pool. Assets with higher weights have a greater impact on the pool's pricing, while assets with lower weights have a lesser impact. This ensures that the pool maintains stability and low slippage for trades involving the most significant assets.
4. Trading Dynamics: The weights also affect the trading dynamics within the veRWA pools. They determine the slippage, or price impact, of trades. Assets with higher weights will experience lower slippage, making them more liquid and desirable for trading.
5. Balancing Trade-offs: The choice of weights involves a trade-off between providing liquidity and stability for different assets. The weights are set based on considerations such as market conditions, asset liquidity, and user demand. Adjusting the weights can help optimize the trading experience and ensure efficient swaps within the pool.
6. Governance and Updates: In some cases, the weights assigned to assets within a veRWA pool can be updated through governance processes. This allows the protocol to adapt to changing market conditions or address any imbalances that may arise.

By assigning weights to assets within liquidity pools, veRWA aims to provide efficient and low-slippage trading for stablecoins and similar assets. The specific weight allocations used in different pools are determined by the protocol developers and are subject to ongoing evaluation and adjustment.

The core functions that manage gauge and protocol weights are

function _gauge_relative_weight(address _gauge, uint256 _time) private view returns (uint256) {

function gauge_relative_weight(address _gauge, uint256 _time) external view returns (uint256) {

function gauge_relative_weight_write(address _gauge, uint256 _time) external returns (uint256) {

function _change_gauge_weight(address _gauge, uint256 _weight) internal {

function change_gauge_weight(address _gauge, uint256 _weight) public onlyGovernance {

function get_gauge_weight(address _gauge) external view returns (uint256) {

function get_total_weight() external view returns (uint256) {

Bias

In the context of the veRWA codebase, the concept of "bias" refers to a parameter that influences the pricing and trading dynamics within the veRWA related pools.

In more detail:

1. Bias Parameter: The bias parameter is a configurable value within the Curve protocol that influences the pricing and trading dynamics. It is set by the protocol developers and can vary for different pools.

2. Impact on Pricing: The bias parameter affects the pricing curve of the stablecoin pool. It determines how the prices of stablecoins change as the trading volume and liquidity in the pool fluctuate. A higher bias value results in a more aggressive price curve, while a lower bias value leads to a flatter price curve.

3. Trading Dynamics: The bias parameter also impacts the trading dynamics within the veRWA pools. It affects the slippage, or price impact, of trades. A higher bias value can result in lower slippage for smaller trades but higher slippage for larger trades. Conversely, a lower bias value can lead to higher slippage for smaller trades but lower slippage for larger trades.

4. Balancing Trade-offs: The choice of bias value involves a trade-off between providing low-slippage trades for smaller transactions and maintaining stability and efficiency for larger transactions. The bias parameter is set based on considerations such as market conditions, liquidity requirements, and user experience.

By adjusting the bias parameter, veRWA aims to optimize the trading experience and provide efficient stablecoin swaps within its pools. The specific bias values used in different pools are determined by the protocol developers and are subject to ongoing evaluation and adjustment.

Check-ins and historical data

In the context of veRWA, a "check-in" refers to a specific action or interaction that liquidity providers are required to perform within a designated timeframe. Check-ins are an essential part of the protocol's mechanism to ensure active participation and management of liquidity positions. Let's explore the concept of check-ins in more detail:

1. Check-In Mechanism: veRWA implements a check-in mechanism to incentivize liquidity providers to actively manage their positions. This mechanism encourages regular interaction with the protocol to ensure the stability and efficiency of the liquidity pools.

2. Timeframe: The check-in mechanism operates within a specific timeframe, typically on a weekly basis. Liquidity providers are expected to perform certain actions or updates within this timeframe to maintain their participation in the pool.

3. Actions and Updates: The specific actions or updates required for a successful check-in may vary depending on the protocol and pool. They can include activities such as adding or removing liquidity, adjusting weights, or performing other necessary operations.

4. Importance of Check-Ins: Check-ins serve several purposes within the Curve protocol:

a. Pool Management: Regular check-ins help liquidity providers actively manage their positions and respond to changes in market conditions. This ensures that the liquidity pools remain efficient and responsive.

b. Governance Participation: Check-ins often play a role in governance processes within the protocol. Liquidity providers may be required to check-in and vote on proposals related to parameter updates, such as adjusting the bias parameter or making changes to the protocol's functionality.

c. Reward Distribution: Check-ins may also be necessary to ensure liquidity providers receive their rewards accurately and in a timely manner. By checking in, providers confirm their continued participation and eligibility for rewards.

If a liquidity provider fails to perform the required actions or updates within the designated timeframe, it is considered a "missed check-in." This may result in penalties or a reduction in rewards, depending on the specific rules and mechanisms implemented by the protocol.

The following functions in GaugeController can be observed to fill historic gauge weights week-over-week for missed checkins, return the sum of weights for the future week and return both the sum of weights and gauge weight respectively.

The functions that handle historic weights and sum-of-weights are:

function _get_sum() internal returns (uint256) {

function _get_weight(address _gauge_addr) private returns (uint256) {   

Overall, the check-in mechanism in veRWA encourages liquidity providers to actively manage their positions, participate in governance processes, and contribute to the stability and efficiency of the liquidity pools.

Slopes

Slopers refer to a parameter that influences the rate of change in the pricing curve of a liquidity pool. The slope determines how the prices of assets within the pool adjust in response to changes in supply and demand. It plays a crucial role in determining the trading dynamics and slippage within the veRWA pools.vote_for_gauge_weights() concerns itself with generating slopes for gauge calculations.

Concerns about Governance

As mentioned previously, one of the main changes to the GaugeController was an increased role of Governance in the administration of the protocol. This can be seen in the code snippet of the modifier below.

modifier onlyGovernance() {
    require(msg.sender == governance);
    _;
}

Specifically in the context of the previous High audit finding in the original Curve protocol codebase (https://solodit.xyz/issues/several-loops-are-not-executable-due-to-gas-limitation-difficulty-high-trailofbits-curve-dao-pdf) in which a DoS vector was possible due to the permisonless nature of adding gauges in the original Curve protocol.

Due to the onlyGovernance modifier being added to the add_gauge function, this DoS vector is no longer viable in the veRWA codebase. Nonethless, it does again add a risk of centralization within the protocol and priveleged persons (whale token holders i.e project team and founders) potentially being able to abuse their position of trust.

A note on the recent Curve exploit

GaugeController is a Solidity port of Curve’s Vyper GaugeController. Due to recent events in which Curve pools with contracts utilizing the Vyper programming language were exploited, alarm could be raised over veRWA’s use of this code. However, the exploit was due to a vulnerability within the Vyper compiler itself and not the code. As such, the exploit would not be viable within the ported Solidity code.

Conclusion

This comprehensive breakdown of the GaugeController protocol, explanation of key concepts such as Gauges, Weights, Bias, Slopes and Check-In’s and modifications from the original Curve codebase should give an external observer a robust understanding of the architecture and operation of the contract.

LendingLedger.sol

The Lending Ledger is responsible for allowing users to interact with Canto's veRWA system in order to provide liquidity for different lending markets which are whitelisted by the governance system. In addition to this responsibility, the users who provide liquidity are rewarded in the form of CANTO, respective to their share. As a result, the lending ledger has a duty of ensuring how much liquidity a specific user has supplied for any given market, during any specified timeframe. Therefore, it is quite important to analyse the possibilities of any attack vector, whether this is from the result of a governance proposal or from a malicious exploit.

Core Behaviour

When a user interacts with the LendingLedger, their transaction is ultimately processed via sync_ledger():

function sync_ledger(address _lender, int256 _delta) external {

The sync_ledger checks to ensure that there are no underflows, and that the user is interacting with a valid lending market which has been whitelisted:

  address lendingMarket = msg.sender;
  require(lendingMarketWhitelist[lendingMarket], "Market not whitelisted");

  ...
  uint256 currEpoch = (block.timestamp / WEEK) * WEEK;
  int256 updatedLenderBalance = int256(lendingMarketBalances[lendingMarket][_lender][currEpoch]) + _delta;
  require(updatedLenderBalance >= 0, "Lender balance underflow"); // Sanity check performed here, but the market should ensure that this never happens

  ...
  int256 updatedMarketBalance = int256(lendingMarketTotalBalance[lendingMarket][currEpoch]) + _delta;
  require(updatedMarketBalance >= 0, "Market balance underflow"); // Sanity check performed here, but the market should ensure that this never happens

The use of the _checkpoint_lender function is rather important, because a user’s first deposit is actually stored and updated via this function, and without it being included within sync_ledger, the user would never have the ability to successfully execute the claim function days, weeks or years after providing their liquidity to any given market:

_checkpoint_lender(lendingMarket, _lender, type(uint256).max);

This was an initial problem upon first discovery, because it was not clear whether the user has an updated value for userLastClaimed upon first deposit however they do, purely because sync_ledger calls _checkpoint_lender, and within the checkpoint lender function we have the following lines of code which makes the accounting aspect of this successful:

if (lastUserUpdateEpoch == 0) {
  // Store epoch of first deposit
  userClaimedEpoch[_market][_lender] = currEpoch;
  lendingMarketBalancesEpoch[_market][_lender] = currEpoch;
  ...

Feedback

A robust protocol and comprehensive test suite

With this being said, the tests which exist for epoch vulnerabilities are very well made, and had a well-thought-out process. The Red Lotus team was unable to find any vulnerabilities pertaining to the reusage of the same epoch, or attempting to claim rewards for future epochs which do not exist at that point in time.

Additionally, from an accounting perspective, it is remarkable that Canto implemented a fail-safe methodology to “fill in gaps in the market total balances history (if any exist)” because if any problems were to arise, Canto has the ability to successfully maintain accurate accounting. This is something that not every protocol considers, and therefore we deem it as a good addition to the protocol:

function _checkpoint_market(address _market, uint256 _forwardTimestampLimit) private {
    ...
    if (lastMarketUpdateEpoch == 0) {
        lendingMarketTotalBalanceEpoch[_market] = currEpoch;
    } else if (lastMarketUpdateEpoch < currEpoch) {
        // Fill in potential gaps in the market total balances history
        uint256 lastMarketBalance = lendingMarketTotalBalance[_market][lastMarketUpdateEpoch];
        for (uint256 i = lastMarketUpdateEpoch; i <= updateUntilEpoch; i += WEEK) {
            lendingMarketTotalBalance[_market][i] = lastMarketBalance;
        }
        ...
    }
}

Usability Critiques

Regarding the actual mechanism of the protocol, we would argue that it is important to account for user mistakes such as the ones which relate to “Users can forfeit the rewards for some epochs by skipping them when calling claim”. This problem can be eliminated and as a result, would improve user friendliness for the usage of the protocol overall and would provide a safer environment for the users.

Liquidity and Governance concerns

Something that we would like to bring to light is the actual mechanism of the protocol, where the governance essentially has to assure that LendingLedger at all times contains enough CANTO. It can be argued that it is quite fragile in this sense because if the Governance system were to fail, this would mean that LendingLedger would also fail because the Governance system itself assures that the LendingLedger contains enough CANTO, and this allows for the possibility that users may not have the ability to withdraw their CANTO rewards due to an inaccurate balance on Canto’s side - something which is out of the user’s control and is in the hands of Canto.

A note on confusion and Canto team’s response

It’s important to note that attempts to clarify with the developers which token was actually accepted by LendingLedger revealed the assumption that only cNOTE is accepted as a valid form of currency within the veRWA protocol, it is apparent from the audit that CANTO is the expected token of deposit.

It is not clear that cNOTE is the only accepted form of currency when it comes to providing liquidity for the markets, and without a crystal clear notice to the users, it may cause the users to lose out on their potential liquidity assignment to the specified markets, also allowing them to not retrieve the funds back which is a tragedy. Whilst this may not be a big problem for Canto, it is a big problem for the architectural design and how it links to user-friendliness.

VotingEscrow.sol

Curve Voting Escrow (ve) has become the common way for DeFi projects to get, lose, and use voting power in their smart contract functions. The motivation for ve is to provide the ability to vote for which pools will get the share of limited rewards emitted by the protocol. To combat the gaming of votes by powerful voters, decay in the voting power is included.

Decay in voting power is mediated by three major factors:

1. The amount of tokens locked
2. How long they are locked for
3. How long it’s been since first locking

ve makes later lockers competitive with early lockers. While it introduces the problem of continuous risk/reward judgements by investors due to the game theory involved, it keeps the protocol’s markets available to a larger pool of investors wanting to compete for the fees and rewards by the protocol.

Since its inception, ve has remained a popular DeFi mechanic. Forks and ports have been made and remade. Originally, the first version of the contract was written in Vyper. Projects like Frax, Reflexer, and Workhard Finance later provided a version in Solidity. The version used in veRWA is a fork of the FIAT DAO implementation, which is itself a fork of Curve's original implementation.

Overview

Users can lock up CANTO (for five years) in the VotingEscrow contract to get veCANTO.

When you lock up your CANTO, any further actions such as increasing your locked amount (**increaseAmount**) will reset your lock-time to 5 years. This is primarily a strategic choice in terms of business or design, and its done to prevent mercenary capital from locking a large number of tokens for a brief period to gain significant voting power for a limited number of periods. These short-term investors can provide negative value to the protocol because of the constant flux in locked capital, making it less attractive for long-term investors looking for safe and consistent gains.

For the rest of this analysis, we’ll delve into the mechanics of the smart contract that implements veRWA’s ve functionality.

Core ve Functionality

1. Locking/Depositing
2. Withdrawing
3. Increasing locked deposits
4. Delegating

Caveats and Linear Decay

You can't transfer the veCANTO token. The only method to get veCANTO is by locking up CANTO. As the time left until you unlock your CANTO decreases, your veCANTO balance goes down in a straight line. For instance, if you lock up 4000 CANTO for one year, you'll have the same veCANTO as if you locked up 2000 CANTO for two years, or 1000 CANTO for four years.

Locking/Depositing

Locks can be created with createLock .

createLock updates your locked balance with the chosen amount of CANTO and sets the unlock time. Your delegated tokens also increase, and you become the delegatee.

Withdrawing

The withdraw function allows you to retrieve your locked tokens after your lock-time has passed.

Actions:

• Calculates the amount of tokens to send back based on the locked balance.
• Updates your locked balance to show that you've withdrawn the tokens.
• Clears the lock details, including the amount, end time, delegation, and delegatee.
• Triggers a checkpoint to record the change in your voting power.
• Sends the tokens back to your address.

Increasing Locked Deposits

The increaseAmount function allows you to add more tokens to an existing locked balance, enhancing your voting power.

If you're updating your own locked balance (not delegating), your locked balance is updated:

• The existing amount and end values are copied to a new LockedBalance.
• The new amount is increased by the chosen _value.
• The end time is updated to the rounded down week after the current time.\

If you're updating a delegated lock:

• Your lock is updated first, and your voting power is checkpointed.
• Then, the delegatee's lock is updated:
  • If the delegatee's lock hasn't expired, their delegated amount increases by the chosen _value.
  • The delegatee's lock and voting power are checkpointed.

Delegating

Users can let someone else control when their lock ends and how much they have for voting. Both the person giving control (delegator) and the person receiving it (delegatee) should have active locks when this happens. Also, the delegatee's lock should last longer than the delegator's lock.

Actions:

• If you're delegating to yourself:
  • Your lock becomes the new delegatee's lock.
• If you're undelegating:
  • The old delegatee's lock becomes your new lock.
• If you're re-delegating to someone else:
  • The old delegatee's lock becomes the new delegatee's lock.
  • Your lock remains the same if not involved in delegation.

Time spent:

90 hours