Omniscia Avant Protocol Audit

AvUSDMinting Code Style Findings

AvUSDMinting Code Style Findings

AUM-01C: Inefficient Event Emissions

Description:

The referenced event emissions are inefficient as they will temporarily store a value to memory and will additionally read the new value via storage.

Example:

contracts/AvUSDMinting.sol
532/// @notice Sets the max mintPerBlock limit
533function _setMaxMintPerBlock(uint256 _maxMintPerBlock) internal {
534  uint256 oldMaxMintPerBlock = maxMintPerBlock;
535  maxMintPerBlock = _maxMintPerBlock;
536  emit MaxMintPerBlockChanged(oldMaxMintPerBlock, maxMintPerBlock);
537}
538

539/// @notice Sets the max redeemPerBlock limit
540function _setMaxRedeemPerBlock(uint256 _maxRedeemPerBlock) internal {
541  uint256 oldMaxRedeemPerBlock = maxRedeemPerBlock;
542  maxRedeemPerBlock = _maxRedeemPerBlock;
543  emit MaxRedeemPerBlockChanged(oldMaxRedeemPerBlock, maxRedeemPerBlock);
544}

Recommendation:

We advise the events to be emitted prior to each storage assignment, permitting the storage value to be used directly in the event's emission and the input argument to be used as the second value in an optimal way.

Alleviation:

The event emissions have been optimized per our recommendation thus minimizing the number of storage reads they perform.

AUM-02C: Inefficient Remainder Transfer

Description:

The AvUSDMinting::_transferCollateral and AvUSDMinting::_transferEthCollateral functions will inefficiently perform two transfers to allocate any remainder to the last address entry of the addresses list.

Example:

contracts/AvUSDMinting.sol
475/// @notice transfer supported asset to array of custody addresses per defined ratio
476function _transferCollateral(
477  uint256 amount,
478  address asset,
479  address benefactor,
480  address[] calldata addresses,
481  uint256[] calldata ratios
482) internal {
483  // cannot mint using unsupported asset or native ETH even if it is supported for redemptions
484  if (!_supportedAssets.contains(asset) || asset == NATIVE_TOKEN) revert UnsupportedAsset();
485  IERC20 token = IERC20(asset);
486  uint256 totalTransferred = 0;
487  for (uint256 i = 0; i < addresses.length;) {
488    uint256 amountToTransfer = (amount * ratios[i]) / ROUTE_REQUIRED_RATIO;
489    token.safeTransferFrom(benefactor, addresses[i], amountToTransfer);
490    totalTransferred += amountToTransfer;
491    unchecked {
492      ++i;
493    }
494  }
495  uint256 remainingBalance = amount - totalTransferred;
496  if (remainingBalance > 0) {
497    token.safeTransferFrom(benefactor, addresses[addresses.length - 1], remainingBalance);
498  }
499}

Recommendation:

We advise each for loop to iterate until addresses.length - 1, and the remainder to be directly transferred to the last address directly.

Alleviation:

The last member disbursement flow has been optimized per our recommendation ensuring that a single transfer is performed.

AUM-03C: Inefficient Split of Logic

Description:

The AvUSDMinting::_deduplicateOrder function will inefficiently invoke the AvUSDMinting::verifyNonce function and use its return arguments to affect the _orderBitmaps data entry.

Example:

contracts/AvUSDMinting.sol
442/// @notice verify validity of nonce by checking its presence
443function verifyNonce(address sender, uint256 nonce) public view override returns (uint256, uint256, uint256) {
444  if (nonce == 0) revert InvalidNonce();
445  uint256 invalidatorSlot = uint64(nonce) >> 8;
446  uint256 invalidatorBit = 1 << uint8(nonce);
447  uint256 invalidator = _orderBitmaps[sender][invalidatorSlot];
448  if (invalidator & invalidatorBit != 0) revert InvalidNonce();
449

450  return (invalidatorSlot, invalidator, invalidatorBit);
451}
452

453/* --------------- PRIVATE --------------- */
454

455/// @notice deduplication of taker order
456function _deduplicateOrder(address sender, uint256 nonce) private {
457  (uint256 invalidatorSlot, uint256 invalidator, uint256 invalidatorBit) = verifyNonce(sender, nonce);
458  _orderBitmaps[sender][invalidatorSlot] = invalidator | invalidatorBit;
459}

Recommendation:

