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

refactor: improve key prefix constants and documentation #1570

Open
wants to merge 1 commit into
base: v0.34.x-celestia
Choose a base branch
from

Conversation

tiendn
Copy link
Contributor

@tiendn tiendn commented Dec 20, 2024

Description

Add constants for key prefixes used in block store with clear documentation
of their purpose and format. This change:

  • Define key prefix constants with clear naming and comments
  • Document key format patterns in function comments
  • Use consistent string formatting (%d for integers, %x for hashes)
  • Group related constants together
  • Replace magic strings with named constants

This improves maintainability and reduces the chance of key collisions
in the database. The change is backwards compatible as the actual key
formats remain unchanged.


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use
    unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments

@tiendn tiendn requested a review from a team as a code owner December 20, 2024 10:15
@tiendn tiendn requested review from rootulp and evan-forbes and removed request for a team December 20, 2024 10:15
Copy link
Collaborator

@rootulp rootulp left a comment

Choose a reason for hiding this comment

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

Overall the PR seems clear but the change is probably better targeted at CometBFT than this repo.

Comment on lines +497 to +507
// Key prefixes for different types of data
const (
// Key prefixes
blockMetaPrefix = "H" // Height -> BlockMeta
blockPartPrefix = "P" // Height + index -> Block Part
blockCommitPrefix = "C" // Height -> Commit
seenCommitPrefix = "SC" // Height -> Seen Commit
blockHashPrefix = "BH" // Hash -> Height
txHashPrefix = "TH" // Hash -> TxInfo
stateKey = "blockStore" // Fixed key for block store state
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

[question] what motivated this change? Can it be upstreamed to CometBFT so that we can pull it here? None of this seems specific to Celestia.

Ref: https://github.com/celestiaorg/celestia-core?tab=readme-ov-file#contributing

@rootulp rootulp changed the title fix: improve key prefix constants and documentation refactor: improve key prefix constants and documentation Dec 20, 2024
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.

2 participants