la-dao / xocolatl-contracts Goto Github PK

Implementar un control para hacerle cap a los depositos de una reserva en HouseOfReserve.
Este parametro sera administrado por la governancia.

Desplegar un nuevo smart contract para depositar WBTC

https://github.com/PipilaDAO/xocolatl/blob/d1355d10d70161bbf94f63a106f1b18449c2ba39/contracts/HouseOfCoin.sol#L66

Existen funciones que se deben identificar como unicamente de governancia.
Compound Governor se puede tomar como ejemplo.
La tarea puede incluir agregar diferentes tipos de access control.

Usando Gelato Network podemos crear una tarea on-chain de que cada vez que existan usuarios liquidables entonces Gelato ejecuta una liquidación en Xocolatl.

https://app.gelato.network/new-task

The admin role has the ability to register a ‘house’. The method accepts two arguments. The ‘house’ and the ‘asset’ address. The ‘asset’ address is the ‘reserved’ and the ‘backed’ addresses from the corresponding house. Each house holds its asset address. This address is the same as the second parameter ‘asset’.

function registerHouse(address houseAddress, address asset)  
external   
onlyRole(DEFAULT_ADMIN_ROLE)   
{   
...   
}  

Recommendation
The contract could remove the second parameter ‘asset’ and take the address value from the ‘houseAddress’ parameter.

The LIQUIDATOR role has the authority to transfer tokens. The ‘safeTransferFrom’ method is solely called by the ‘HouseOfCoin’ addresses. As an extra validation step, the contract could check if the sender is registered in the ‘_isARegisteredHouse’ property.

function safeTransferFrom(  
address from,  
address to,  
uint256 id,  
uint256 amount,  
bytes calldata data  
) public override onlyRole(LIQUIDATOR_ROLE) {  
_safeTransferFrom(from, to, id, amount, data);  
}

After completing the audit recommendations from Cyberscope, Xoc system contracts were refactored and improved.
This issue is to deploy the new contracts and make they live on polygon mainnet.

Los scripts en el repositorio actualmente no preservan en algun archivo los "addresses" de los contratos deplegados.

Si se corre multiples veces se vuelven a desplegar los contratos, sin revisar si los contratos ya estan desplegados o no en esta "chain".

Seguir modelo de Fuji para estructurar los scripts.

contracts/HouseOfCoin.sol#L132

Description
Detected missing events for critical arithmetic parameters. There are functions that have no event emitted, so it is difficult to track off-chain changes.

backedAssetDecimals = IERC20Extension(backedAsset).decimals()

Propuesta:
Debido a que recientemente se hizo disponible el price feed en Chainlink de MXN/USD en la siguiente 'address': https://gnosisscan.io/address/0xe9cea51a7b1b9b32e057ff62762a2066da933cd2, es posible desplegar el sistema XOC en gnosis chain.
Algunos de los miembros con los que se hablo despues del lanzamiento en Polygon indicaron interés en apoyar la liquidez en ese blockchain si el sistema se desplegaba ahi.
La Gnosis chain tambien permite hacer transacciones a muy bajo costo, inclusive mas barato que Polygon.

Observaciones;
El sistema de oraculo de UMA no estará disponible como back-up en Gnosis chain ya que los contratos de implementacion por parte del equipo de UMA nunca fueron integrados. Los contratos que han deplegado se pueden encontrar aqui: https://github.com/UMAprotocol/protocol/blob/master/packages/core/networks/100.json, sin embargo, el "Finder" no regresa valor alguno cuando se llama el metodo getImplementationAddress.

Where:

Description:
After the first audit, it was identified that for a reserve collateral type on a chain, there can only be one type of reverveId in AssetsAccountant.sol. However, there could be multiple HouseOfReserves for the same reserve collateral asset. This is intended behaviour.

In addition, Cyberscope identified that there is missing events for critical arithmetic parameters. There are functions that
have no event emitted, so it is difficult to track off-chain changes.

Recommendation
Add a parameter to how the reserveTokenId is generated to make it unique.

reserveTokenID =
uint256(keccak256(bytes)(abi.encodePacked(reserveAsset,backedAsset,collateral)))

And emit an event when reserveTokenId and BackedTokenId are set.

Ya esta la parte del Daigaro en los contratos, nomas falta el Frontend.

Segun el comentario en el codigo, esta funcion deberia ser interna, pero es publica y cualquiera la puede llamar

