Sip 120 minor issues

[artdgn]

This PR addresses several minor issues in #1127 contract changes:

• Not using safemath for price and amount calculations in ExchangeRatesWithDexPricing
• Allowing setting dex oracle to zero address in SystemSettings
• Not checking dex oracle returned price to at least not be 0 in effectiveAtomicValueAndRates
• Not having comments about the the dex oracle IDexPriceAggregator: impelementation source, refernce interface source.

This PR also fixes a few issues with tests in that PR:

• Tests for exchangeAtomically not being correctly set up to trigger volatility checks (no configurtion for the check which causes the check to be skipped comletely). Which makes the test skip an important part of functionality it's testing, and also makes gas measurement inaccureate.
• Using .sort() for numeric times incorrectly (using alphabetic sort for numbers) - and so setting up the mock correctly only for some test values.
• Setting minutes to 3600 seconds in tests instead of 60 seconds, which makes the test use numbers that are too different from the values used in real world config.

[codecov]

Codecov Report

Merging #1596 (799f909) into sip-120 (aa92214) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           sip-120    #1596   +/-   ##
========================================
  Coverage    95.58%   95.58%           
========================================
  Files           79       79           
  Lines         1879     1882    +3     
  Branches       584      586    +2     
========================================
+ Hits          1796     1799    +3     
  Misses          83       83           

Impacted Files	Coverage Δ
contracts/ExchangeRatesWithDexPricing.sol	100.00% <100.00%> (ø)
contracts/SystemSettings.sol	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa92214...799f909. Read the comment docs.

[jjgonecrypto]

This PR is solid. Thank you. 🙏

[sohkai]

🙏 Thanks, good catches!

[sohkai]

I found this a bit weird, now if you want to turn off atomic exchanges you'll have to set the equivalent to a random address like 0xdead?

[artdgn]

It's worth to add an explicit way to turn it off through system settings if that was the previous usage of the zero-address. I don't think that setting to zero address or 0xdead is a good choice in this case - it relies too much on the knowledge and (hope) of this parameter's usage implementation (that it's checked, and the call / result is checked correctly, etc).

[sohkai]

Fair points. I originally did it this way to avoid adding another system setting to enable/disable which assets are atomically exchangeable since it's unlikely we'll query from a synth-based AMM pool in future.

For clarity, ExchangeRatesWithDexPricing does check the parameter, failing price queries if the mapping doesn't exist, and at the moment the only way to disable a synth from being atomically exchanged once the mapping is set would be to reset it to a random address to fail the price query.