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

Clean up oracle spec declaration on market #5714

Closed
ValentinTrinque opened this issue May 26, 2022 · 7 comments · Fixed by #5980
Closed

Clean up oracle spec declaration on market #5714

ValentinTrinque opened this issue May 26, 2022 · 7 comments · Fixed by #5980

Comments

@ValentinTrinque
Copy link
Contributor

ValentinTrinque commented May 26, 2022

Today, we can only specify an Oracle as data source. The introduction of internal data source was a hack based on the existing Oracle definition, for experiment purpose.

It's time to get this clean up.

Before

The payload used for Oracle data source declaration was:

// An oracle spec describe the oracle data that a product (or a risk model)
// wants to get from the oracle engine.
message OracleSpecConfiguration {
  // pubKeys is the list of authorized public keys that signed the data for this
  // oracle. All the public keys in the oracle data should be contained in these
  // public keys.
  repeated string pub_keys = 1;
  // filters describes which oracle data are considered of interest or not for
  // the product (or the risk model).
  repeated Filter filters = 2;
}


// Filter describes the conditions under which an oracle data is considered of
// interest or not.
message Filter {
  // key is the oracle data property key targeted by the filter.
  PropertyKey key = 1;
  // conditions are the conditions that should be matched by the data to be
  // considered of interest.
  repeated Condition conditions = 2;
}

// PropertyKey describes the property key contained in an oracle data.
message PropertyKey {
  // name is the name of the property.
  string name = 1;
  // type is the type of the property.
  Type type = 2;
  // Type describes the type of properties that are supported by the oracle
  // engine.
  enum Type {
    // The default value.
    TYPE_UNSPECIFIED = 0;
    // Any type.
    TYPE_EMPTY = 1;
    // Integer type.
    TYPE_INTEGER = 2;
    // String type.
    TYPE_STRING = 3;
    // Boolean type.
    TYPE_BOOLEAN = 4;
    // Any floating point decimal type.
    TYPE_DECIMAL = 5;
    // Timestamp date type.
    TYPE_TIMESTAMP = 6;
  }
}

// Condition describes the condition that must be validated by the
message Condition {
  // comparator is the type of comparison to make on the value.
  Operator operator = 1;
  // value is used by the comparator.
  string value = 2;
  // Comparator describes the type of comparison.
  enum Operator {
    // The default value
    OPERATOR_UNSPECIFIED = 0;
    // Verify if the property values are strictly equal or not.
    OPERATOR_EQUALS = 1;
    // Verify if the oracle data value is greater than the Condition value.
    OPERATOR_GREATER_THAN = 2;
    // Verify if the oracle data value is greater than or equal to the Condition
    // value.
    OPERATOR_GREATER_THAN_OR_EQUAL = 3;
    // Verify if the oracle data value is less than the Condition value.
    OPERATOR_LESS_THAN = 4;
    // Verify if the oracle data value is less or equal to than the Condition
    // value.
    OPERATOR_LESS_THAN_OR_EQUAL = 5;
  }
}

To specify an internal data source we reused this payload with a few tweaks:

  • skipping the pubKeys field as it was not applicable
  • using a specific and reserved property key in the filters to understand we should use the internal data source and not the oracle one.
NewMarket {
  OracleSpecForTradingTermination {
    PubKeys [ ... ]
    Filters [
       {
          Key: { Name: "vegaprotocol.builtin.timestamp", Type: TIMESTAMP }
          Conditions: [ { Value: 123456789 Operator: GREATER_THAN_OR_EQUAL } ]
       }
    ]
  }
  OracleBindings {
     "tradingTermination": "vegaprotocol.builtin.timestamp",
  }
}

After

// DataSource specifies from where the data we are listening to should come from.
message DataSourceDefinition {
  // Type of the data source to use.
  oneof type {
    InternalDataSourceDefinition internal = 1;
    OracleDataSourceDefinition oracle = 2;
  }
}

message InternalDataSourceDefinition {
  // Type of the internal data source to use.
  oneof type {
    InternalTimeDataSource time = 1;
  }
}

// InternalTimeDataSource allow to subscribe to the internal data source emitting timestamps.
// The name of the property will be normalized (ex: `internal.timestamp`) for the binding.
message InternalTimeDataSourceDefinition {
  // conditions are the conditions that should be matched by the emitted timestamps to be
  // considered of interest.
  repeated Condition conditions = 1;
}

// Renamed from OracleSpecConfiguration
message OracleDataSourceDefinition {
  // pubKeys is the list of authorized public keys that signed the data for this
  // oracle. All the public keys in the oracle data should be contained in these
  // public keys.
  repeated string pub_keys = 1;
  // filters describes which oracle data are considered of interest or not for
  // the product (or the risk model).
  repeated Filter filters = 2;
}


// SAME AS BEFORE !

// Filter describes the conditions under which an oracle data is considered of
// interest or not.
message Filter {
  // key is the oracle data property key targeted by the filter.
  PropertyKey key = 1;
  // conditions are the conditions that should be matched by the data to be
  // considered of interest.
  repeated Condition conditions = 2;
}

// PropertyKey describes the property key contained in an oracle data.
message PropertyKey {
  // name is the name of the property.
  string name = 1;
  // type is the type of the property.
  Type type = 2;
  // Type describes the type of properties that are supported by the oracle
  // engine.
  enum Type {
    // The default value.
    TYPE_UNSPECIFIED = 0;
    // Any type.
    TYPE_EMPTY = 1;
    // Integer type.
    TYPE_INTEGER = 2;
    // String type.
    TYPE_STRING = 3;
    // Boolean type.
    TYPE_BOOLEAN = 4;
    // Any floating point decimal type.
    TYPE_DECIMAL = 5;
    // Timestamp date type.
    TYPE_TIMESTAMP = 6;
  }
}