We advise the AvUSDMinting::_deduplicateOrder function to directly verify and mutate the _orderBitmaps data entry, permitting the _orderBitmaps mapping to be looked up once efficiently.

Alleviation:

The Avant Protocol team evaluated the gas optimization that results from this exhibit and opted to acknowledge it in favour of code readability.

AUM-04C: Inefficient mapping Lookups

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

contracts/AvUSDMinting.sol
290function confirmDelegatedSigner(address _delegatedBy) external {
291  if (delegatedSigner[msg.sender][_delegatedBy] != DelegatedSignerStatus.PENDING) {
292    revert DelegationNotInitiated();
293  }
294  delegatedSigner[msg.sender][_delegatedBy] = DelegatedSignerStatus.ACCEPTED;
295  emit DelegatedSignerAdded(msg.sender, _delegatedBy);
296}

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

As the compiler's optimizations may take care of these caching operations automatically at-times, we advise the optimization to be selectively applied, tested, and then fully adopted to ensure that the proposed caching model indeed leads to a reduction in gas costs.

Alleviation:

All referenced inefficient mapping lookups have been optimized to the greatest extent possible, significantly reducing the gas cost of the functions the statements were located in.

AUM-05C: Non-Standard Modifier Structure

Description:

The AvUSDMinting::belowMaxMintPerBlock and AvUSDMinting::belowMaxRedeemPerBlock modifiers are non-standard in the sense that their control variables are incremented outside their code block.

Example:

contracts/AvUSDMinting.sol
238/**
239 * @notice Redeem stablecoins for assets
240 * @param order struct containing order details and confirmation from server
241 * @param signature signature of the taker
242 */
243function redeem(Order calldata order, Signature calldata signature)
244  external
245  override
246  nonReentrant
247  onlyRole(REDEEMER_ROLE)
248  belowMaxRedeemPerBlock(order.avusd_amount)
249{
250  if (order.order_type != OrderType.REDEEM) revert InvalidOrder();
251  verifyOrder(order, signature);
252  _deduplicateOrder(order.benefactor, order.nonce);
253  // Add to the redeemed amount in this block
254  redeemedPerBlock[block.number] += order.avusd_amount;
255  avusd.burnFrom(order.benefactor, order.avusd_amount);
256  _transferToBeneficiary(order.beneficiary, order.collateral_asset, order.collateral_amount);
257  emit Redeem(
258    msg.sender,
259    order.benefactor,
260    order.beneficiary,
261    order.collateral_asset,
262    order.collateral_amount,
263    order.avusd_amount
264  );
265}

Recommendation:

We advise the mintedPerBlock and redeemedPerBlock increment statements to be relocated to each modifier's body, ensuring that the variables are incremented right after being evaluated.

Alleviation:

The modifier declarations have been adjusted as advised ensuring that they adhere to the CEI pattern and are self-contained.

AUM-06C: Non-Standard Negation of Conditional

Description:

The referenced statement will negate a complex conditional clause involving two equalities and an OR (||) clause.

Example:

