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

uses Option<Slot> for SlotMeta.parent_slot #21808

Merged
merged 1 commit into from
Dec 14, 2021

Conversation

behzadnouri
Copy link
Contributor

Problem

SlotMeta.parent_slot for the head of a detached chain of slots is
unknown and that is indicated by u64::MAX which lacks type-safety:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L203-L205

Summary of Changes

The commit changes the type to Option. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.

SlotMeta.parent_slot for the head of a detached chain of slots is
unknown and that is indicated by u64::MAX which lacks type-safety:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L203-L205

The commit changes the type to Option<Slot>. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.
@codecov
Copy link

codecov bot commented Dec 11, 2021

Codecov Report

Merging #21808 (34f5619) into master (eeb97fe) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #21808   +/-   ##
=======================================
  Coverage    81.3%    81.3%           
=======================================
  Files         515      515           
  Lines      144033   144029    -4     
=======================================
+ Hits       117126   117149   +23     
+ Misses      26907    26880   -27     

// TODO use Option<Slot> instead.
pub parent_slot: Slot,
// The parent slot of the head of a detached chain of slots is None.
#[serde(with = "serde_compat")]
Copy link
Contributor

Choose a reason for hiding this comment

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

This backwards compatibility looks good to me. cc @yhchiang-sol and @steviez for future blockstore changes

@carllin
Copy link
Contributor

carllin commented Dec 14, 2021

Looks good! Feel free to tag @yhchiang-sol and @steviez for future blockstore changes. They're working in this area and would like to keep abreast of developments in this area of the code

@behzadnouri behzadnouri merged commit 8d980f0 into solana-labs:master Dec 14, 2021
@behzadnouri behzadnouri deleted the slot-meta-parent-slot branch December 14, 2021 18:57
mergify bot pushed a commit that referenced this pull request Dec 14, 2021
SlotMeta.parent_slot for the head of a detached chain of slots is
unknown and that is indicated by u64::MAX which lacks type-safety:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L203-L205

The commit changes the type to Option<Slot>. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.

(cherry picked from commit 8d980f0)

# Conflicts:
#	ledger-tool/src/main.rs
mergify bot added a commit that referenced this pull request Dec 14, 2021
* uses Option<Slot> for SlotMeta.parent_slot (#21808)

SlotMeta.parent_slot for the head of a detached chain of slots is
unknown and that is indicated by u64::MAX which lacks type-safety:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L203-L205

The commit changes the type to Option<Slot>. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.

(cherry picked from commit 8d980f0)

# Conflicts:
#	ledger-tool/src/main.rs

* removes backport merge conflicts

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
@brooksprumo brooksprumo mentioned this pull request Jan 5, 2022
mergify bot pushed a commit that referenced this pull request Feb 4, 2022
SlotMeta.parent_slot for the head of a detached chain of slots is
unknown and that is indicated by u64::MAX which lacks type-safety:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L203-L205

The commit changes the type to Option<Slot>. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.

(cherry picked from commit 8d980f0)

# Conflicts:
#	core/src/repair_generic_traversal.rs
#	ledger-tool/src/main.rs
#	ledger/src/blockstore.rs
mergify bot added a commit that referenced this pull request Feb 4, 2022
* uses Option<Slot> for SlotMeta.parent_slot (#21808)

SlotMeta.parent_slot for the head of a detached chain of slots is
unknown and that is indicated by u64::MAX which lacks type-safety:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L203-L205

The commit changes the type to Option<Slot>. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.

(cherry picked from commit 8d980f0)

# Conflicts:
#	core/src/repair_generic_traversal.rs
#	ledger-tool/src/main.rs
#	ledger/src/blockstore.rs

* removes mergify merge conflicts

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
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