Buttonwood Cash

When a user requests a mortgage, either the collateral token or USDX is transferred from the user to the order pool, depending on if the request is compounding or not.

If the order expires before it is processed, the order is simply cancelled and the user's tokens are not returned. The order is deleted.

  function _processOrder(uint256 index, uint256 hintPrevId) internal returns (uint256 collectedGasFee) {
    // Fetch the order from the orders mapping
    PurchaseOrder memory order = _orders[index];

    // If the order has expired, cancel it. Otherwise, process it.
    if (order.expiration < block.timestamp) {
      // Cancel the mortgage request
      IGeneralManager(generalManager).burnMortgageNFT(order.mortgageParams.tokenId);

      // Delete the order
      delete _orders[index];

      // Emit the PurchaseOrderExpired event
      emit PurchaseOrderExpired(index);

Return the user's collateral or USDX depending on the type of order.

The originationPoolDeployCallback function lacks access control. A malicious actor can invoke it directly with a collateralAmount of 0 and an excessively large amountBorrowed. This results in the minting of amountBorrowed Consol tokens to the malicious user.

Implement access control for the originationPoolDeployCallback function to prevent unauthorized usage.

Cancelled withdrawal requests are not removed from the queue; instead, the request.amount and request.share values are set to 0 to allow these requests to be skipped.

However, the UsdxQueue.processWithdrawalRequests function does not skip these cancelled requests and attempts to withdraw zero amounts from the Consol, which reverts with an AmountTooSmall error.

Since withdrawal processing is linear, subsequent requests cannot be processed until the previous ones are completed. As a result, a single cancellation causes all subsequent withdrawal requests to be effectively DoSed.

Note that the exact same issue occurs in ForfeitedAssetsQueue.processWithdrawalRequests function as well.

Skip cancelled requests. Do not withdraw from Consol if request.amount is zero.

Users create mortgage requests in the GeneralManager contract, and these requests are processed by the fulfiller using the processOrders function in the OrderPool.

During this process, the GeneralManager.originate function is called, which subsequently calls OriginationPool.deploy, and finally invokes the originationPoolDeployCallback function in GeneralManager. The _enqueueMortgage function is called within this callback when originationParameters.conversionQueue is non-zero.

However, the _enqueueMortgage function requires the mortgageGasFee to be sent as msg.value, but neither the originate nor the originationPoolDeployCallback functions are payable, making it impossible to send this fee as msg.value. As a result, processOrders will always fail when a mortgage request has a conversionQueue with a non-zero mortgageGasFee.

Since compounding orders must include a conversionQueue, this issue will have the greatest impact on them. However, it is not limited to compounding orders, as non-compounding orders can also have a conversionQueue set.

Ensure that the required mortgageGasFee is passed as msg.value during the processOrders flow. However, since the fulfiller will invoke this function, the required amount should be transferred from the user to the fulfiller at the time of order creation.

Another important consideration is that the fulfiller can process multiple orders at once and directly using msg.value within the processOrders loop may introduce new issues. Care should be taken when sending value, and only the required amount for each order should be sent.

In the _insertMortgage function of the MortgageQueue contract the hintPrevId value is only validated and does not functionally serve to reduce the iterations necessary to lodge a mortgage in the right spot in the queue.

When the triggerPrice of the head is less than the triggerPrice of the newly inserted mortgage the hintPrevId always begins from the head. Therefore the insertion must always traverse the full set of nodes to insert a new mortgage.

In many cases the queue will either see an amount of natural usage or an intentional malicious amount of usage such that subsequent mortgages require more iterations than the block gas limit to insert. This would prevent the origination process for compounding mortgages from being fulfilled as well as other processes in the Buttonwood protocol that rely on this underlying queue logic.

Do not reset the hintPrevId to the head node if the hintPrevId has already proven to have a trigger price less than or equal to the triggerPrice of the new mortgage.

In the MultiTokenVault the inflation attack is defended against by using a decimalsOffset to preserve a larger amount of shares relative to the number of assets deposited.

However this mechanism can be circumvented because there is a public forfeit function which can be used to burn shares of the vault and counteract the decimalsOffset, allowing the total initial shares to be manipulated back down to 1 wei by an attacker.

As a result the typical inflation attack can be carried out, resulting in the victim receiving 0 or significantly reduced shares of the vault.

Firstly, make the forfeit function only callable by the LoanManager contract.

Even then the inflation attack may still be possible by transferring Consol tokens directly to the LoanManager before it invokes forfeit with it’s total balance. Consider either requiring that some initial shares are minted to the dead address, or that forfeit cannot be used to forfeit shares below a certain reasonable threshold of totalShares.

In the GeneralManager contract the enqueueMortgage function is not marked as payable and therefore cannot accept the necessary Ether to call the conversionQueue with the expected value for gas fees.

As a result the enqueueMortgage function cannot be used to enqueue a mortgage as expected.

Make the external enqueueMortgage function on the GeneralManager payable so that it can accept the necessary gas fee ether to send to the conversionQueue.

In the LoanManager redeemMortgage function if the async value is provided as true the non-async path will be taken and if the async value is false the asynchronous path will be taken.

Flip the case such that the async path is taken if the async value is true.

The expandBalanceSheet action does not perform any validation to ensure that the expansion totalPeriods matches the totalPeriods of the existing mortgage.

As a result, users may create a minimum size expansion directly after initiating a 3-year mortgage at a lower rate, as the three-year treasury rate will typically be lower than the fiver year treasury rate, and expand their existing mortgage to be extended to 5 years while maintaining this lower rate.

This way those who are given the expansion role are able to game the protocol and obtain a 5-year length mortgage at the rate of a 3-year length mortgage. Even without paying any kind of refinance fee.

Validate that an expansion can only be made with a totalPeriods value that matches the existing mortgage.

The expandBalanceSheet function in the GeneralManager performs no validations that the caller is the owner of the balance sheet being expanded, this allows approved callers of the expandBalanceSheet function to extend the length of any user’s mortgage at will while expanding their mortgage balance by only the minimum amount.

This results in users holding a mortgage for longer than they expect and requires them to pay more interest than initially promised.

Perform validation that the caller of the expandBalanceSheet function is the owner of the mortgage being expanded

The penalty payments made through the loanManager penaltyPay function and the interest paid through the periodPay function both cause a stepwise increase in the value of consol tokens which can be taken advantage of, especially for large payments.

An opportunistic actor can observe that a large penalty/interest is about to be paid, or have a large outstanding penalty/interest payment they themselves have to make, and frontrun this action to deposit a large amount of USDX into the consol. With a non-trivial share of the total supply of consol shares, the opportunistic actor can realize a significant portion of the penalty/interest payment made and effectively vampire attack this from other Consol depositors who held their shares while the penalty/interest was actually accrued over time.

The opportunistic actor may not be able to instantly withdraw their Consol to realize the immediate stepwise gain, however they profit this amount on paper immediately compared to the other Consol share holders who held for the entire time previous and now had their payment diluted. Furthermore, there may be cases where a large conversion is available through the conversion or forfeited assets queue which does allow the opportunistic actor to enter and exit consol in a very short period while gaining yield which should have accrued to other Consol holders.

There are many potential solutions to resolve this stepwise increase in share value issue. A fee for outside deposits into Consol could be added to disincentivize this behavior. The yield paid to the Consol contract could be spread out over time to avoid single stepwise jumps in share value which create such opportunities. Or a deposit delay could be implemented such that there is no guarantee that the actor is able to frontrun the interest/penalty payment value increase.

In the expandBalanceSheet function there is no validation that requires the caller to provide a nonzero conversionQueue address if that mortgage is currently enqueued, even though the mortgage must be re-enqueued into the ConversionQueue at the new trigger price.

If this mortgage was previously enqueued in the ConversionQueue and is not re-enqueued in the ConversionQueue then within the processWithdrawalRequests function the mortgage will be executed at the original trigger price when the mortgage was originally enqueued.

This trigger price is not accurate to the trigger price of the new mortgage and in some cases may even reflect a loss for the mortgage’s average buy price, if there have not been frequent withdrawals processed, which can result in reverts which block the queue from progressing for an extended period of time.

If the mortgage being expanded in the expandBalanceSheet function is already enqueued in the ConversionQueue, require that the conversion queue address provided is nonzero and a valid conversion queue.

When a large percentage of Consol holders have pending withdrawal requests in the various queues a large mortgage holder can recoup a large portion of their interest payments by simply depositing USDX into Consol and holding consol while they make their payments. The borrower can then process the queue entries as they are available so that each entry returns their share of the yield to the borrower’s Consol holdings.

In the case where a large mortgage exists and a significant portion of Consol holders are deposited in a queue with entries that can be processed or can be processed soon the mortgage owner can simply pay off their entire mortgage and then recoup the interest in a short period.

To disincentivize this behavior a fee could be levied directly to deposits made to Consol through USDX and outside of the Buttonwood system. Otherwise, there can be caps on the percentage of Consol that is allowed to be deposited into queues at any given time to limit the feasibility of this scenario.

Throughout the codebase there are several invocations to burnExcessShares throughout the various withdrawal queues. This function call creates a stepwise jump that re-distributes the yield from the holder who is withdrawing their tokens from the queue to the rest of the Consol holders.

This can cause a significant stepwise jump in the Consol token price, especially for large Consol withdrawals which have spent a non-trivial amount of time in the queue. This is likely given the rate of interest payments made and length of mortgages in the Buttonwood cash protocol.

This stepwise jump can be arbitraged by other users who obtain consol right before this stepwise jump and lock in a guaranteed profit that otherwise they would have had to wait to accumulate honestly.

Instead of retro-actively burning the excess shares of the users who deposit into the withdrawal queue, consider removing them from the Consol token pool by withdrawing their assets from the Consol vault immediately upon their request to withdraw in the queue.

If the request is cancelled, then simply deposit these assets back into the Consol token at the updated share price.

When dequeuing and re-enqueuing a mortgage a new gas fee is required to be covered by the user, however the gas fee provided for the original queue position is not recovered. As a result, this gas fee amount is left trapped in the ConversionQueue contract.

Refund the gas fee collected by the old, queued mortgage and validate that the current gas fee has been provided.

The updateMortgageId function is flawed because it does not remove the association of the previous mortgageId with the tokenId. Therefore, the tokenId is still associated with the old mortgageId in the getTokenId mapping.

As a result, one tokenId could be used to occupy many common mortgageId’s, or these leftover getTokenId mapping values can cause confusion and issues for consumers of this mapping in normal use.

Be sure to remove the getTokenId entry for the previous mortgageId in the updateMortgageId function.

When forclosing a position into the forfeited assets queue the amount of forfeited asset tokens minted corresponds to the outstanding debt of the position without interest. It is one issue that this amount does not include interest.

However, this amount also does not include any penalty payments that the mortgage holder had outstanding. This makes sense from the third party Consol holder’s perspective. However, from the perspective of the mortgage holder, if they have accrued unpaid penalty payments up to this point then it makes sense for them to instead obtain Consol, enter the forfeited assets queue and allow their mortgage to forclose to redeem their underlying collateral without the penalty payments.

To protect against this potential gaming, consider setting aside the unpaid penalties from the collateral seized on forclosure and distributing it to all Consol holders similar to how would be done in the penaltyPay function.

The deployOriginationPool function does not have access control, and pools are deployed based on predetermined configs added by the admin.

However, the function does not check if the config for the user-provided oPoolConfigId exists, allowing pools to be deployed with an empty config.

A malicious actor can front-run the admin just before a new config is added, fetch the oPoolConfigId, and deploy a pool before the config is stored. The resulting pool is unusable, as the Consol and USDX addresses are set to 0. Since OriginationPool deployments can only occur once per epoch, the initial epoch is effectively DoS'ed by this action, and the correct pool with a legitimate config can only be deployed in the next epoch.

If the function is intended to be external, add a check to ensure that config.consol != address(0) and verify that a valid config exists for the provided oPoolConfigId.

Additionally, consider restricting the deployOriginationPool function to admin access only.

Forclosures only occur if the mortgage owner misses more than MAXIMUM_MISSED_PAYMENTS payments. There is no check on the backing collateral value of the mortgage. When the market declines and the collateral value drop significantly, there is no incentive to continue making monthly payments to keep the collateral and redeem it later.

Consider defining a safe ratio between the backing collateral value and the debt and allow mortgages to be foreclosed if the collateral value falls below that ratio, even if there are no missed payments. Additionally, allow borrowers to add more collateral to secure their loans.

The USDX contract is a MultiTokenVault with no overrides on the deposit or withdrawal functions, and the conversion functions simply adjust the token amount by the relevant decimals. Therefore, asset A can be deposited and asset B can be withdrawn at exactly the same exchangeRate.

This allows for a slippage free swap on the supported tokens, e.g. USDC and USDT. If one token depegs by even a small amount USDX would be immediately arbitraged to hold the depegged token at the cost of all USDX holders.

This could be addressed in many ways, including but not limited to adding an exchange rate mechanism for the tokens being deposited and withdrawn from USDX, adding a withdrawal fee that applies when withdrawals are made shortly after deposits, or requiring a cooldown period for withdrawals.

Otherwise acknowledge and be aware of the risk of arbitrage in the case where one token backing USDX depegs, or a user simply wishes to abuse a slippage free swap.

When a mortgage is foreclosed, the entire remaining collateral is deposited into the forfeited assets pool, and an amountOutstanding amount of ForfeitedAssetPool tokens is minted. Neither the mint amount nor the added collateral value is relevant to the current state of the forfeited assets pool. As a result, each foreclosure can significantly change the value of a single ForfeitedAssetsPool token.

Users who want to withdraw from the ForfeitedAssetsPool burn their Consol tokens and enter the queue, expecting to get underlying assets from the pool based on the current value. However, there is no guarantee that the same amount of ForfeitedAssetsPool token will be worth a similar value when this withdrawal request is processed, and users may end up withdrawing their Consol tokens at a much lower value than expected.

Consider allowing users to provide a minimum value when requesting a withdrawal from the ForfeitedAssetsPool, or clearly document this behavior, since users who enter the ForfeitedAssetsQueue should be aware of it.

Due to the structure of the USDX queue, some lenders who deposited into the origination pool and received Consol tokens will not be able to withdraw their principal or yield for a significant period of time.

Because a queue was chosen for this process, the lenders who submit their withdrawal requests in the USDX queue first will be fully redeemed, with interest, entirely first. And the ones who queue after them will have to wait until the final mortgages are fully repaid to redeem their principle and realize any interest.

As a result, some lenders will be able to withdraw within hours to days, while others will have to wait months to withdraw any amount of their Consol backing amount.

Consider restructuring the Consol redemption such that all Consol holders can withdraw a portion of their backing tokens as principal and interest is paid down over time, where this portion increases for all Consol holders evenly over time as payments are made.

The forfeited assets queue burn function iterates through all of the supported tokens and transfers them out to the burner relative to their holdings of the forfeited assets shares.

However, if any of the redeemedAssets tokens reverts on zero value transfers and the corresponding contract balance is empty, due to no forclosures occurring for that token, then a zero-value transfer revert will occur.

This will result in a stuck forfeited assets queue until a forclosure occurs for that asset or the admin sends some tokens to the contract to correct it.

Only perform the safeTransfer call if the amount to transfer is nonzero.

In the burn function of the MortgageNFT contract there is no clearing of the getMortgageId or getTokenId mappings for the relevant tokenId. Therefore, the tokenId continues to occupy the entries in these mappings and act as if it still existed.

Remove the entries associated with the tokenId that is being burned in the burn function.

For the last holder of the Consol token the yield generated is trapped in the withdrawal queue contract and cannot be recovered by the protocol.

Consider the following series of actions using a withdrawal request and subsequent cancellation as an example:

• 100 total shares 100 total tokens
• Request withdrawal of 100 shares worth 100 tokens
• Withdrawal time: 100 shares worth 110 tokens, price per share = 1.1
• 100 total shares 110 total tokens
• SharesMinted = 100 * (100 - 100) / (110 - 100) > (100-100 totalShares is 0) > 100
• Shares - shares minted = 0
• No Adjustment for totalSupply or the sender’s balance
• Transfer(100)
• Convert to shares(100) > shares = 100 * 100 / 110 = 90.909 shares transferred
• 100 - 90.909 = 9.091 shares representing 10 tokens remain trapped in the withdrawal queue contract.

If the final Consol divestment is large this could result in a significant amount of funds trapped in the lending queue.

Consider adding a way to rescue any yield that could be trapped in the lending queue that would occur when the last Consol holder divests or cancels from the queue.

In the _prepareOrder function the interest rate for an order is fetched from the PythInterestRateOracle. However, the interest rate recorded from the oracle at the time of request creation may be significantly different than the current rate in effect when the mortgage is actually originated.

Since there is no validation that the expiry is at most a certain amount in the future it’s possible that some mortgage requests could stick around in the OrderPool and use stale interest rates when originated.

Consider validating that the expiry is not too far in the future upon creation requests.

When assets are forfeited to the forfeited assets pool during foreclosure the amount of outstanding debt is minted as forfeited asset shares, however the entire mortgage collateral is deposited into the pool.

In cases where the mortgage position has already had payments made these changes the exchange rate of forfeited asset shares and allows the Consol holder at the front of the forfeited assets queue to withdraw more value than their consol should be worth.

For the most explicative example, consider a mortgage where all but 1 wei of the termBalance has been paid off, and the collateral is 10 BTC. The amount flash swapped from the Consol contract is simply 1 wei, and therefore the amount of new forfeited assets shares minted is just 1 wei. However, 10 BTC goes into the forfeited assets pool, and all of a sudden, the first Consol holder in the queue can redeem 1 wei of Consol for 10 BTC, while the queued redemptions behind them do not get the liquidity they should have to exit from this foreclosure.

Either mint and transfer the current dollar amount of all collateral forfeited to the Consol contract, thus increasing the balance of all Consol holders and allowing them to exit fairly through the forfeited assets queue or consider converting between the amount of forfeited assets shares and underlying dollar value of collaterals in the forfeited assets pool so that Consol holders can be redeemed at a 1 USD value per Consol like the other exit queues.

The whitepaper suggests that multiple OriginationPool’s can be used to originate a loan for an order, however the order fulfillment flow in the OrderPool contract only allows for one OriginationPool that has been specified by the user to be used with the mortgage origination.

This may limit the size of individual mortgages that can be created with a single request, and deviates from the whitepaper.

Confirm if this is the expected behavior or not, and if the whitepaper is simply outdated.

The minimum and maximum caps for the chosen collateral token are validated upon a mortgage request but not upon the execution of that mortgage request.

Therefore, if these caps are updated in between the request and execution the resulting order execution may violate the most up to date caps.

Consider validating the minimum and maximum caps for an order during the execution of that order in addition to the request creation.

In the origination flow, if the order being executed is an expansion order the reenqueue boolean is always marked as true when a conversion queue is provided.

However, if the mortgage being expanded is not already in the conversion queue, then this enqueue request will fail with the TokenIdNotInQueue error.

This leads to unexpectedly un-executable expansion orders which must be re-requested without a conversion queue and enqueued in a separate transaction.

Consider only re-enqueuing a mortgage if that mortgage is already present in the ConversionQueue.

The conversion queue transfers collateral tokens directly to the withdrawer’s address. As a result, if the withdrawer is blacklisted for the underlying collateral token, they can halt the withdrawal queue due to a revert which occurs when attempting to transfer to their blacklisted address.

A similar action can occur that prevents the Forfeited assets queue from functioning.

If the transfer fails in any queue, consider incrementing a mapping value whereby the withdrawer can claim the collateral token in a separate transaction. Otherwise, be sure to only list collateral tokens which do not have a blacklist mechanism.

In the periodPay and penaltyPay functions there is no validation that prevents a 0-amount payment from occurring. This could lead to unexpected issues since a zero payment is not an expected operation. Furthermore, a minimum amount validation could be performed to add additional safety checks.

Validate that the amount being paid in the periodPay and penaltyPay functions is nonzero and ideally introduce a minimum amount validation for these payments.

The refinanceMortgage function performs no validation that prevents a user from re-financing a mortgage that is nearly entirely paid off or entirely paid off already. This allows the user to create a mortgage with a term balance of 0 which can lead to unexpected edge cases in the protocol.

Validate that the mortgage being refinanced has above a minimum threshold of debt before allowing the refinance to occur.

In the processWithdrawalRequests function for the conversionQueue, the numberOfRequests is only decremented when a withdrawal request is fully fulfilled. Additionally, there is no maximum limit on the size of a withdrawal request.

Therefore, for large withdrawal requests it is possible that the queue processing can be halted due to the invocation of the processWithdrawalRequests function with even a 0 numberOfRequests value. This is possible if there are many mortgages near the minimum size which can be matched against a single large withdrawal request, requiring that many loop iterations be completed before the processWithdrawalRequests function can finish execution, which could use more than the block gas limit to fully process.

To resolve this issue, consider implementing a maximum size for the individual withdrawal request amount, such that it is a multiple of the minimum mortgage size which can be processed in a single block.

Otherwise, adapt the usage of the numberOfRequests value in the processWithdrawalRequests function such that it decrements upon every loop iteration, even when the withdrawal request is only partially processed and a mortgage is fully processed.

When a position is fully converted without making any period payments the termBalance becomes 0. As a result, the position no longer accrues late payments in the _applyPendingMissedPayments function and cannot be forclosed on as a result.

For any position which had accrued penalty payments and then gets entirely converted they will not have any time constraint on paying these penalties as forclosure cannot occur.

The user still has to pay off these penalties to redeem their position, and this serves as the only incentive to do so. Be aware of this case where a mortgage cannot be forclosed on and penalty payments can be put off.

The requestMortgageCreation function accepts a creationRequest object with a mortgageId NFT id attribute to mint the corresponding mortgage NFT id to the caller.

However, if this mortgageId is already taken then the mint invocation and thus the higher level requestMortgageCreation function invocation reverts.

As a result, it may be possible for a malicious actor to frontrun and DoS a user’s calls to the requestMortgageCreation function by taking their specified mortgageId first.

Be aware of this and consider refactoring the mortgageId logic to be keyed based on the owner of the mortgage.

In the SubConsol contract there is no way to withdraw yield from the underlying yieldStrategy beyond the collateral deposited into it. This may be unexpected depending on the intended use-case for the yieldStrategy and the target yieldStrategy implementations.

Be sure there is a way to withdraw the yield from the yieldStrategy so that this amount is not lost.

In the case where an asset is being updated as no longer supported and has a nonzero balance in either the USDX or Consol contract a stepwise decrease in the vault shares value will occur.

In the case of Consol this creates a DoS for the queues which rely on the share price increasing monotonically due to the burnExcessShares function.

Be aware of this risk and be sure to never delist an asset that has a nonzero balance in the Consol contract. It may be best to not explicitly limit this at a contract level to allow the admin to make important updates in emergency situations.

Furthermore, consider updating the burnExcessShares function so that it can successfully execute and early return if the share value has decreased since the withdrawal request was made.

In the requestMortgageCreation function there is no validation which prevents a user from providing a SubConsol address that is invalid either because it is not supported as a SubConsol address or it does not match the collateral token the user is using.

This leads to orders being created which are inexecutable due to reverts in the LoanManager.createMortage function.

Add validation in the requestMortgageCreation to prevent users from creating orders with a SubConsol address that is not a registered SubConsol contract and does not support the collateral they are providing.

In the addConfig function there is no validation that prevents a config with a zero consol address from being added, such a config would be able to be added again, creating a duplicate in the _oPoolConfigIds list and not allowing this config to be removed.

Consider validating that the consol on the provided config is not the zero address.

Across the codebase in the ForfeitedAssetsPool, Consol, USDX, and SubConsol contracts 0 value deposits and withdrawals or burns are allowed which may lead to unexpected edge cases or event emissions.

Validate that the amount being deposited or withdrawn or burned is nonzero to avoid unnecessary paths and unexpected edge cases.

In the GeneralManager requestMortgageCreation function among others, there is no ether value refund in the event that the value sent is greater than the order pool’s gas fee.

In the event that the default admin re-assigns the gas fee value for the OrderPool before a user’s transaction is recorded, this may result in the user sending more gas than is necessary and not receiving an appropriate refund back.

Consider sending a refund of any excess ether to the caller at the end of the transaction.

In the periodsSinceTermOrigination function the periods amount is casted to a uint8 without checking if the result fits within a uint8 object. This will result in a perturbment of the resulting periods value when the time passed exceeds 256 periods.

In the periodsPaid function the resulting value is casted to a uint8 without validating that the result fits within this storage.

In the interestRate function of the pyth interest rate oracle the rate result is casted to a uint16 without checking if it fits within this storage.

In the price function the price result is casted to a uint64 value without checking if it fits within this storage.

Use safeCast for all of these casting results as well as all other casting that occurs in the codebase to avoid any unexpected overflows.

In the convert function, when a partial conversion is occurring the remaining termPaid is calculated using Math.Rounding.Ceil, however the termPaid amount should be rounded down to round against the user and in favor of the protocol as a best practice.

Round the termPaid by the user down instead of up to favor the safety of the protocol.

The OrderPool sendOrder function allows orders to be stored that have an expiration that has already passed.

Do not allow these orders to be stored and instead revert.

The fulfiller of a mortgage origination receives no spread on the collateral price for fulfilling the order.

Furthermore, the price used is a static price from when the order was originated, therefore if price increases after the pyth price used at the time of the request initiation then it is unlikely that the order will be fulfilled, and this is simply a waste of the gas downpayment made by the user.

Consider improving the fulfillment incentives so that mortgage requests are more likely to be fulfilled.

Given that the Pyth feed for the US treasury rate is only updated during relevant market hours it is not possible to query the rate with a recent published result during off-market hours.

As a result, actions like loan request creation and refinancing are unavailable during off-hours.

Be aware of this behavior and be sure to surface it to users in a palatable way.

In the case where a borrower attempts to pay their full penalty or term value, a malicious user can cause the borrower's payment transaction to revert by frontrunning and paying the loan down by 1 wei.

The reason for the revert is because the borrower is not able to overpay their penalties or outstanding principal. The single-wei donation will cause the borrower's full payment to overpay by 1 wei, leading to their payment transaction reverting.

This is possible for all penalty payments and only period payments when the mortgage has a payment plan.

The simple solution is to lock down payments only to the mortgage owner. Otherwise, allow the borrower to overpay and simply refund the excess balance at the end of the transaction.

The removeAsset function in the ForfeitedAssetsPool contract removes the asset from the list without checking the contract balance. If this asset has already been forfeited before, that balance remains locked in the contract unless the asset is added back to the list, since a removed asset cannot be redeemed during a burn.

Consider checking the contract balance in removeAsset and either reverting if the asset has already been forfeited or clearing the balance. However, note that clearing the balance might be unfair to users who are awaiting withdrawal.

The USDX.sol contract allows users to deposit and withdraw various USD-valued tokens. Since the deposit and withdrawal flow does not impose fees, users are able to swap between tokens freely.

This poses a problem because when users specify their withdrawal token, a malicious actor can deposit another token and withdraw all of the user's desired token's balance. All withdrawals can be temporarily grieved through this method.

Note this possibility in the documentation.

The ForfeitedAssetsQueue handles each withdrawal request sequentially and must fill have enough liability in the ForfeitedAssetsPool to be able to fulfill the request, otherwise it reverts.

Therefore, a large Consol holder can queue a withdrawal through the ForfeitedAssetsQueue and DoS the queue until there are enough forfeited assets to satisfy the large withdrawal.

The user could cancel their request resulting in simply stalling the distribution of forfeited assets and a delay in allowing users to exit their Consol positions.

This is unlikely to occur in practice because it would take a large financial commitment by the malicious user and provide little to no benefit.

Document the possibility of such a scenario.

The _findFirstTriggered function has the following Natspec comment: "@return tokenId the tokenId of the first MortgagePosition in the Conversion Queue that has a trigger price greater than or equal to the input trigger price."

However, the function returns the position with a trigger price less than or equal to the input trigger price.

Change the comment.

Redeemed or foreclosed mortgages remain in the ConversionQueue until the dequeueMortgage function is called. If such a mortgage becomes the head of the queue, the processWithdrawalRequests function will revert until that mortgage is removed.

Consider either dequeuing a mortgage when it becomes inactive or skipping it in the while loop of the processWithdrawalRequests function.

When updating the forfeitedAssetsPool in Consol.sol, the previous value of forfeitedAssetsPool remains in the supportedTokens enumerable set.

The cap value for the previous pool is deleted, therefore it would be wise to remove the pool token from the list of supported tokens to avoid its use in the flashSwap() function.

Consider removing the previous forfeitedAssetsPool from the enumerable set with supportedTokens.remove().

When an order is expired, the order record is deleted twice. Firstly, in the if clause:

    if (order.expiration < block.timestamp) {
      // Cancel the mortgage request
      IGeneralManager(generalManager).burnMortgageNFT(order.mortgageParams.tokenId);

      // Delete the order
      delete _orders[index];

      // Emit the PurchaseOrderExpired event
      emit PurchaseOrderExpired(index);

And finally at the end of function execution:

    collectedGasFee = order.gasFee;

    // Delete the order
    delete _orders[index];

Remove the deletion in the if clause as the order will eventually be deleted at the end of function execution.

Borrowers may close their mortgages early if they choose. However, closing early does not recalculate the interest or total amount payable, and borrowers are still required to pay interest for the entire mortgage term.

Document this behavior and ensure users are aware of it.

Consol is a MultiTokenVault that supports USDX and ForfeitedAssetsPool tokens. The total supply of MultiTokenVaults is calculated based on the balances of the supported tokens, and MultiTokenVault assumes that all supported tokens share the same unit of account (UOA).

However, the UOA of the supported tokens in Consol can differ significantly. For example, a single share of ForfeitedAssetsPool can easily be worth 1.50 due to seized collaterals, while USDX is 1,00. As a result, Consol supporting both of these tokens contradicts the intended behavior of a MultiTokenVault.

Since seized assets can be purchased at 1:1 by burning Consol, a race condition can occur when a single share of ForfeitedAssetsPool is worth more than $1. Users will attempt to withdraw from the ForfeitedAssetsPool first, followed by withdrawals from USDX.

Consider documenting this if it is the intended behavior. Otherwise, ensure that all supported assets in Consol share the same UOA. This may require calculating the mint amount of ForfeitedAssetsPool tokens at the time of foreclosure based on the value of the seized collateral and the value of Consol at that point.

When calculating the number of penalties that the borrower owes, the penalty calculation rounds down in favor of the late borrower. Likely this only results in a negligible advantage for the late borrower; however, it is best practice to always round in favor of the protocol.

Use Math.Rounding.Ceil to round in favor of the protocol.

oPoolAdmin in the OriginationPoolScheduler must have the DEFAULT_ADMIN_ROLE. However, this role is not granted to oPoolAdmin during the initialization process. Instead, it is granted to msg.sender, which may not be the oPoolAdmin.

Grant the admin role to oPoolAdmin during initialization.

If the admin updates the penalty rate value stored in GeneralManager.sol, the currently late mortgages could experience increased penalty fees. These mortgage owners enter into the agreement with a set rate and can experience fluctuations depending on the global penalty rate setting.

Consider writing the current global penaltyRate for the mortgage at the time of origination request and use that value in subsequent penalty calculations.

There is a possible, but unlikely theft vector within USDX.sol. The USDX contract allows users to deposit and withdraw various USD-valued tokens. Because each token may have different decimals, there is a scaling mechanism that multiplies the token amount by a numerator and divides by a denominator.

The potential theft vector occurs when the denominator is greater than 1, for example 10e12.

Imagine USDX is set up with 6-decimal precision and 18-decimal USD tokens must be scaled down to 6 decimals. When calculating the mint amount of 1e18 token, the resulting amount would be 1e6.

When the user attempted to withdraw 1e18 tokens, the same conversion would take place and require that they burn 1e6 pool tokens.

The theft can occur if the user specifies that they wish to withdraw (1e18 + 1e11) tokens. With precision loss, the amount of pool tokens to burn will still be 1e6.

However, the user will receive 1e11 tokens more than they deposited. They can perform this call many times to drain funds from the pool.

Ensure that the denominator is always 1, as is the case in the deployment script.

When cancelling a withdraw request, the consol token is transferred to the user, however, the amount is not updated to 0 until after the external call. If Consol were to include a hook for the receiver of the token, the withdrawal queue could be drained. It is unlikely that such would be the case.

Move the storage writes above the external call to the consol token.

The calculateNewAvergageInterestRate function misspells average.

Correct the spelling of average in the calculateNewAvergageInterestRate function name.

• Corresponding -> a Guardian proof of concept
• Surplus -> a Guardian proof of concept
• Additional -> a Guardian proof of concept

Consider, fixing these typos.

In the cancelWithdrawal function the validation on the index ensures that the index is less than the withdrawalQueueLength. However, the withdrawalQueueLength is variable and the indexes are now fixed.

Therefore, there can be a withdrawal at absolute index 10, while the withdrawal head is at 10 and the withdrawal queue length is 1. The index provided to the cancelWithdrawal function must be 10, which is greater than the withdrawalQueueLength of 1.

Consider the following example:

• The withdrawal queue is empty with the head being 0 and length being 0
• Bob creates withdrawal A, which is stored at index 0, the head is still 0 and length is now 1
• Alice creates withdrawal B, which is stored at index 1, the head is still 0 and length is now 2
• Bob’s withdrawal A is now processed; the head is at 1 and the length is now 1
• Alice attempts to cancel her withdrawal, however providing the index 1 fails the index validation and she cannot cancel

Furthermore, the current validation allows requests that are before the head to be cancelled, however these requests have already been processed.

Correct the index validation such that only indexes that satisfy head <= index < head + length are allowed.

The amountConverted variable has been changed to represent the amount of debt converted in past terms. And the current term converted value is now tracked in the termConverted variable. This subtly changes all formulas that currently use amountConverted, and if termConverted is not also tracked there this causes notable accounting issues.

The amountForfeited function uses amountConverted but does not account for the newly introduced termConverted correctly.

The original implementation of amountForfeited was as follows:

amountBorrowed - amountConverted - amountOutstanding

Where amountOutstanding = amountBorrowed - amountConverted - amountPrior - convertToPrincipal(termPaid)

And in this context amountConverted represents the entire amount converted across all terms for the mortgage.

This simplifies to:

amountBorrowed - amountConverted - amountOutstanding

= amountBorrowed - amountConverted - (amountBorrowed - amountConverted - amountPrior - convertToPrincipal(termPaid))

= amountPrior + convertToPrincipal(termPaid)

This is correct because this is the amount of debt that the user has already paid and is forfeiting during liquidation.

However, the new implementation of amountForfeited is as follows:

amountBorrowed - amountConverted - principalRemaining

Where principalRemaining = amountBorrowed - amountConverted - amountPrior - convertToPrincipal(termPaid + termConverted)

And in this context amountConverted represents the entire amount converted across only past terms for the mortgage.

This simplifies to:

amountBorrowed - amountConverted - principalRemaining

= amountBorrowed - amountConverted - (amountBorrowed - amountConverted - amountPrior - convertToPrincipal(termPaid + termConverted))

= amountPrior + convertToPrincipal(termPaid + termConverted)

This is incorrect because now the mortgage holder is credited with forfeiting the converted amount in the current term.

Update the forfeitedAssets function as follows:

function amountForfeited(MortgagePosition memory mortgagePosition) internal pure returns (uint256) {
	if (mortgagePosition.status != MortgageStatus.FORECLOSED) {
		return 0;
	}

- return mortgagePosition.amountBorrowed - mortgagePosition.amountConverted - mortgagePosition.principalRemaining();
+ return mortgagePosition.amountPrior + mortgagePosition.convertPaymentToPrincipal(mortgagePosition.termPaid);

}

The enqueueMortgage function uses the _calculateRequiredGasFee function to calculate the msg.value that is required to send to all of the conversion queues being added with the conversionQueueList parameter.

However, the _calculateRequiredGasFee function computes the required gas fees for all conversion queues that the tokenId currently has in their conversionQueues(tokenId) list, not just the ones that are being added with the call to enqueueMortgage.

Therefore, the requiredGasFee is larger than what is necessary to send to the new conversion queues, and this amount is lost to the mortgage owner.

Instead of using the _calculateRequiredGasFee function which computes the necessary gas fee for all of the conversion queues, old and new. Implement a bespoke for-loop to loop through all of the new conversion queues in the provided conversionQueueList parameter.

In the applyPenalties function, for mortgages that do not have a payment plan, the additionalPaymentsMissed are assigned to:

periodsSinceOrigination - mortgagePosition.totalPeriods + 1 - mortgagePosition.paymentsMissed

However, in the periodPay and convert functions, the mortgagePosition.paymentsMissed value is assigned to the following for mortgages with and without payment plans:

mortgagePosition.paymentsMissed = _periodsPaid > periodsSinceOrigination ? 0 : periodsSinceOrigination - _periodsPaid;

There are several issues with this, the most glaring being that in conversion the paymentsMissed update does not account for the fact that mortgages without a payment plan cannot have missed payments before their mortgage is complete. This is not an issue in the periodPay function since mortgages without payment plans must be totally paid off when invoking the periodPay function.

Secondly, if the periodsSinceOrigination is larger than the total mortgage periods then the paymentsMissed will always be assigned to a nonzero amount even if the total debt has been paid off. The paymentsMissed logic should be updated like so:

    uint8 periodsSinceOrigination = mortgagePosition.periodsSinceTermOrigination(latePenaltyWindow);
+   uint8 cappedPeriodsPassed = Math.min(periodsSinceOrigination, mortgage.totalPeriods);
    uint8 _periodsPaid = mortgagePosition.periodsPaid();
-   mortgagePosition.paymentsMissed = _periodsPaid > periodsSinceOrigination ? 0 : periodsSinceOrigination - _periodsPaid;
+   mortgagePosition.paymentsMissed = _periodsPaid > cappedPeriodsPassed ? 0 : cappedPeriodsPassed - _periodsPaid;

Thirdly, both the periodPay function and the convert function do not account for the + 1 that is applied to the additionalPaymentsMissed in the applyPenalties function and therefore overwrite this additional missed payment period for mortgages that do not have a payment plan.

Firstly, in the convert function, be sure to account for the fact that mortgages without a payment plan cannot have missed payments before their term is up.

Secondly, account for the additional payment period that is applied to non-payment-plan mortgages in the periodPay and convert functions.

In the cancelWithdrawal function in some cases the number of shares being transferred for the request.amount will round to be one wei greater than the number of shares that the LenderQueue holds for that withdrawal request after the burnExcessShares invocation.

In cases where there is only one withdrawal request in the LenderQueue, the cancellation will revert due to ERC20InsufficientBalance, and in the case where there are multiple withdrawal requests, the cancellation will take 1 wei that should have been allocated to other withdrawal requests, ultimately causing a revert for the last withdrawal being cancelled or even processed.

The rounding of burnExcessShares and the subsequent transfer must be overhauled to avoid sending an additional wei. A simple solution may be to transfer request.amount - 1 to ensure this case does not arise.

The latest logic in the origination flow sends the available consol token balance of the GeneralManager contract to the immediate Origination pool being processed.

IERC20($._consol).safeTransfer(_msgSender(), IConsol($._consol).balanceOf(address(this)));

This however leaves an insufficient amount of consol tokens to repay the following origination pools. This is masked by the following logic:

uint256 consolBalance = IConsol($._consol).balanceOf(address(this));
if (consolBalance < returnAmount) {
    IConsol($._consol).deposit($._usdx, IConsol($._consol).convertUnderlying($._usdx, returnAmount - consolBalance));
}

Which will take from the USDX balance of the contract to repay the following origination pools. This is not surfaced immediately as an error because the fulfiller now receives the balance of the contract instead of the purchaseAmount they should have received.

Instead of transferring the entire GeneralManager balance to the immediate origination pool being processed, transfer the specified returnAmount.

This way there still may be small rounding which affects the fulfiller, but it is not confused with an amount that covers a dearth for other origination pools.

In the OrderPool contract the executor of the processOrders function has an adverse incentive to wait until orders are expired before processing them, this is because the order.mortgageGasFee is added to the collectedGasFee in _processOrder when an order has expired.

This means that when an order expires the user loses their mortgageGasFee even though a mortgage was not created, and the executor gets to keep it for themselves.

If an order expires, still collect the order.orderPoolGasFee, however the order.mortgageGasFee should be refunded to the user. It should be refunded by incrementing a mapping value entry and allowing the user to claim this Ether in a separate transaction to avoid any DoS vectors or gas griefing vectors. Or by wrapping the Ether to wrapped Ether which does not allow for these attacks.

When a mortgage creation request is created, the purchaseAmount and collateralAmount are defined based on the price of the asset using the pyth oracle in the block of the creation request.

The creation request is then executed up to 5 minutes later, using these prices from as old as 5 minutes ago.

This allows users to potentially arbitrage price movements which occur in those 5 minute timeframe, by deciding whether or not their order is executed.

For example:

• At time 100, User A makes their expand balance sheet request when Bitcoin is $100,000
• User A configures their creation request to enqueue into a conversion queue that they are already enqueued in
• User A sets their receive function to revert based on a boolean switch, which is currently set to true to prevent the order from being executed
• At time 110, Bitcoin rises to $110,000
• User A sees that Bitcoin price has moved in their favor and flips the boolean switch allowing the receive function to no longer revert and the refund that occurs on the re-enqueue action to proceed
• User A’s action is now processed using an outdated price of $100,000 per Bitcoin for their borrow
• If price had not moved in User A’s favor, then they would have just left the switch and allowed their order to be cancelled.

Be aware of this time difference arbitrage risk, the OrderPool execution fee may be enough to disincentivize this, furthermore the net interest rate paid on a mortgage will always overshadow what can be arbitraged through this method in the current expiration window.

When processing withdrawals, collateralToUse is computed by dividing the payment by the stored triggerPrice rather than the current oracle price. If the market price has risen above the trigger price, the system will consume more collateral than necessary for the same repayment.

While the borrower accepted the trigger price for conversion, this should not mean that the borrower converts their collateral at a rate lower than its market value. Additionally, the comment in the code states: 'Figure out how much collateral corresponds to the amountToUse at the current price.'

Consider using the current value of the collateral during conversions, as in the previous version. However, if this change is an intentional design choice, clearly document this behavior for users, since they will convert their collateral at a lower rate unless the trigger price matches the current value. In that case, also update the comment accordingly.

In the periodPay function validation is performed on mortgages with no payment plan to ensure that the amount is at least the mortgagePosition.termBalance.

However, for mortgages with no payment plan that have been partially converted, there isn’t enough debt to pay off the entire termBalance, since the termConverted is reducing a portion of this debt.

Due to the refund logic this doesn’t strictly prevent these mortgages from being paid off, however users must obtain and approve more than their actual debt amount of Consol to pay off their remaining debt in this case.

Update the validation to:

if (!mortgagePosition.hasPaymentPlan && amount < mortgagePosition.termRemaining()) {
  revert CannotPartialPrepay(mortgagePosition);
}

Mortgages without payment plans require that their entire debt is paid off in a single periodPay invocation.

However, these mortgages in particular, create a large arbitrage opportunity when they pay back their debt. Where the forfeited Consol that pays the interest on the loan is forfeited, increasing the Consol price.

This may be abused by third parties by depositing into USDX, then obtaining Consol tokens right before the large payment and then immediately exiting one of the available queues, likely the USDX queue using their own deposited USDX and the mortgage holders USDX, as long as this queue is not too full.

Furthermore, this may serve as an avenue for the mortgage holder to avoid paying interest. If they are able to flash loan or make a short period loan of a large amount of Consol tokens, they themselves could absorb much of the Consol price increase and re-coup their interest payment.

Be aware of this heightened risk of gaming, particularly for mortgages without a payment plan.

In the cancelWithdrawal function there is no validation that prevents an empty, already cancelled, withdrawal from being processed again.

There is no net effect of processing such a withdrawal, however this execution path should be limited to avoid unexpected outcomes and the emission of a misleading WithdrawalCancelled event.

Validate that either of the request shares and amount must be nonzero to allow processing to occur.

In the MultiTokenVault deposit function the safeTransferFrom action comes after the minting of tokens to the user, therefore the user receives shares before officially paying for them.

To avoid any unexpected re-entrancies with arbitrary supported tokens, the safeTransferFrom should occur before the _mint action so there is no point where the protocol is in an invalid state at the time of an external call.

Perform the safeTransferFrom action before the mint action in the deposit function.

In the addSupportedToken function validation has been added to ensure that the scalarNumerator is greater than or equal to the scalarDenominator.

This disallows tokens with greater than 18 decimals from being used with the system, such as yamV2 which has 24 decimals.

Be aware of this incompatibility or consider removing the validation.

In the USDX burn function the USDX burn occurs for the user at the end of the function after all tokens have been redeemed to the user. However, to follow CEI the burn should occur first.

Invoke _burn after initial validations in the burn function.

In the requestWithdrawal function if the amount is a few wei the action may produce a withdrawalRequest that holds a nonzero amount value but a zero shares value.

This creates an unexpected case in the queues that attempt to process such withdrawals.

Furthermore, if the amount specified is zero, this allows users to create withdrawals with both zero-amount value and zero share value.

Both of these cases are disallowed when the minimumWithdrawalAmount is assigned to a non-trivial value. Ensure that the minimumWithdrawalAmount is always assigned to a minimum reasonable amount to ensure no game-ability and rounding issues are present in all queues.

The ConversionQueue has a whenNotPaused guard which applies to the processWithdrawalRequests function, however the USDXQueue and ForfeitedAssetsQueue do not implement such a validation.

Consider if the USDXQueue and ForfeitedAssetsQueue should use a whenNotPaused modifier for their processWithdrawalRequests functions.

In the OrderPool the processOrders function is the only function which has a nonReentrant modifier which provides limited protection against reentrancy into the processOrders function.

The sendOrder function still poses a risk of unexpected reentrancy and therefore should also use the nonReentrant modifier so that it cannot be unexpectedly entered into during a processOrder execution.

Add a nonReentrant guard to the sendOrder function.

In the case that an order is expired, the collected collateral and USDX are returned to the user and the executor gets to collect the associated gas fee.

However, if the user happens to be blacklisted for the collateral token, the _processOrder function will not be able to execute and process their order as it attempts to send tokens to a blacklisted address.

Be aware of this in the event that any supported collateral tokens implement a blacklist. Consider using a mapping to track balances that a user may claim in a separate transaction rather than pushing assets to the user.

In the _processOrder function the _orders mapping is cleared at the end of the function, however, to follow CEI it would be best if this mapping entry is cleared at the beginning of the function.

Clear the _orders mapping at the beginning of the _processOrder function directly after caching the order in memory.

When creating a mortgage request the collateralAmounts length must match the originationPools length, however this is not validated in the requestMortgageCreation function.

Add validation so that the collateralAmounts length is required to match the originationPools length.

In the OrderPool contract sendOrder function the validation performed on the msg.value is still against the gasFee, however the gasFee only represents the order pool gas fee and does not include the mortgage gas fee that should be provided.

Update the msg.value validation in the sendOrder function so that it includes the _calculateMortgageGasFee result.

A comment in the originationPoolDeployCallback function states:

// Send in the collateral to the LoanManager before creating the origination pool

However, this should read:

// Send in the collateral to the LoanManager before creating the mortgage

Correct the comment.

When enqueuing a mortgage in the ConversionQueue, if the mortgage was previously enqueued it is first removed from the queue to be re-enqueued. In this process the original gas fee is returned to the user, however this allows the user to execute arbitrary action in their receive function.

This may cause unexpected issues due to the user being able to DoS, gas grief the executor, or re-enter into the system unexpectedly.

Instead of sending Ether directly to the user, consider incrementing a mapping value where they can claim their owed Ether in a separate transaction. Otherwise wrap the Ether and send it to the user as to avoid triggering untrusted code.

In the _validateBorrowCaps function there are some baseline validations that the borrowed amounts are within a set range. However, there is no validation that the individual borrows amounts in each originationParameters.borrowAmounts list entry are within an expected range.

Therefore, while the total may be inside the expected range, the amount requested from each origination pool may be unexpected.

Consider validating that each origination pool requested amount is above some minimum expected amount, or at least above 0.

Now multiple origination pools are allowed to be used for an origination of a mortgage. However, there is no validation to ensure that each entry of the originationParameters.originationPools list is unique upon creation of the request.

Consider validating that each entry is unique when a mortgage request is being created.

Throughout the GeneralManager there are important functions which could be reentered into which are lacking reentrancy guards.

These functions include:

• requestMortgageCreation
• requestBalanceSheetExpansion
• originate
• enqueueMortgage
• convert

Add the nonReentrant modifier to these functions.

In the ConversionQueue contract the subConsol storage variable is unused.

Consider removing it.

/**
 * @title QueueProcessor
 * @author SocksNFlops
 * @notice The StaticInterestRateOracle contract is a contract that returns a static interest rate for new Mortgages being originated. //@audit-issue L incorrect Natspec comment
 */

The @notice comment in QueueProcessor contract belongs to StaticInterestRateOracle.

Update the comment.

The convert function in the MortgageMath library checks that the current price is greater than the conversionTriggerPrice.

if (mortgagePosition.conversionTriggerPrice() > currentPrice) {
   revert ConversionTriggerPriceNotMet(mortgagePosition, currentPrice);
}

However, the same check is performed again at line 673, making it redundant.

Remove the redundant check.

When a mortgage is created with a conversion queue or enqueued later, the _conversionQueues and _mortgageEnqueued mappings are populated. However, these mappings are never cleared, even though the mortgage is eventually fully converted.

A fully converted mortgage is removed from its respective ConversionQueue during the _popMortgage call. However, it still remains in other ConversionQueues and must be popped from these queues during withdrawal request processing. Additionally, it remains in the GeneralManager mappings as if it were still in the queue.

Consider updating or clearing the _conversionQueues and _mortgageEnqueued mappings in GeneralManager when a mortgage is fully converted. However, care must be taken to ensure that this does not cause a DoS when the mortgage still needs to be popped from other ConversionQueues during loop iterations.

The ForfeitedAssetsQueue can be held up by large withdrawal requests which cannot be fully processed because the Consol holdings have not acquired enough ForfeitedAssetsPool tokens to withdraw.

This can reasonably occur especially for foreclosures where the remaining debt of the position is small at the time of foreclosure, in which case it would require many such foreclosures to occur to allow the forfeited assets queue to fill a significant withdrawal in the queue.

In future iterations consider allowing partial withdrawals. Otherwise, be aware of this blocking behavior and communicate it with users.

The requestWithdrawal function does not implement a refund mechanism of too much ether is sent, this does not match other functions in the GeneralManager that do perform refunds for native ETH.

Consider introducing a refund for the requestWithdrawal function in future iterations.

In the burn function the Withdraw event for the final withdrawal is intended to have the remaining amount - totalBurned, however this case is entered for all withdrawals that come before the final withdrawal, because the main if case is i == supportedTokens.length() - 1, meaning that the last withdrawal goes into the first if branch.

Update the if case to use the condition i != supportedTokens.length() - 1.

In the ConversionQueue, the count variable which is meant to track iterations of the processing loop can unexpectedly be incremented twice in a single loop iteration when the amountToUse matches both the request.amount and the mortgagePosition.principalRemaining().

This may be unexpected for the caller of the processWithdrawalRequests and may result in less withdrawal requests being processed than expected.

Be aware of this edge case and inform callers of the processWithdrawalRequests function.

The latest logic in the origination flow sends the available consol token balance of the GeneralManager contract to the immediate Origination pool being processed.

IERC20($._consol).safeTransfer(_msgSender(), IConsol($._consol).balanceOf(address(this)));

This however leaves an insufficient amount of consol tokens to repay the following origination pools. This is masked by the following logic:

uint256 consolBalance = IConsol($._consol).balanceOf(address(this));
if (consolBalance < returnAmount) {
    IConsol($._consol).deposit($._usdx, IConsol($._consol).convertUnderlying($._usdx, returnAmount - consolBalance));
}

Which will take from the USDX balance of the contract to repay the following origination pools. This is not surfaced immediately as an error because the fulfiller now receives the balance of the contract instead of the purchaseAmount they should have received.

Instead of transferring the entire GeneralManager balance to the immediate origination pool being processed, transfer the specified returnAmount.

This way there still may be small rounding which affects the fulfiller, but it is not confused with an amount that covers a dearth for other origination pools.