// Condition describes the condition that must be validated by the
message Condition {
  // comparator is the type of comparison to make on the value.
  Operator operator = 1;
  // value is used by the comparator.
  string value = 2;
  // Comparator describes the type of comparison.
  enum Operator {
    // The default value
    OPERATOR_UNSPECIFIED = 0;
    // Verify if the property values are strictly equal or not.
    OPERATOR_EQUALS = 1;
    // Verify if the oracle data value is greater than the Condition value.
    OPERATOR_GREATER_THAN = 2;
    // Verify if the oracle data value is greater than or equal to the Condition
    // value.
    OPERATOR_GREATER_THAN_OR_EQUAL = 3;
    // Verify if the oracle data value is less than the Condition value.
    OPERATOR_LESS_THAN = 4;
    // Verify if the oracle data value is less or equal to than the Condition
    // value.
    OPERATOR_LESS_THAN_OR_EQUAL = 5;
  }
}

That would result in:

NewMarket {
  DataSourceForTradingTermination {
    Internal {
      Time {
          Conditions: [ { Value: 123456789 Operator: GREATER_THAN_OR_EQUAL } ]
      }
    }
  }
  DataSourceBinding {
     TradingTermination {
        // TO BE DEFINED
     }
  }
}
@davidsiska-vega
Copy link
Contributor

  1. Could we dispense with (i.e. remove) the word "Oracle"? As far as we care everything is just a data source...
message InternalTimeDataSourceDefinition {
  // conditions are the conditions that should be matched by the emitted timestamps to be
  // considered of interest.
  repeated Condition conditions = 1;
}

What would the relation between conditions here and filter in the OracleDataSourceDefinition be? Same syntax and functionality?

@ValentinTrinque
Copy link
Contributor Author

  1. I think Oracle is just the name of a give type of Data Source. Later we might have Blockchain Bridge for example. Having just Data Source would require us to create a generic declaration of data source which might be harder to apprehend and use for the user, and tricky for the implementation.

  2. I will add a complete example

@ValentinTrinque
Copy link
Contributor Author

@davidsiska-vega Alright, updated

@barnabee
Copy link
Member

barnabee commented May 26, 2022

So, it would make more sense to me based on the design and how I think about things if this looked like (please excuse anything that isn't valid protobufs…):

message DataSourceDefinition {
  oneof type {
    InternalDataSourceDefinition internal = 1;
    OracleDataSourceDefinition oracle = 2;
    FilterDataSourceDefinition filter = 3;
  }
}

message InternalNetworkDataSourceDefinition {
  enum Type {
    BLOCK_TIMESTAMP = 1;
    // later, others
  }
  Type type;
}

message SignedMessageOracleDataSourceDefinition {
  repeated string pub_keys = 1;
}

message FilterDataSourceDefinition {
  DataSourceDefinition input = 1;
  repeated Filter conditions = 2;
}

So that the example you gave would be:

NewMarket {
  DataSourceForTradingTermination {
    Filter {
      input: InternalNetworkData { type: BLOCK_TIMESTAMP }
      conditions: [ { operator: GREATER_THAN_OR_EQUAL, value: 123456789 } ] 
    }
  }
}

But I appreciate that this may not be compatible with the current implementation due to the fact it would allow nesting of filters (though perhaps we could just error in validation and say nesting filters not [yet] supported…), so if something more like the above is too much hassle for now, I think the proposal above works.

I am also not quite clear what the binding thing does exactly… 

@jeremyletang
Copy link
Member

So, it would make more sense to me based on the design and how I think about things if this looked like (please excuse anything that isn't valid protobufs…):

message DataSourceDefinition {
  oneof type {
    InternalDataSourceDefinition internal = 1;
    OracleDataSourceDefinition oracle = 2;
    FilterDataSourceDefinition filter = 3;
  }
}

message InternalNetworkDataSourceDefinition {
  enum Type {
    BLOCK_TIMESTAMP = 1;
    // later, others
  }
  Type type;
}

message SignedMessageOracleDataSourceDefinition {
  repeated string pub_keys = 1;
}

message FilterDataSourceDefinition {
  DataSourceDefinition input = 1;
  repeated Filter conditions = 2;
}

So that the example you gave would be:

NewMarket {
  DataSourceForTradingTermination {
    Filter {
      input: InternalNetworkData { type: BLOCK_TIMESTAMP }
      conditions: [ { operator: GREATER_THAN_OR_EQUAL, value: 123456789 } ] 
    }
  }
}

But I appreciate that this may not be compatible with the current implementation due to the fact it would allow nesting of filters (though perhaps we could just error in validation and say nesting filters not [yet] supported…), so if something more like the above is too much hassle for now, I think the proposal above works.

I am also not quite clear what the binding thing does exactly…

There's one issue here, which would be that the Filter type require the key name on which we want to filter. That works with the oracle data source, but not the internal one then as that field would need to be empty, and it goes against why you wanted to change so we don't have non needed fields where we don't need them?

@gordsport
Copy link
Contributor

gordsport commented Jun 13, 2022

Blocked:
Requires an agreement on what we will do and also needs to be reflected in the spec:

@gordsport gordsport removed the blocked label Jul 11, 2022
@gordsport gordsport transferred this issue from vegaprotocol/protos Aug 1, 2022
@gordsport
Copy link
Contributor

Originally in protos:

this PR can be moved over to vega repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
6 participants