Aconomy

stake() and addStake() allows anyone to stake any ERC20 token. In case the ERC20 token deducts fees on transfer, the calculation while storing/incrementing the stakedAmount would be incorrect.E.g. The Validator stakes 100 tokens, the stakedAmount for that validator increased by 100 on L63. But while transferring tokens on L65 only 90 tokens were transferred (as 10 tokens deducted as some tax/fee by token contract). In this case, the increase of 100 on L63 is incorrect. So this can trick the owner into refunding 100 tokens to the validator address even though less than 100 tokens were transferred. (It should be noted that while transferring the tokens back to the validator with refundStake(), less will be transferred in case of fee deducting tokens.) In piNFTMethods.addERC20() allows validators to add any ERC20 token. In case the ERC20 token deducts fees on transfer, the calculation in updateERC20() for adding _value in erc20Balances mapping would be incorrect. There’s a similar possibility in both piMarket and NFTlendingBorrowing regarding the usage of fees deducting tokens. While this issue can be rare, we have decided to highlight the issue as there would be unpredictable possibilities for an open system like this when it comes to the types and incorrect implementations of ERC20 tokens.

The isLoanDefaulted() function is currently unused in the codebase, while in an isolated event this wouldn't be an issue but the poolAddress contract has no checks for borrowers who default on loans. As it stands, nothing stands between a defaulting borrower and a new loan being approved - not even with the scrutiny of the proposed lender. Within the loanRequest() function, there isn't any check/flag that indicates the borrower submitting a loan request has any bad debt. This isn't taken into consideration when the lender calls AcceptLoan(). The borrower's active loans are added to when in the actual sense, previous loans could be past their expiration time, thus defeating the purpose of a liquidation which occurs when liquidators (who are usually incentivized) clear bad debts from the system. In both FundingPool and poolAddress the precaution needs to be taken if the borrower never returns the taken loan back.

When the lender supplies the amount to the pool with supplyToPool() it's getting checked that the _amount is greater or equal to 1000000.But when the poolOwner tries to repay the monthly amount after accepting the lender's bid, in the repayMonthlyInstallment() on L528 it's getting checked that fundDetail.amount / fundDetail.installment.installments greater or equal to 1000000. Let's say the lender entered _amount as 1000000 for the supplyToPool(), the check on L184 passed as the _amount is equal to 1000000. Then while paying the monthly installment on L528 it will revert as in this case fundDetail.amount / fundDetail.installment.installments would be 1e6/6 = 166666, which is less than 1000000. Resulting in denial of service while paying the monthly amount. While there can be an alternate solution to repay the full amount, the monthly repayment functionality would be useless. A similar situation occurs in poolAddress.sol with the loanRequest amount being checked to be greater or equal to 1000000, and repayMonthlyInstallment checks that the principal divided by the number of installments is greater or equal to 1000000 (1e6). If the borrower takes a loan worth 1000000 (1e6) tokens, he would be unable to repay the monthly

The users can take non collateralized loans in the Fundingpool and poolAddress. The ability to take non collateralized loans creates the possibility where the borrowers may never return the loans.

piNFT and validatedNFT allow functionality to mint NFTs. both piNFT.mintNFT() and validatedNFT.mintValidatedNFT() allows anyone to mint the NFTs.Assuming that these NFTs represent the on-chain presence of real-world tokens, allowing anyone to create/mint the tokens on-chain through these contracts may affect the business logic.

There is a possible scenario where the lender adds the bid with short bidDetail.expiration and can claim NFT directly after the block.timestamp goes ahead of bidDetail.expiration. So here the borrower gets less (or say inconsistent) time frame to pay the loan and interest amount.In this case showing the timeframe on frontend in which the borrower needs to pay the loan would be helpful, so that the borrower would be aware while/before accepting the loan that after the block.timestamp goes ahead of bidDetail.expiration the lender can claim his(borrower's) NFT.

In FundingPool on L504 if (monthsSinceStart > lastPaymentCycle) is true then it returns the fundDetail.paymentCycleAmount that would be the amount for the 1 cycle/month but it can happen that more than one months are elapsed after the last repayment in this case the amount needs to be adjusted accordingly.The same thing is happening in the poolAddress’s viewInstallmentAmount().

Nft is not getting transferred from the NFT owner to the contract when the validator is adding ERC20 with addERC20(). In this case the validator needs to trust the NFT owner, NFT owner may decide to not call redeemOrBurnPiNFT() assuming that the user is already the owner of NFT.

As a way of identifying irregularities or where certain requirements are not met, errors are logged in smart contracts. Many of the require statements currently do not have error messages attached to them. Functions could revert without the user knowing/receiving adequate feedback. Adding error messages can also help while handling exceptions on the front end.

Instead of attestation functionality simple mapping for address to boolean can be used to verify lender and borrower addresses in the poolRegistry. In this way the functionality would be more simple to understand and gas efficient. Eventually the one who can add the lender and borrower would be the same address i.e. pool owner. In turn it will reduce the attack surface.