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

feat(cardano-chain-follower): Implementation #43

Merged
merged 24 commits into from
Jan 23, 2024

Conversation

FelipeRosa
Copy link
Contributor

@FelipeRosa FelipeRosa commented Dec 15, 2023

This PR adds an implementation for cardano-chain-follower crate (defined in #33).

  1. Reader can be used to read blocks from the chain.
  2. Follower should be used to follow chain updates.

@FelipeRosa FelipeRosa self-assigned this Dec 15, 2023
@FelipeRosa FelipeRosa changed the title cardano chain follower impl feat(cardano-chain-follower): Implementation Dec 15, 2023
@FelipeRosa FelipeRosa force-pushed the cardano-chain-follower-api-design branch 2 times, most recently from ce58da3 to db18905 Compare December 15, 2023 21:07
@FelipeRosa FelipeRosa force-pushed the cardano-chain-follower-api-design branch from db18905 to 1f5233a Compare December 18, 2023 13:26
@FelipeRosa FelipeRosa marked this pull request as ready for review December 20, 2023 14:51
Mr-Leshiy
Mr-Leshiy previously approved these changes Dec 21, 2023
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy left a comment

Choose a reason for hiding this comment

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

LGTM

@FelipeRosa FelipeRosa force-pushed the cardano-chain-follower-api-design branch from 29ccdb9 to f85cb88 Compare January 3, 2024 13:14
Base automatically changed from cardano-chain-follower-api-design to main January 4, 2024 07:29
@stevenj stevenj dismissed Mr-Leshiy’s stale review January 4, 2024 07:29

The base branch was changed.

stevenj
stevenj previously approved these changes Jan 4, 2024
Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

LGTM but need to correct conflicts now that earlier PR has merged.

Mr-Leshiy
Mr-Leshiy previously approved these changes Jan 5, 2024
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

Should remove the Cargo.lock
This is also waiting on integration with the Mithril snapshot data, correct?

* feat(cardano-chain-follower): Split reader and follower and reader now supports reading single block and block ranges from Mithril snapshots

* feat(cardano-chain-follower): Follower now uses request-response to communicate with its background task

* feat(cardano-chain-follower): Follower reads blocks from Mithril
snapshot (if configured)

* chore(cardano-chain-follower): Cleanup traces and add missing docs

* chore(cardano-chain-follower): Fix linter issues

* chore(cardano-chain-follower): Fix formatting

* chore(cardano-chain-follower): Fix spelling and cspell config

* chore(cardano-chain-follower): Use pallas_hardano as a separate crate to avoid license issues

* chore: Add cleanup comment to Cargo.toml

* chore: Update CI files
Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

LGTM

@stevenj stevenj dismissed minikin’s stale review January 23, 2024 07:19

For follow up PR

@stevenj stevenj merged commit b027909 into main Jan 23, 2024
19 checks passed
@stevenj stevenj deleted the cardano-chain-follower-impl branch January 23, 2024 07:20
This pull request was closed.
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.

4 participants