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

add GetEpoch local state query #320

Closed
wants to merge 13 commits into from

Conversation

falcucci
Copy link
Contributor

@falcucci falcucci commented Oct 30, 2023

fixes #315

@falcucci falcucci changed the title implement GetEpoch local state query w examples add GetEpoch local state query Oct 30, 2023
@falcucci falcucci changed the title add GetEpoch local state query addGetEpoch local state query Oct 30, 2023
@falcucci falcucci changed the title addGetEpoch local state query add GetEpoch local state query Oct 30, 2023
@falcucci falcucci marked this pull request as ready for review October 30, 2023 16:58
@@ -399,6 +401,150 @@ pub async fn local_state_query_server_and_client_happy_path() {
_ = tokio::join!(client, server);
}

#[tokio::test]
pub async fn local_state_query_server_and_block_query_get_epoch_client_happy_path() {
Copy link
Member

Choose a reason for hiding this comment

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

there's no need to repeat the whole code of the test for each type of query. Put everything in a single happy path test that includes all of the available queries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scarmuega done.

@scarmuega
Copy link
Member

@falcucci we're missing part of the requirements of the issue: "we have primitives to represent the response". What I mean by that is that we need to get rid of the GenericResponse and start using structs that represent the decoded version of the query results.

@falcucci falcucci marked this pull request as draft October 31, 2023 10:47
```diff
diff --git a/pallas-network/tests/protocols.rs b/pallas-network/tests/protocols.rs
index 5812f88838ed..b625632f0f15 100644
--- a/pallas-network/tests/protocols.rs
+++ b/pallas-network/tests/protocols.rs
@@ -290,7 +290,6 @@ pub async fn local_state_query_server_and_client_happy_path() {
                 .unwrap();

             // server receives range from client, sends blocks
-
             let clientacquirerequest(maybe_point) =
                 server_sq.recv_while_idle().await.unwrap().unwrap();

@@ -322,7 +321,6 @@ pub async fn local_state_query_server_and_client_happy_path() {
                 .unwrap();

             // server receives release from the client
-
             match server_sq.recv_while_acquired().await.unwrap() {
                 clientqueryrequest::release => (),
                 x => panic!("unexpected message from client: {x:?}"),
@@ -360,7 +358,6 @@ pub async fn local_state_query_server_and_client_happy_path() {
             assert_eq!(*server_sq.state(), localstate::state::acquired);

             // server receives reaquire from the client
-
             let maybe_point = match server_sq.recv_while_acquired().await.unwrap() {
                 clientqueryrequest::reacquire(p) => p,
                 x => panic!("unexpected message from client: {x:?}"),
@@ -372,7 +369,6 @@ pub async fn local_state_query_server_and_client_happy_path() {
             server_sq.send_acquired().await.unwrap();

             // server receives release from the client
-
             match server_sq.recv_while_acquired().await.unwrap() {
                 clientqueryrequest::release => (),
                 x => panic!("unexpected message from client: {x:?}"),
```
currently supporting only non params local state queries
```diff
diff --git a/pallas-network/src/miniprotocols/localstate/queries.rs b/pallas-network/src/miniprotocols/localstate/queries.rs
index dabc73e646d0..71921c1672c7 100644
--- a/pallas-network/src/miniprotocols/localstate/queries.rs
+++ b/pallas-network/src/miniprotocols/localstate/queries.rs
@@ -42,7 +42,7 @@ impl encode<()> for blockquery {
         e.u16(0)?;
         e.array(2)?;
         /*
-            todo: i think this is era or something? first fetch era with
+            todo: think this is era or something? first fetch era with
             [3, [0, [2, [1]]]], then use it here?
         */
         e.u16(5)?;
```
renames the variants of the `blockqueryresponse` enum in the `queries.rs` file of the `localstate` miniprotocol.

the following changes were made:

- `getledgertip` is now `ledgertip`
- `stakepools` is now `currentpparams`
- `getcurrentpparams` is now `proposedpparamsupdates`
- `getproposedpparamsupdates` is now `stakedistribution`
- `getstakedistribution` is now `genesisconfig`
- `getgenesisconfig` is now `debugchaindepstate`
- `getrewardprovenance` is now `rewardprovenance`
- `getstakepools` is now `stakepools`
- `getrewardinfopools` is now `rewardinfopools`
this has been an attempt to map the server requests into our responses.

tests must be updated.
we added the struct. maybe we should adapt to use it as turbofish
- changed the argument of the `do_localstate_query` function from `localstate::queries::request` to `request`
- updated the function call in `main` to use `request` instead of `localstate::queries::request`
- removed unnecessary impls for `epochno`
@falcucci falcucci marked this pull request as ready for review November 6, 2023 23:42
@scarmuega
Copy link
Member

closing in favor of #326

@scarmuega scarmuega closed this Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: implement GetEpoch local state query
2 participants