During the month of November, the Attic Lab team has been an amazing smart contract review partner. Coming from the staking & validator world, but also operating many different spokes within the blockchain world, the team conducted a thorough, holistic approach towards reviewing our smart contracts.

The full smart contract review report may be accessed here at the StableXSwap Github repo.

StableXSwap Team Updates After Attic Lab Review Report

Below, we will respond to all the great suggestions that the Attic Lab team made in their review. We want to make sure that users know that we take all security concerns seriously and will always take the time to build things safely, which is why we first made sure we addressed all suggestions made by the Attic Lab team. An audit is not just a stamp of approval, but an ongoing process to further make our defi contracts a safer place to keep money. Without further ado:

Regarding the proxies, we ultimately removed them again in our final deployment, as the errors caused for having multiple Solidity versions as well as difficulty in ensuring the proper linkage between Factory and Router contracts, limiting the true upgradability of these contracts (which we will touch on in another point)
We also reduced the fee on withdrawal from 1% to 0.4975% to avoid any possible errors when users withdraw using the default slippage parameter of 0.5%
With regards to the initilaising and having an initcodehash, this initcodehash cannot be updated and makes the router fixed, and users may check this initcodehash to ensure they are interacting with the correct router. We will further work to make sure our contracts are fully labeled and verified on BSCscan to make sure users use the correct contracts.
We have fixed an error in a previous version of our comments that left in the proportion (4/5ths) of the rewards that go to STAX treasury (and subsequently STAX stakers as part of our BLISS program) and 1/5th going to LPs. This did not affect the code and had no changes to our fee distribution logic, only failed to update this comment in the explanation of the code on one of the published branches. We will be sure to make sure our comments are the clearest in the defi space so regular users can read and understand our contracts.
Regarding the additional recommendations mentioned in the third page of the audit review regarding our burn (used in removeLiquidity) function:
1. The _mintFee function was chosen to return a boolean (true/false) rather than an address because a bool takes up fewer slots in EVM’s stack than an address does, and that this burn function was already running into some “Stack too deep errors”, so the saving of the stack slot is paramount to the increased efficiency of returning a fee address instead, where we could limit the scope of the call of the feeTo address within a smaller scope within the burn function.
2. Regarding the updating values of amount{0,1} within the burn function, this suggestion was great in being able to reduce the reliance on the .mul function from safe math and create a more simple calculation for levying the fee on the withdrawal. We have since revised this to divide by 201 instead of 100 as stated in the audit review to achieve our new withdrawal fee of 0.4975%. This fee is a max fee — users who hold STAX will receive fee rebates on this, and depending on their holdings, can receive up to 100% of the fee reimbursed back to them for fee-free withdrawals. This number was chosen so that user would not need to toggle their slippage parameter which is default set to 0.5% for all users. Users can now utilise the front end to toggle Expert Mode if they would like more custom slippage parameters, but the regular user will not have to worry about this.
3. This suggestion is one that if we were need to further optimise, this is a good future improvement. However, we wanted to not change the logic of the current mintFee parameter, which works fine on any pool update (add/withdraw liquidity), but this transfer of the withdrawalFee only occurs on withdrawals (burns), so from a complexity standpoint the contract wouldn’t have to deal with this on any mints (deposits of Liquidity). With BSC transactions being relatively cheap, we can afford this additional transfer call to make the contracts more composable and distinct.
4. Again, due to the limitations of the EVM stack size within a function, storing a fee variable would add one more thing to our stack, so to avoid the Stack too deep error, we hardcoded the fee to be a higher end of what we expect it to be in the long run, and then rebate back via governance and STAX holdership, but in an ideal world, we would love to have this be a governance-upgradeable fee that the community can help decide. This can be worked on in future upgrades.
Regarding division methods, Uniswap in fact utilised this before SafeMath became popular, and never needed the use of a division at all in their contracts somehow, so they didn’t have a .div function whatsoever, so the shortest way we were able to include a safe .div function was to combine OpenZeppelins canonical div function combined with Uniswap’s existing math operations. We did not modify Uniswap’s existing math functions to try to minimize what we would change.

In the future, we will provide more rigorous testing, at the time of writing, we did a lot of testing on testnets but will work on automating more of the testing process in the future for certain fee changes.

Finally, as we write more of our own code and deal less with forks, we will also improve our code style and formatting, and make sure that all of our code is as readable as possible for everyone to take a look.

We really appreciate Attic Lab’s professional report & guidance throughout the process to help us make our contracts as safe as can be, and hope to work with them in the future with other upcoming contracts.

Impossible Finance

StableXSwap Completes Smart Contract Audit…