https://github.com/PipilaDAO/xocolatl/blob/31fa124dc706e427242adcf8700d1ab5784a7933/contracts/HouseOfCoin.sol#L479-L488

https://rinkeby.etherscan.io/address/0xF3A1C091f110F7b931c02d3603ec8bC771182466#readContract

contracts/HouseOfCoin.sol#L559,561,563,522,562,327,132,560,118

Description
Solidity defines a naming convention that should be followed. Rule exceptions:
● Allow constant variable name/symbol/decimals to be lowercase.
● Allow _ at the beginning of the mixed_case match for private variables and unused parameters.

_globalBase
_liquidationThreshold
_collateralPenalty
_reserveAsset
_liquidationPricePenaltyDiscount
_backedTokenID
_assetsAccountant
_backedAsset
_marginCallThreshold
...

Ticket para discutir la propuesta de una perspectiva técnica

The contract is processing variables that have not been properly sanitized and checked that they form the proper shape. These variables may produce vulnerability issues. The contract doesn’t sanitize the address on the initializer.

function initialize(address _backedAsset, address _assetsAccountant)
    public
    initializer
{
    backedAsset = _backedAsset;
    backedAssetDecimals = IERC20Extension(backedAsset).decimals();
    assetsAccountant = _assetsAccountant;

UMA tiene una fuente del precio MXNUSD de parte de TraderMade.

UMA Docs -- Price IDs: https://docs.umaproject.org/uma-tokenholders/approved-price-identifiers
TraderMade Currencies Supported: https://marketdata.tradermade.com/live-currencies-list
UMA UMIPs Github -- https://github.com/UMAprotocol/UMIPs/blob/master/UMIPs/umip-139.md

Where:

Description:
The contract is processing variables that have not been properly sanitized and checked that they form the proper shape. These variables may produce vulnerability issues. The contract doesn’t sanitize the address on the initializer.

Recommendation
The contract should properly check the variables according to the required specifications. The addresses _reserveAsset, _backedAsset, _assetsAccountant and _WETH should not be zero.

BSC is one of the networks with the highest adoption in Mexico and curiously it is the first network that many people use for the first time.

$jMXN is already in BSC where there are also DEXes ve(3,3) deployed.

By launching $XOC in BSC it is possible to generate yield and attract more users, increasing the XOC minted in BSC.

This support of the multi-chain function proposed by LaDAO and will create an opportunity to put XOC to a higher impact use. Thus becoming a multipurpose stable.

BSC es de las redes con mayor adopción en México y curiosamente es la primera red que muchas personas utilizan por primera vez.

$jMXN ya se encuentra en BSC donde también hay DEXes ve(3,3) desplegados.

De lanzar $XOC en BSC es posible generar yield y atraer a más usuarios, incrementando el XOC acuñado en BSC.

Esto apoyado de la función multi-cadena propuesta por La DAO y creará una oportunidad de darle un uso a XOC de mayor impacto. Convirtiendo así una estable multipropósito.

Refer to Dework task

Este repo tiene licensia MIT, eso significa, has lo que quieras con el codigo.

Sugiero cambiarlo a GPL, que implica, has lo que quieras con el codigo, pero libera el codigo de tus modificaciones con la misma licensia open source.

El razonamiento es que ahora alguien nos puede forkear, avanzar el protocolo y no compartirlo, con GPL alguien forkeandonos estaria contribuyendo a open source, y si sus modificaciones fueran utiles las podriamos integrar nosotros.

The contract contains statements that are not used and have no effect. As a result, those segments increase the code size of the contract unnecessarily.

An attacker has the ability to mock the mintCoin parameters in order to mint unlimited amount of backed tokens. The caller can create a custom contract that behave similarly to a “houseOfReserve” contract.

Recommendation
The contract should accept parameters only from approved addresses that cannot be manipulated.

El sistema box permite desplegar un contrato, del cual se puede deplegar un sistema completo.
Asi se mantiene un control rigido para que las direcciones de los contratos sean las mismas en todos los blockchain donde se haga despliegue.

Where:
Throughout code

Description:
Solidity defines a naming convention that should be followed. Rule exceptions:
● Allow constant variable name/symbol/decimals to be lowercase.
● Allow _ at the beginning of the mixed_case match for private variables and unused parameters.

HouseOfReserve.sol contract has functions that take variables with _ as prefix, and this does not follow solidity naming conventions,

Recommendation:
Follow the Solidity naming convention.