Description

Several coding inaccuracies were found during the review; they are detailed below.

The start_only parameter could be provided even after AUCTION_STATE is set

The vault_keeper_start_auction puzzle could be used to start the liquidation auction for a collateral vault. A vault keeps track of the state of a liquidation auction in the AUCTION_STATE curried arg. When the initiator starts the auction, they provide some parameters needed to start the auction in the start_only solution parameter. If a time-out occurs without all debt having been recovered but with collateral left in the vault, then a liquidation auction can be restarted by calling the start operation again. The start_only parameter is only needed for starting the auction the first time. But the start_only values could be provided even if the AUCTION_STATE is not nil, that is, even if the auction has already been started.

Redundant timestamp check during oracle update

User-provided timestamps are checked in conditions to confirm their accuracy. During price updates, an ASSERT_SECONDS_ABSOLUTE condition is produced in both the oracle and statues puzzles to check the same timestamp. It is only necessary to check the timestamp in either the oracle or statues puzzle.

Extraneous temp_ban_list logic in oracle mutation

The oracle mutation operation unconditionally assigns nil to temp_ban_list_to_apply and later sets temp_ban_list by checking if temp_ban_list_to_apply is nil or not. As the temp_ban_list_to_apply is already nil, the if condition would always go to the else statement, hence the if condition could be removed.

(price_to_apply temp_ban_list_to_apply outlier_info) (if OUTLIER_INFO
  ; outlier is set, block all price mutations, only price resolver can be used
  (x)
  (if (or (not cut_price_infos) (above-threshold cut_price_infos mean_price auto_approve_price_threshold_bps))
    ; we have a price outlier, set the outlier info
    (list 0 () (c () (c mean_price current_timestamp)))
    ; price is within threshold, no change, no outlier info
    (list (c mean_price current_timestamp) () ())
  )
)

; ...

temp_ban_list (if temp_ban_list_to_apply
      ; outlier is resolved, set temp ban list to what outlier resolution has
      (sha256tree temp_ban_list_to_apply)
      (if TEMP_BAN_LIST
        (if expired_ban_list
          ; ban has expired, reset it
          ()
          ; ban is still active
          TEMP_BAN_LIST
        )
        ()
      )
    )

Unnecessary my_coin_id parameter in savings vault

The savings vault expects a user to provide the my_coin_id solution parameter, but it is not used anywhere in the puzzle and thus could be removed along with the condition that verifies its validity.

Impact

The issues discussed above do not pose any security risks and are solely recommendations for enhancing the code.

Recommendations

We recommend resolving the aforementioned inaccuracies.

Remediation

This issue has been acknowledged by Voltage Technologies Ltd., and fixes were implemented in the following commits:

• 139aa3a6↗
• 9c685d58↗
• b3d095c7↗
• 30165496↗