Allow rpc to query final result #1890
Conversation
bowenwang1996
requested review from
evgenykuzyakov,
MaksymZavershynskyi and
SkidanovAlex
as code owners
Codecov Report
@@ Coverage Diff @@
## staging #1890 +/- ##
===========================================
- Coverage 87.03% 86.99% -0.04%
===========================================
Files 166 166
Lines 31506 31491 -15
===========================================
- Hits 27422 27397 -25
- Misses 4084 4094 +10
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
If we make turn this flag on by the default on nearlib side lots of stuff is going to timeout. CC @vgrichina @frol .
| @@ -248,7 +248,7 @@ impl JsonRpcHandler { | |||
| } | |||
|
|
|||
| async fn query(&self, params: Option<Value>) -> Result<Value, RpcError> { | |||
| let (path, data) = parse_params::<(String, String)>(params)?; | |||
| let (path, data, is_final) = parse_params::<(String, String, bool)>(params)?; | |||
There was a problem hiding this comment.
This is a breaking change to the API, right? This API is going out of our hands and introducing it this way we just keep breaking things further. Besides, booleans are rarely a good idea for APIs as we ask people to send something that is e.g. ["account/frol.near", "", false] (meh...).
There was a problem hiding this comment.
What do you suggest we do instead? My understanding is that the raw API is rarely used directly and people will most likely use it through nearlib, which means we can make it backward compatible by providing a default value.
There was a problem hiding this comment.
nearlib is a JS library, and I assume some may rely on the raw API as they simply implement it in another language.
I suggest creating a separate endpoint (e.g. query-finalized / query-final / ...) and internally, we can reuse the query method by adding a function argument to it.
| } | ||
|
|
||
| impl Query { | ||
| pub fn new(path: String, data: Vec<u8>) -> Self { | ||
| Query { path, data, id: generate_random_string(10) } | ||
| pub fn new(path: String, data: Vec<u8>, is_final: bool) -> Self { |
There was a problem hiding this comment.
Let's use enums instead of booleans:
There was a problem hiding this comment.
I don't know it it could be applied as a general rule, but it makes sense to use enum in this case
|
@nearmax This exact change will not lead to timeouts right away as it will break the APIs first, and only when this is fixed, this may lead to timeouts if a requesting code calls this endpoint with |
|
I will finish this up in the upcoming days. |
|
Superseded by #2127 |
For query rpc allow specifying whether to query from finalized blocks only. Need to change nearlib correspondingly.