[Audit] FM_PC_ExternalPrice_Redeeming, LM_ManualExternalPriceSetter_v1,

[Zitzak]

What needs to be audit.

The contracts that need to be audited are

• FM_PC_ExternalPrice_Redeeming
• LM_ManualExternalPriceSetter_v1
• ERC20Issuance_Blacklist_v1

[omega-audits]

According to the docs above, this should be the _collateralTokenDecimals, not _issuanceTokenDecimals.

[vendrell46]

I'm confident the implementation is correct as is.

For redemption, we're pricing 'how many collateral tokens you get for 1 issuance token', so the input price should be in issuance token decimals, as stated in the documentation. This maintains consistency with the pricing model where 1 unit is always denominated in the issuance token.

[Zitzak]

Feeds into the conversation about using the collateral price for all calculation see comment

[omega-audits]

The other states don't seem to really be used

[vendrell46]

It will be used when integrated with a different module

[Zitzak]

An indexer will be used to track state changes and redemption orders which will be visualized within our admin panel.

However the block.timestamp can probably be removed as recommended elsewhere, as those can be retrieved from the event

[omega-audits]

issuanceFeeAmount is just one of the fees charged, it's unclear why it does not pass all fees combined

[omega-audits]

(jelle:) I thnk it is because the protocolFeeAmount, projectFeeAmount are taken from the collateral, and the issaunceFee from the issuance token.

[Zitzak]

You are right (jelle), we charge different fees here. The project fee can only be charged from the collateral while the protocol fee can be charge from issuance and collateral.

The event emitting the fee amount in the _createAndEmitOrder function serves the purpose of giving the project information they need for their users, as well as their admin panel.

However this question did reveal to me that what we should have been passing into the function is not the issuanceFeeAmount (which is part of the protocol fee), but the projectFeeAmount. So turns out to still be a great catch @omega-audits

[omega-audits]

buyingisenabled is also checked in super.buyFor, so it could be removed here

[vendrell46]

from my point of view, we need it because the function is public and could be called directly.

[Zitzak]

I have to agree here with @omega-audits, as calling this function directly, as you're mentioning, will direct to the base buyFor where the modifier is located.

Could you please expand a bit on your point @vendrell46. Thank you

[leeftk]

    function buyFor(address _receiver, uint _depositAmount, uint _minAmountOut)
        public
        virtual
        override(BondingCurveBase_v1)
        validReceiver(_receiver)
        buyingIsEnabled
    {
        (uint amountIssued, uint collateralFeeAmount) =
            _buyOrder(_receiver, _depositAmount, _minAmountOut);
        _addVirtualIssuanceAmount(amountIssued);
        _addVirtualCollateralAmount(_depositAmount - collateralFeeAmount);
    }
    ```
    
    This is correct this is redundant and should be removed.