feat: expose experimental apis #285
Conversation
ghost
marked this pull request as ready for review
|
@frol This is ready to be reviewed |
| where | ||
| T: NetworkClient, | ||
| { | ||
| pub async fn changes_in_block( |
There was a problem hiding this comment.
@ChaoticTempest Why are JSON RPC methods are exposed through "worker"? This looks like an abstraction leak to me.
There was a problem hiding this comment.
@ChaoticTempest kindly pinging you
There was a problem hiding this comment.
hmm, why would this be an abstraction leak? Worker here is just the interface where we want to surface RPC APIs we deem safe to do so, or APIs that are not directly tied to an account/contract that requires signing a transaction with. Worker here also protects against unwanted RPC methods by constraining certain RPC APIs to certain networks like sandbox's fast_forward. Where do you imagine it would live otherwise?
| where | ||
| T: NetworkClient, | ||
| { | ||
| pub async fn changes_in_block( |
There was a problem hiding this comment.
hmm, why would this be an abstraction leak? Worker here is just the interface where we want to surface RPC APIs we deem safe to do so, or APIs that are not directly tied to an account/contract that requires signing a transaction with. Worker here also protects against unwanted RPC methods by constraining certain RPC APIs to certain networks like sandbox's fast_forward. Where do you imagine it would live otherwise?
| let block_reference = { | ||
| let hash = outcome.outcome().block_hash; | ||
| near_primitives::types::BlockReference::BlockId(near_primitives::types::BlockId::Hash( | ||
| near_primitives::hash::CryptoHash(hash.0), | ||
| )) | ||
| }; | ||
|
|
||
| // NOTE: this API is under the "experimental" flag and no guarantees are given. | ||
| let protocol_config = worker.protocol_config(block_reference).await?; |
There was a problem hiding this comment.
Note, this will make the supplying of block_reference inconsistent with: https://github.com/near/workspaces-rs/blob/35151975c165b693ec795841107978ab5e6800b6/examples/src/various_queries.rs#L90
It would be great if these methods were utilized with the following struct https://github.com/near/workspaces-rs/blob/35151975c165b693ec795841107978ab5e6800b6/workspaces/src/rpc/query.rs#L57-L87 so that we can keep things consistent and not have the user do all boilerplate each time
| let receipt_ref = { | ||
| let mut ids = outcome.outcome().receipt_ids.clone(); | ||
| if ids.is_empty() { | ||
| println!("no receipt ids present"); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| println!("receipts found: {ids:?}"); | ||
|
|
||
| ReceiptReference { | ||
| receipt_id: near_primitives::hash::CryptoHash( | ||
| ids.pop().expect("expected at least one receipt id").0, | ||
| ), | ||
| } | ||
| }; | ||
|
|
||
| // NOTE: this API is under the "experimental" flag and no guarantees are given. | ||
| let resp = worker.receipt(receipt_ref).await?; |
There was a problem hiding this comment.
We don't need to expose the fact that we're using ReceiptReference since all it's taking is a CryptoHash. So, we can just have the signature fn receipt(CryptoHash) instead.
| let block_ref = { | ||
| let hash = near_primitives::hash::CryptoHash(outcome.outcome().block_hash.0); | ||
| BlockReference::BlockId(near_primitives::types::BlockId::Hash(hash)) | ||
| }; | ||
|
|
||
| let state_changes = { | ||
| StateChangesRequestView::ContractCodeChanges { | ||
| account_ids: vec![contract.id().clone()], | ||
| } | ||
| }; | ||
|
|
||
| // NOTE: this API is under the "experimental" flag and no guarantees are given. | ||
| let res = worker.changes(block_ref, state_changes).await?; |
There was a problem hiding this comment.
Same with this one and consistency with Query struct. We only need to supply state_changes while further methods can supply block_ref if it's needed with Finality::Final being the default when not supplied
|
I'll put up a patch PR to address the reviews, thank you @ChaoticTempest |
ghost
mentioned this pull request
Closes #217
The PR exposes experimental apis under an
experimentalflag.