Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setup oracle account, Add CLI commands to report oracle values #1394

Closed
wants to merge 67 commits into from

Conversation

yerdua
Copy link
Contributor

@yerdua yerdua commented Oct 18, 2019

Description

This adds a new account type that can have its private key and account address derived from the mnemonic. This price oracle account gets funded in the genesis block to cover the fees of its transactions.

The other component of this PR is two new CLI commands. One of them reports the exchange rate between a specified token (currently just StableToken) and CeloGold. The other gets the currently reported rates. This one I added to easily check if the reporting was working, and figured it might be useful enough to keep.

Tested

The oracle account's inclusion in the genesis block and ability to act as an oracle was tested manually by spinning up a testnet and running the added CLI commands. Report transactions sent from the oracle account were successful and ones sent from other accounts were not.

Other changes

In the code-comments documentation for SortedOracles.report, the denominator param was missing. I added an explanation of what it is.

Related issues

@yerdua yerdua changed the title [WIP] price oracle Setup oracle account, Add CLI commands to report oracle values Nov 1, 2019
@yerdua yerdua requested review from nambrot and asaj November 4, 2019 19:14
const sortedOracles = await this.kit.contracts.getSortedOracles()

const rates = await sortedOracles.getRates(res.args.token)
console.log(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: You might want to use cli.table that we use in other cli commands as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh cool! I didn't know about it until now and it's great!

numerator = numerator.multipliedBy(multiplier)
denominator = denominator.multipliedBy(multiplier)
}
// const numerator = price.multipliedBy(denominator).toNumber()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove

const res = this.parse(ReportPrice)
let token: CeloToken
if (res.flags.token === CeloContract.StableToken) {
token = CeloContract.StableToken
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does token have to be set if res.flags.token is equal to StableToken? Alternatively, you could set the options option on the flag to require token to be member of a set

async run() {
const supportedTokens: CeloToken[] = [CeloContract.StableToken]
const res = this.parse(GetRates)
if (!supportedTokens.includes(res.args.token)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the options option of the argument to ensure that token is in a specific set

@nambrot nambrot assigned yerdua and unassigned nambrot Nov 4, 2019
@timmoreton timmoreton added the Priority: P0 Blocker label Nov 20, 2019
@nambrot nambrot added betanet-blocker and removed Priority: P0 Blocker labels Nov 20, 2019
@yerdua
Copy link
Contributor Author

yerdua commented Nov 21, 2019

I split this up into multiple PRs

@yerdua yerdua closed this Nov 21, 2019
@yerdua yerdua deleted the yerdua/price-oracle-with-brownian-motion branch November 21, 2019 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants