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

fix(rust/cardano-chain-follower): Refactor chain follower to use cardano-blockchain-types #115

Merged
merged 46 commits into from
Jan 14, 2025

Conversation

bkioshn
Copy link
Contributor

@bkioshn bkioshn commented Dec 19, 2024

Description

Refactor cardano-chain-follower to use cardano-blockchain-types

Related Issue(s)

#121

Description of Changes

  • Use the type in cardano-blockchain-types include - Slot, Network, Txn_Index, Fork etc.
  • Remove unused dependencies
  • Modify the follow_chain.rs
  • Break down stat.rs
  • Remove metadata
  • Remove unused test data
  • Remove utils and use function from catalyst-types
  • Revisit documents and make sure its convey the meaning of the code

Breaking Changes

All crate that originally use cardanno-chain-follower need to beware of

  • Type changes - eg.u64 -> Slot etc.
  • metadata module is now removed

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@bkioshn bkioshn marked this pull request as draft December 19, 2024 03:12
@bkioshn bkioshn changed the title fix(rust/cardano-chain-follower): use Network, MultiEraBlock, Point from c… fix(rust/cardano-chain-follower): Refactor chain follower to use cardano-blockchain-types Dec 19, 2024
@bkioshn bkioshn self-assigned this Dec 19, 2024
rust/cardano-chain-follower/src/metadata/cip36.rs Outdated Show resolved Hide resolved
rust/cardano-chain-follower/src/utils.rs Outdated Show resolved Hide resolved
@bkioshn bkioshn added enhancement New feature or request review me PR is ready for review labels Jan 9, 2025
@stevenj stevenj marked this pull request as ready for review January 10, 2025 04:51
@stevenj stevenj marked this pull request as draft January 10, 2025 05:04
Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 260/260}$ | ${\color{red}Fail: 0/260}$ |

Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 260/260}$ | ${\color{red}Fail: 0/260}$ |

@bkioshn bkioshn requested a review from stevenj January 10, 2025 07:16
@bkioshn bkioshn marked this pull request as ready for review January 10, 2025 07:21
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.

Looks pretty good.
Mostly, I think we need to add some features to the Fork type, and then incorporate those into this code.

rust/cardano-chain-follower/examples/follow_chains.rs Outdated Show resolved Hide resolved
rust/cardano-chain-follower/src/chain_sync.rs Show resolved Hide resolved
rust/cardano-chain-follower/src/chain_sync.rs Outdated Show resolved Hide resolved
rust/cardano-chain-follower/src/chain_sync.rs Outdated Show resolved Hide resolved
rust/cardano-chain-follower/src/chain_sync_live_chains.rs Outdated Show resolved Hide resolved
rust/cardano-chain-follower/src/chain_sync_live_chains.rs Outdated Show resolved Hide resolved
rust/cardano-chain-follower/src/follow.rs Outdated Show resolved Hide resolved
Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 260/260}$ | ${\color{red}Fail: 0/260}$ |

Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 260/260}$ | ${\color{red}Fail: 0/260}$ |

Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 260/260}$ | ${\color{red}Fail: 0/260}$ |

@bkioshn bkioshn requested a review from stevenj January 14, 2025 08:31
Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 260/260}$ | ${\color{red}Fail: 0/260}$ |

@bkioshn bkioshn requested a review from Mr-Leshiy January 14, 2025 12:29
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.

LGTM

@stevenj stevenj merged commit e182213 into main Jan 14, 2025
20 of 22 checks passed
@stevenj stevenj deleted the fix/refactor-chain-follower branch January 14, 2025 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request review me PR is ready for review
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants