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

Modify tip cardano-cli query to also return the current epoch #2440

Merged
merged 2 commits into from
Mar 9, 2021

Conversation

Jimbo4350
Copy link
Contributor

@Jimbo4350 Jimbo4350 commented Mar 3, 2021

Closes #1737

> cabal exec cardano-cli -- query tip --shelley-era --shelley-mode --testnet-magic 42
{
    "epoch": 0,
    "hash": "19fdfc54e7a1e2fb7ff45254a6aa0aa88ea7a80de4b48297924c9abf353f9cb7",
    "slot": 42,
    "block": 3
}

cardano-api/src/Cardano/Api/Block.hs Outdated Show resolved Hide resolved
Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

I think this will be better to do entirely in the CLI in the short term. I don't think we want to change the getLocalChainTip, since it changes it from being a single chain sync query to being two protocols, with a possibility of failure. It doesn't make for a nice API.

In the CLI we can use the existing getLocalChainTip, and then use the existing queryNodeLocalState with the QueryEpoch query. That would not need any API changes.

Now it's true that this is imperfect, since it needs two queries that are not guaranteed to be synchronised (which is the root of the "Maybe" problem in the API in the current version of the patch).

What would be better here, in the medium term, would be to properly make use of the ability of the query to supply no point and implicitly acquire the current point. Then we can use QueryChainPoint and QueryEpoch. The only thing we're missing to make this perfect is QueryChainBlockNo. If we add that query to the consensus, and expose it in the API, then we can do this all as a single query (or rather a single atomic batch of queries).

@Jimbo4350 Jimbo4350 force-pushed the jordan/add-epoch-to-get-tip branch 2 times, most recently from da4b3ea to 659dee5 Compare March 5, 2021 10:41
Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

This looks much better.

Two minor comments below.

Lets squash down to one patch again, and clean up the unnecessary changes in whitespace and comments (that are just there as a result of us going back and forth on things).

cardano-cli/src/Cardano/CLI/Shelley/Run/Query.hs Outdated Show resolved Hide resolved
cardano-cli/src/Cardano/CLI/Shelley/Run/Query.hs Outdated Show resolved Hide resolved
@Jimbo4350 Jimbo4350 force-pushed the jordan/add-epoch-to-get-tip branch from 659dee5 to ecb418e Compare March 8, 2021 11:42
@Jimbo4350 Jimbo4350 force-pushed the jordan/add-epoch-to-get-tip branch 2 times, most recently from 7aff1e1 to 2237ff3 Compare March 8, 2021 11:46
@Jimbo4350
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 8, 2021
2440: Modify tip cardano-cli query to also return the current epoch r=Jimbo4350 a=Jimbo4350

```
> cabal exec cardano-cli -- query tip --shelley-era --shelley-mode --testnet-magic 42
{
    "epoch": 0,
    "hash": "19fdfc54e7a1e2fb7ff45254a6aa0aa88ea7a80de4b48297924c9abf353f9cb7",
    "slot": 42,
    "block": 3
}
```

Co-authored-by: Jordan Millar <jordan.millar@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 8, 2021

Build failed:

@Jimbo4350
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 8, 2021
2440: Modify tip cardano-cli query to also return the current epoch r=Jimbo4350 a=Jimbo4350

Closes #1737 
```
> cabal exec cardano-cli -- query tip --shelley-era --shelley-mode --testnet-magic 42
{
    "epoch": 0,
    "hash": "19fdfc54e7a1e2fb7ff45254a6aa0aa88ea7a80de4b48297924c9abf353f9cb7",
    "slot": 42,
    "block": 3
}
```

Co-authored-by: Jordan Millar <jordan.millar@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 8, 2021

Build failed:

@Jimbo4350 Jimbo4350 force-pushed the jordan/add-epoch-to-get-tip branch 2 times, most recently from a3dec68 to 3aebb51 Compare March 9, 2021 08:46
@Jimbo4350 Jimbo4350 force-pushed the jordan/add-epoch-to-get-tip branch from 3aebb51 to 627f97a Compare March 9, 2021 09:20
@Jimbo4350
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 9, 2021
2440: Modify tip cardano-cli query to also return the current epoch r=Jimbo4350 a=Jimbo4350

Closes #1737 
```
> cabal exec cardano-cli -- query tip --shelley-era --shelley-mode --testnet-magic 42
{
    "epoch": 0,
    "hash": "19fdfc54e7a1e2fb7ff45254a6aa0aa88ea7a80de4b48297924c9abf353f9cb7",
    "slot": 42,
    "block": 3
}
```

Co-authored-by: Jordan Millar <jordan.millar@iohk.io>
@Jimbo4350
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 9, 2021
2440: Modify tip cardano-cli query to also return the current epoch r=Jimbo4350 a=Jimbo4350

Closes #1737 
```
> cabal exec cardano-cli -- query tip --shelley-era --shelley-mode --testnet-magic 42
{
    "epoch": 0,
    "hash": "19fdfc54e7a1e2fb7ff45254a6aa0aa88ea7a80de4b48297924c9abf353f9cb7",
    "slot": 42,
    "block": 3
}
```

Co-authored-by: Jordan Millar <jordan.millar@iohk.io>
@Jimbo4350
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 9, 2021

@iohk-bors iohk-bors bot merged commit c985cc4 into master Mar 9, 2021
@iohk-bors iohk-bors bot deleted the jordan/add-epoch-to-get-tip branch March 9, 2021 12:35
robinboening added a commit to lacepool/cardano-node-docker that referenced this pull request Apr 9, 2021
with cardano-node v1.26.1 the query tip returns a different JSON schema, see IntersectMBO/cardano-node#2440
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.

[FR] - Add epoch and slot in epoch to tip command
3 participants