Fair Minting Code Review

[warrenpuffett]

While working on the updated fair minting docs I found a few spots that concerned me.

• Refactor and rename check_fairminter_soft_cap. "check" indicates that this function has no side effects; however, it makes many database updates. The conditional logic contained does not clearly separate when the soft cap is met and when it is not. This code may well be correct but it is not easy to understand.
• minted_asset_commision being a float is dangerous. It should probably be sent from the frontend as an integer. This would also simplify the logic around converting between int and decimal and dividing by a million.
• In fairmint.py, I think it makes sense to move this check in `validate function:

  if quantity != 0:
     fairminter = ledger.get_fairminter_by_asset(db, asset)
     if fairminter["price"] == 0:
         raise exceptions.ComposeError("quantity is not allowed for free fairminters")

• Related to the above, In fairmint.py in validate we can simplify the if fairmint["price"] > 0 block by just setting quantity = Fairminter.max_mint_per_tx if Fairmint.price == 0
• I believe we need to add a check that Fairminter.soft_cap_deadline_block is greater than start_block.
• We need a check to ensure, when Fairminter.price is 0 and Fairminter.hard_cap is set that Fairminter.max_mint_per_tx divides into Fairminter.hard_cap evenly.

[ouziel-slama]

While working on the updated fair minting docs I found a few spots that concerned me.

Thank you very much

• Refactor and rename check_fairminter_soft_cap. "check" indicates that this function has no side effects; however, it makes many database updates. The conditional logic contained does not clearly separate when the soft cap is met and when it is not. This code may well be correct but it is not easy to understand.

Done as much as possible without risk to break the protocol

• minted_asset_commision being a float is dangerous. It should probably be sent from the frontend as an integer. This would also simplify the logic around converting between int and decimal and dividing by a million.

The conversion has to be done at some point anyway. This way we prioritize ease for the user and that it is consistent with other contracts

• In fairmint.py, I think it makes sense to move this check in `validate function:

  if quantity != 0:
     fairminter = ledger.get_fairminter_by_asset(db, asset)
     if fairminter["price"] == 0:
         raise exceptions.ComposeError("quantity is not allowed for free fairminters")

This was placed here intentionally to avoid a protocol change

• Related to the above, In fairmint.py in validate we can simplify the if fairmint["price"] > 0 block by just setting quantity = Fairminter.max_mint_per_tx if Fairmint.price == 0

if fairmint["price"] == 0 the quantity is just ignored by the contract so no need to update it

• I believe we need to add a check that Fairminter.soft_cap_deadline_block is greater than start_block.

good catch! done

• We need a check to ensure, when Fairminter.price is 0 and Fairminter.hard_cap is set that Fairminter.max_mint_per_tx divides into Fairminter.hard_cap evenly.

we use partial minting for the last mint to solve this problem.