contracts/AvUSDMinting.sol
411if (!(signer == order.benefactor || delegatedSigner[signer][order.benefactor] == DelegatedSignerStatus.ACCEPTED)) {

Recommendation:

We advise the negation to be incorporated to each conditional directly, optimizing the statement's gas cost as well as legibility.

Alleviation:

The condition's negation has been properly incorporated within its inner comparators addressing this exhibit.

AUM-07C: Potentially Improper Role Assignment

Description:

The AvUSDMinting::constructor will assign the DEFAULT_ADMIN_ROLE to both the msg.sender and the _admin if the _admin does not match the msg.sender which will result in an inefficient transfer of administrator-ship based on the SingleAdminAccessControl::_grantRole function.

Example:

contracts/AvUSDMinting.sol
135_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
136

137for (uint256 i = 0; i < _assets.length;) {
138  addSupportedAsset(_assets[i]);
139  unchecked {
140    ++i;
141  }
142}
143

144for (uint256 j = 0; j < _custodians.length;) {
145  addCustodianAddress(_custodians[j]);
146  unchecked {
147    ++j;
148  }
149}
150

151// Set the max mint/redeem limits per block
152_setMaxMintPerBlock(_maxMintPerBlock);
153_setMaxRedeemPerBlock(_maxRedeemPerBlock);
154

155if (msg.sender != _admin) {
156  _grantRole(DEFAULT_ADMIN_ROLE, _admin);
157}

Recommendation:

We advise the role assignment to be performed once towards the correct role, optimizing the events emitted by the contract.

Alleviation:

The Avant Protocol team evaluated this exhibit and acknowledged that the initialization process could be made more efficient, however, they wish to retain the current implementation in place as the cost involved is incurred once.

AUM-08C: Redundant Condition

Description:

The AvUSDMinting::_transferEthCollateral function will inefficiently evaluate whether the asset is equal to the NATIVE_TOKEN and not equal to the WAVAX token, the latter implying that the code should fail if the asset is any value other than the WAVAX token.

Example:

contracts/AvUSDMinting.sol
509if (!_supportedAssets.contains(asset) || asset == NATIVE_TOKEN || asset != address(WAVAX)) revert UnsupportedAsset();

Recommendation:

We advise the former conditional to be omitted, optimizing the code's gas cost.

Alleviation:

The redundant part of the conditional has been omitted, optimizing its gas cost.

AUM-09C: Redundant Duplication of Code

Description:

The AvUSDMinting::mint and AvUSDMinting::mintWAVAX functions differentiate between them via a single statement.

Example:

contracts/AvUSDMinting.sol
174/**
175 * @notice Mint stablecoins from assets
176 * @param order struct containing order details and confirmation from server
177 * @param signature signature of the taker
178 */
179function mint(Order calldata order, Route calldata route, Signature calldata signature)
180  external
181  override
182  nonReentrant
183  onlyRole(MINTER_ROLE)
184  belowMaxMintPerBlock(order.avusd_amount)
185{
186  if (order.order_type != OrderType.MINT) revert InvalidOrder();
187  verifyOrder(order, signature);
188  if (!verifyRoute(route)) revert InvalidRoute();
189  _deduplicateOrder(order.benefactor, order.nonce);
190  // Add to the minted amount in this block
191  mintedPerBlock[block.number] += order.avusd_amount;
192  _transferCollateral(
193    order.collateral_amount, order.collateral_asset, order.benefactor, route.addresses, route.ratios
194  );
195  avusd.mint(order.beneficiary, order.avusd_amount);
196  emit Mint(
197    msg.sender,
198    order.benefactor,
199    order.beneficiary,
200    order.collateral_asset,
201    order.collateral_amount,
202    order.avusd_amount
203  );
204}
205

206/**
207 * @notice Mint stablecoins from assets
208 * @param order struct containing order details and confirmation from server
209 * @param signature signature of the taker
210 */
211function mintWAVAX(Order calldata order, Route calldata route, Signature calldata signature)
212  external
213  nonReentrant
214  onlyRole(MINTER_ROLE)
215  belowMaxMintPerBlock(order.avusd_amount)
216{
217  if (order.order_type != OrderType.MINT) revert InvalidOrder();
218  verifyOrder(order, signature);
219  if (!verifyRoute(route)) revert InvalidRoute();
220  _deduplicateOrder(order.benefactor, order.nonce);
221  // Add to the minted amount in this block
222  mintedPerBlock[block.number] += order.avusd_amount;
223  // Checks that the collateral asset is WAVAX also
224  _transferEthCollateral(
225    order.collateral_amount, order.collateral_asset, order.benefactor, route.addresses, route.ratios
226  );
227  avusd.mint(order.beneficiary, order.avusd_amount);
228  emit Mint(
229    msg.sender,
230    order.benefactor,
231    order.beneficiary,
232    order.collateral_asset,
233    order.collateral_amount,
234    order.avusd_amount
235  );
236}

Recommendation:

We advise the function statements to be relocated to a modifier that executes statements before and after the function's body, permitting the same modifier to be used across both functions.

Alternatively, we advise the same approach to be adopted albeit via internal functions that are invoked before and after.

Alleviation:

The functions have been merged into a single AvUSDMinting::mint function that differentiates between the previous implementations by evaluating whether the order.collateral_asset is WAVAX or not.

AUM-10C: Redundant Parenthesis Statement

Description:

The referenced statement is redundantly wrapped in parenthesis (()).

Example:

contracts/AvUSDMinting.sol
439return (totalRatio == ROUTE_REQUIRED_RATIO);

Recommendation:

We advise them to be safely omitted, increasing the legibility of the codebase.

Alleviation:

The redundant parenthesis in the referenced statement have been safely omitted.

Prev

AvUSD.sol (AUS-C)

Next

SingleAdminAccessControl.sol (SAA-C)

On this page