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

change(state): Move format upgrades to separate modules #7377

Closed
wants to merge 1 commit into from

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Aug 24, 2023

Scheduling

This change is blocked by #7392.

Motivation

This PR moves the tree deduplication into a separate module ahead of adding more upgrades and state validity checks.

Depends On: #7379.

Solution

  • Moves upgrade to separate file

Review

Anyone can review.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

@arya2 arya2 added C-cleanup Category: This is a cleanup P-Medium ⚡ A-state Area: State / database changes labels Aug 24, 2023
@arya2 arya2 requested a review from a team as a code owner August 24, 2023 03:22
@arya2 arya2 self-assigned this Aug 24, 2023
@arya2 arya2 requested review from upbqdn and removed request for a team August 24, 2023 03:22
@arya2 arya2 changed the title moves format upgrades to seperate files change(state): Move format upgrades to separate modules Aug 24, 2023
@arya2 arya2 added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Aug 24, 2023
@arya2 arya2 closed this Aug 24, 2023
@arya2 arya2 reopened this Aug 28, 2023
@arya2 arya2 force-pushed the refactor-disk-format-upgrades branch from cdee342 to 9cd6fc5 Compare August 28, 2023 20:42
@arya2 arya2 changed the base branch from main to fix-tree-dedup August 28, 2023 20:43
@arya2 arya2 marked this pull request as draft August 28, 2023 22:01
@teor2345
Copy link
Collaborator

Just noting that this code movement PR will conflict with PR #7392, which is a low priority security fix.

Refactors are usually lower priority than fixes, so let's mark this as optional?
(Neither of them are on the critical path to getting RPCs working.)

@arya2
Copy link
Contributor Author

arya2 commented Aug 28, 2023

Just noting that this code movement PR will conflict with PR #7392, which is a low priority security fix.

I think the changes for adding subtrees from existing blocks will conflict with that anyway?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-state Area: State / database changes C-cleanup Category: This is a cleanup C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants