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

[Feature] Improve oracle spec #325

Merged
merged 24 commits into from
Mar 25, 2020
Merged

[Feature] Improve oracle spec #325

merged 24 commits into from
Mar 25, 2020

Conversation

yun-yeo
Copy link
Contributor

@yun-yeo yun-yeo commented Feb 13, 2020

Summary of changes

  • Append aggregate msgs
  • Change oracle param Whitelist struct to include TobinTax
  • Keep tracking VoteTargets in the Keeper

This will enable the voters to do oracle multiple votes with an aggregate msg which is highly downsized compared with normal prevote/vote.

The param structure change will enable to enter illiquid factor, which determines the swap spread, at whitelist registration step via gov proposal.

The core directly refer params.Whitelist to check oracle slashing at columbus-3. If a new denom is appended to params.Whitelist, then it can cause some validators miss a oracle vote because it comes middle of a oracle vote period. In order to prevent this confusion, the oracle keeper will keep VoteTargets and update it at the end of vote period with params.Whitelist.

See discussion: Agora link

Report of required housekeeping

  • Github issue OR spec proposal link
  • Wrote tests
  • Updated relevant documentation (docs/)
  • Added a relevant changelog entry: clog add [section] [stanza] [message]

(FOR ADMIN) Before merging

  • Added appropriate labels to PR
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)
  • Confirm added tests are consistent with the intended behavior of changes
  • Ensure all tests pass

@yun-yeo yun-yeo requested a review from dokwon February 13, 2020 08:37
@yun-yeo yun-yeo self-assigned this Feb 13, 2020
@yun-yeo yun-yeo added the enhancement New feature or request label Feb 13, 2020
@yun-yeo yun-yeo requested a review from hanjukim February 13, 2020 08:40
@codecov
Copy link

codecov bot commented Feb 13, 2020

Codecov Report

Merging #325 into develop will decrease coverage by 1.85%.
The diff coverage is 63.87%.

@@             Coverage Diff             @@
##           develop     #325      +/-   ##
===========================================
- Coverage    72.87%   71.01%   -1.86%     
===========================================
  Files           73       75       +2     
  Lines         3089     3605     +516     
===========================================
+ Hits          2251     2560     +309     
- Misses         731      899     +168     
- Partials       107      146      +39

x/oracle/client/cli/tx.go Outdated Show resolved Hide resolved
@yun-yeo yun-yeo force-pushed the feature/improve-oracle-spec branch from c05901d to 1e44cc1 Compare March 23, 2020 05:51
@dokwon
Copy link
Contributor

dokwon commented Mar 24, 2020

@YunSuk-Yeo instead of VoteTarget, how do you feel about calling these WhitelistWaitQueue? Seems more appropriate given the data structure stores denominations that are about to be entered into the whitelist. Do we need to take the same precaution for removing denoms from the whitelist? (assuming this is also an option)

Copy link
Contributor

@dokwon dokwon left a comment

Choose a reason for hiding this comment

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

Good work, overall:

  • some tests to be added
  • consider using unique tobin taxes per denom instead of creating illiquidfactor weights
  • some variable name changes suggested

@@ -399,7 +399,7 @@ func testAndRunTxs(app *TerraApp) []simulation.WeightedOperation {
{
func(_ *rand.Rand) int {
var v int
ap.GetOrGenerate(cdc, OpWeightMsgUnjail, &v, nil,
ap.GetOrGenerate(cdc, OpWeightMsgSend, &v, nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: not relevant to oracle improvement, tag along to the PR

client/lcd/swagger-ui/swagger.yaml Outdated Show resolved Hide resolved
client/lcd/swagger-ui/swagger.yaml Outdated Show resolved Hide resolved
contrib/devtools/install-golangci-lint.sh Outdated Show resolved Hide resolved
contrib/devtools/install-golangci-lint.sh Outdated Show resolved Hide resolved
x/market/internal/keeper/swap.go Outdated Show resolved Hide resolved
x/market/internal/types/params.go Outdated Show resolved Hide resolved
x/oracle/internal/types/msgs.go Show resolved Hide resolved
x/oracle/internal/types/vote_test.go Show resolved Hide resolved
Copy link
Contributor

@dokwon dokwon left a comment

Choose a reason for hiding this comment

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

Just a few wording changes!

x/market/internal/keeper/params.go Outdated Show resolved Hide resolved
x/oracle/internal/keeper/vote_target.go Show resolved Hide resolved
x/oracle/internal/types/keys.go Outdated Show resolved Hide resolved
@yun-yeo yun-yeo merged commit 00655e2 into develop Mar 25, 2020
@yun-yeo yun-yeo deleted the feature/improve-oracle-spec branch March 25, 2020 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants