[contractkit] SortedOraclesWrapper + tests #1405
Conversation
…onfig value defined
Codecov Report
@@ Coverage Diff @@
## master #1405 +/- ##
==========================================
+ Coverage 65.87% 67.66% +1.78%
==========================================
Files 271 269 -2
Lines 8061 8043 -18
Branches 490 558 +68
==========================================
+ Hits 5310 5442 +132
+ Misses 2639 2488 -151
- Partials 112 113 +1
Continue to review full report at Codecov.
|
| numerator: number, | ||
| denominator: number, |
There was a problem hiding this comment.
@mcortesi numerator and denominator aren't very intuitive, as you pointed out in getRates. Do you think it's worth changing these as well? I can see leaving them as-is to match the contract signature. But I could also see changing them to something like amountToken, amountGold. What do you think?
There was a problem hiding this comment.
There's an issue about changing fraction to fixidity. #1365 Which is something to be done in a later PR.
| async numRates(token: CeloToken): Promise<BigNumber> { | ||
| const tokenAddress = await this.kit.registry.addressFor(token) | ||
| const response = await this.contract.methods.numRates(tokenAddress).call() | ||
| return toBigNumber(response) |
There was a problem hiding this comment.
probably we don't need a big number here, a plain js number would suffice
| const sortedOracles: SortedOraclesInstance = await getDeployedProxiedContract< | ||
| SortedOraclesInstance | ||
| >('SortedOracles', artifacts) | ||
|
|
||
| await sortedOracles.addOracle(stableToken.address, minerAddress) | ||
| for (const oracle of config.stableToken.priceOracleAccounts) { | ||
| console.log(`Adding ${oracle} as an Oracle for StableToken`) |
There was a problem hiding this comment.
Small nit: it's generally preferred to choose a log level when logging. i.e. console.debug or console.info instead of console.log
I guess we don't have lint checks here or it would have caught that
There was a problem hiding this comment.
Every migration file that has logging uses console.log. I'm happy to change all instances of those, but it makes me wonder if there's a reason for it.
* master: [Wallet] Wallet can switch between hosted and local node (#1419) [Wallet] Prevent error from Avatar when name is missing (#1454) [Wallet] Show splash screen until JS is ready on iOS (#1453) Use new segment api keys used by both iOS and Android (#1452) [Wallet] Don't log all props, which includes i18n (#1445) [Helm] Updated the helm package to deploy the upgraded blockscout version (#1129) Tiny copy change (#1429) [contractkit] SortedOraclesWrapper + tests (#1405) [wallet] Refactor leftover thunk to sagas (#1388) [Wallet] Fix repeated QR code scanning and related navigation issues (#1439) [Wallet] Show the currency values with correct rounding. (#1435) [Wallet] Fix firebase initialization error on iOS after reinstalling the app (#1423) [Wallet] Use exit on iOS since we can't restart like Android (#1424) [Wallet] Update local currency styles and layout (#1325) Reset pincode cache if unlock fails (#1430)
Description
This adds a wrapper for the SortedOracles contract. In some cases it derives arguments that the caller shouldn't have to figure out, but are required by the contract. It also includes helper functions for StableToken.
The work here was started in the still-WIP #1394 as some foundational pieces. For ease of reviewing, I've broken this out.
Tested
Tests are included in this PR.
Other changes
In order to make the tests run and to make it possible for CLabs to run a single oracle to report on the cUSD/cGLD exchange rate, I modified the StableToken migration to change how oracles are added before ownership is transferred to Governance. Rather than taking the address of the first miner, read the accounts from migrationsConfig and allow it to be overridden with migration overrides. For now, it's just an empty array except during contractkit tests. In #1394, a new account type will be added.
Related issues