refactor: Is Eligible Changes #20
Conversation
…e is that and checks dr_id is not resolved
| #[cfg(test)] | ||
| pub(crate) fn create_query(self, proof: Vec<u8>) -> Query { | ||
| let data = format!("{}:{}:{}", self.public_key, self.dr_id, proof.to_hex()); | ||
|
|
||
| Query { | ||
| data: Self::encode_data(&data), | ||
| } | ||
| } |
There was a problem hiding this comment.
Imo it would be better to not mix test helpers and actual code.
There was a problem hiding this comment.
@gluax It may be nice to sort of expose this let data = format!("{}:{}:{}", self.public_key, self.dr_id, proof.to_hex()); in a function. The overlay node right now has no way to just get the raw data (+ base64 encoded) because it only gets the QueryMsg which i cannot easily extract the data property
There was a problem hiding this comment.
Oh! Maybe I should make the create_message return the Query type instead? You'd have to call .into on it, but it would let you access the parts method as well as the field itself, which has the encoded string.
There was a problem hiding this comment.
I went for changing create_message to return a tuple of the message and the base64.
| IsExecutorEligible { proof: String, dr_id: String }, | ||
| IsExecutorCommitteeEligible { public_key: String }, | ||
| #[cfg_attr(feature = "cosmwasm", returns(bool))] | ||
| IsExecutorEligible(is_executor_eligible::Query), |
There was a problem hiding this comment.
To follow the rest of queries, why not like this?
IsExecutorEligible{ data: is_executor_eligible::Query},
There was a problem hiding this comment.
It could be done that way, but our standard practice for messages is that if something is complicated to move to its type, we've been using tuples for it.
If we did this, it would change the json as well from:
"is_executor_eligible": {
"data": "data_str",
}to
"is_executor_eligible": {
"data": {
"data": "data_str",
}
}
|
We have decided to rename |
Motivation
To have the is eligible check more aligned with the spec/requirements as they change.
Explanation of Changes
Renamed the old
IsExecutorEligibletoIsExecutorCommitteeEligible, and this one only checks if the stake is enough.The new
IsExecutorEligiblestill checks that but also takes a message in the form ofbase64(public_key:dr_id:proof).To assist with that, there is a
QueryFactoryfor that now.Testing
I added a new
JSONtest.As well as checking the decoding test.
Related PRs and Issues
N/A