Skip to content

Conversation

@delbonis
Copy link
Contributor

Description

This moves some modules over to a new chaintsn crate from the consensus-logic crate as described in EXP-13. This isn't a complete reorganization as it described because I'm trying not to break too many things all at once.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

  • EXP-13

@delbonis delbonis changed the title Reorganice consensus logic modules Reorganize consensus logic modules Aug 16, 2024
@delbonis
Copy link
Contributor Author

delbonis commented Aug 16, 2024

Is there anything else that it makes sense to move right now? I feel like I should move around some of the modules in state too, splitting apart the state types from the chain types, but that feels like maybe too much? And moving IDs and stuff into the primitives crate.

Copy link
Contributor

@Rajil1213 Rajil1213 left a comment

Choose a reason for hiding this comment

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

Left a few nits.

Is there anything else that it makes sense to move right now? I feel like I should move around some of the modules in state too, splitting apart the state types from the chain types, but that feels like maybe too much? And moving IDs and stuff into the primitives crate.

I haven't worked actively with the CL crate to know which parts should be split but given the title of this PR, the number of changes in it and the corresponding issue description, I think a few more changes should be fine. I'm personally inclined to have these kinds of refactorings be done in one fell swoop and to face the rebasing issues altogether (ripping the bandaid and what-not). But I'm fine with whatever the rest of you decide.

@delbonis delbonis merged commit 543eb0f into master Aug 19, 2024
@Rajil1213
Copy link
Contributor

This one is failing to build as it's missing the trace import in the express-tsn/src/transition.rs. There are a couple of warnings too about unused stuff which we should silence with #![allow_unused] for the sake of clippy.

@storopoli storopoli deleted the EXP-13-reorg-consensus-logic-modules branch November 28, 2024 10:31
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.

3 participants