VaultAccountingFacet Manual Review Findings

VAF-01M: Improper Relay of Message Value

Type	Severity	Location
Language Specific		VaultAccountingFacet.sol:L51

Description:

The VaultAccountingFacet::addCollateral function is meant to support a specialized adapter for the WETH asset via the WETHAdapter implementation, invoking the adapter's WETHAdapter::deposit function via a delegatecall instruction. The current method that relays the msg.value as an argument is non-standard as a delegatecall will also simulate the active msg.value.

Example:

Copy
44if (_s.adapter != address(0)) {
45  CallLib.delegateCallFunc(
46    _s.adapter,
47    abi.encodeWithSelector(
48      IAdapter.deposit.selector,
49      user,
50      amount,
51      msg.value,
52      extraData
53    )
54  );
55} else {

Recommendation:

We advise the msg.value to not be relayed to the WETHAdapter::deposit call, instead updating the WETHAdapter implementation to make use of the context's msg.value properly.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The WETHAdapter implementation was updated as advised, using the same context's msg.value and thus standardizing the code's implementation.

VAF-02M: Non-Standard Application of Fee

Type	Severity	Location
Standard Conformity		VaultAccountingFacet.sol:L131

Description:

The _s.borrowFee applied to VaultAccountingFacet::mintUSD operations is not applied as a percentage decrease of the amount but rather a percentage increase.

Impact:

While the present calculation still behaves correctly, it implements a behaviour potentially undesirable as an _s.borrowFee of 0.01 ether does not necessarily mean a 1% fee.

Example:

Copy
131uint256 fee = amount - ((amount * 1 ether) / (1 ether + _s.borrowFee));

Recommendation:

We advise the code to apply the fee in a standardized format, multiplying amount by _s.borrowFee and dividing by 1 ether.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The fee is properly calculated as a percentage in the updated code, alleviating this exhibit.

VAF-03M: Improper Emergency Mode Checks

Type	Severity	Location
Logical Fault		VaultAccountingFacet.sol:L40, L72

Description:

The VaultAccountingFacet contract will permit deposits to occur when the contract is in emergency mode, however, it will disallow withdrawals.

Impact:

It is possible to deposit funds to the protocol when its emergency mechanisms have kicked in which should be an undesirable trait as such a state would indicate the protocol wishes to halt operations.

Example:

Copy
35/// @inheritdoc IVaultAccounting
36function addCollateral(
37  uint256 user,
38  uint256 amount,
39  bytes memory extraData
40) public payable override updateUser(user) freezeCheck(true) updateDebt {
41  require(user > 0, "VaultAccountingFacet: Non existent user");
42  require(amount > 0, "VaultAccountingFacet: Cannot add 0 collateral");
43
44  if (_s.adapter != address(0)) {
45    CallLib.delegateCallFunc(
46      _s.adapter,
47      abi.encodeWithSelector(
48        IAdapter.deposit.selector,
49        user,
50        amount,
51        msg.value,
52        extraData
53      )
54    );
55  } else {
56    require(msg.value == 0, "VaultAccountingFacet: Message value not 0");
57
58    _s.asset.safeTransferFrom(msg.sender, address(this), amount);
59  }
60
61  _s.userInfo[user].deposit += amount;
62
63  emit CollateralAdded(user, amount);
64
65  _rebalanceYield(user);
66}
67
68/// @inheritdoc IVaultAccounting
69function removeCollateral(
70  uint256 amount,
71  bytes memory extraData
72) public override updateMessageUser freezeCheck(false) updateDebt {

Recommendation:

We advise either both deposits and withdrawals to be prohibited or withdrawals to be permitted as the protocol should not allow deposits to occur when in emergency mode.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

Collateral deposits and withdrawals are simultaneously allowed when the market has been paused in the updated implementation, alleviating this exhibit.