SparkLend Fork, Follow-Up Review

Function borrow accepts an arbitrary feeAmount through a loan call. If Norlend expects a fee to be paid on borrow, a user can just directly interact with the contract and pass a feeAmount of zero to preserve as much capital.

Add validation on the expected feeAmount for Norlend. Alternatively, consider tieing feeAmount to a fixed percentage of the borrowed/liquidated amount rather than letting the caller supply an arbitrary fee, implement a fixed percentage model (e.g., feeAmount = (borrowedAmount * feePercentage) / 1e18).

Norlend Team: We own all the wallets our customers utilize, so we are not concerned with external parties calling the contract using their own wallets.

Function repay approves amount and then calls SparkLend to repay the amount. The issue is that the paybackAmount within the BorrowLogic is not necessarily equal to amount passed, so a portion of the approved amount will be not be utilized.

This will lead to DoS with tokens such as USDT which require a 0 approval initially.

For all allowances in Norlend, use SafeERC20's forceApprove which will force the allowance to go to zero initially to handle tokens such as USDT. Also, use it after an external call to set the approval to zero after the approval is no longer necessary.

Norlend Team: Added a forceApproval function to the contract that performs the functionality as needed. We also reset the allowance where needed.

Guardian Team: After function swap is performed, the SwapRouter may still have some token approval leftover if the entire amountIn was not utilized.

When users borrow via Norlend, a part of the borrowed assets is paid as a fee to the feeAddress, which is an arbitrary passed parameter, but is validated to be a whitelisted address.

However, whitelisted addresses are also the swap routers and the lending pools. This means users may choose to pay the fee to any other whitelisted address resulting in loss of funds for Norlend.

Create a separate validation for the feeAddress.

Norlend Team: If an external party wishes to utilize the contract without disrupting its function, they are free to do so.

The deployment script currently passes 0x02C3eA4e34C0cBd694D2adFa2c690EECbC1793eE (the PoolAddressesProvider of Spark) to the Vanir constructor, instead of passing 0xC13e21B648A5Ee794902342038FF3aDAB66BE987 (the Spark LendingPool address on Ethereum mainnet).

As a result, the Vanir contract is initialized with the wrong contract reference thereby preventing it from interacting correctly with the Spark lending protocol.

If the contract is meant to integrate with Spark Lend on mainnet, the LendingPool address (0xC13e21B648A5Ee794902342038FF3aDAB66BE987) should replace the PoolAddressesProvider address in the script.

Using the PoolAddressesProvider directly is incorrect in this context because Norlend specifically needs the active LendingPool to perform supply/borrow/repay operations, not just an address provider.

Update the constructor call in DeployVanir (or any relevant deployment script) to use 0xC13e21B648A5Ee794902342038FF3aDAB66BE987 instead of 0x02C3eA4e34C0cBd694D2adFa2c690EECbC1793eE.

Norlend Team: Resolved.

During liquidation, the user pays an additional feeAmount from their collateral. This feeAmount is arbitrarily set by the admin within their LiquidationRequest, and can vary each time.

Without fee validation, the fee may be too small or too large, negatively impacting the protocol and user respectively.

Consider adding on-chain validations for feeAmount on liquidations, or clearly document the fee calculation behavior of your backend system.

Norlend Team: We have a breakdown of the calculations documented in our backend.

Norlend has a target LTV for liquidation within its backend system. If the LTV which Norlend is targetting has a large delta between the current LTV, all liquidation calls will fail as more amountIn is necessary than is approved and available for swap.

Clearly document this behavior and appropriately set the target LTV.

Norlend Team: This is dependent on providing the correct parameters, we have found a target range that works for us and can adjust it to lesser deltas if needed.

The tests are meant to simulate Norlend's operations with current backend configurations, however numerous tests are failing. Some reasons for failure include but are not limited to: 1. Lack of pinned fork block number 2. Improper Target LTV's for partial liquidations 3. Incorrect feeAmount calculation within the tests

All tests should be passing prior to deployment.

Fix all tests and align them with Norlend's backend systems.

Norlend Team: Acknowledged.

Two new functions has been added to the contract to enable the admin to reduce funds - ‘transfer’ and ‘transferEth’. There are no events emitted to notify for the action happening.

Consider if you should emit events in these two functions.

Norlend Team: We have no needs for these events, we utilize a blockchain listener that tracks all Norlend-related activity.

When the reserve configuration in SparkLend isFrozen, supplying liquidity is prevented by repays and withdraws are permitted. If the configuration were set to frozen, most Norlend liquidations would fail since function swap() attempts to supply liquidity that wasn't used as part of the swap cost.

Clearly document this risk.

Norlend Team: Acknowledged.