Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

uses Option<u64> for SlotMeta.last_index#21775

Merged
behzadnouri merged 1 commit intosolana-labs:masterfrom
behzadnouri:slot-meta-last-index
Dec 11, 2021
Merged

uses Option<u64> for SlotMeta.last_index#21775
behzadnouri merged 1 commit intosolana-labs:masterfrom
behzadnouri:slot-meta-last-index

Conversation

@behzadnouri
Copy link
Copy Markdown
Contributor

Problem

SlotMeta.last_index may be unknown and current code is using u64::MAX
to indicate that:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L169-L174

This lacks type-safety and can introduce bugs if not always checked for
Several instances of slot_meta.last_index + 1 are also subject to
overflow.

Summary of Changes

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

jbiseda
jbiseda previously approved these changes Dec 10, 2021
Copy link
Copy Markdown
Contributor

@jbiseda jbiseda left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 10, 2021

Codecov Report

Merging #21775 (b44fa42) into master (0224a8b) will decrease coverage by 0.1%.
The diff coverage is 82.9%.

@@            Coverage Diff            @@
##           master   #21775     +/-   ##
=========================================
- Coverage    81.6%    81.4%   -0.2%     
=========================================
  Files         511      511             
  Lines      143320   143590    +270     
=========================================
- Hits       116976   116929     -47     
- Misses      26344    26661    +317     

SlotMeta.last_index may be unknown and current code is using u64::MAX to
indicate that:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L169-L174

This lacks type-safety and can introduce bugs if not always checked for
Several instances of slot_meta.last_index + 1 are also subject to
overflow.

This commit updates the type to Option<u64>. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.
@mergify mergify Bot dismissed jbiseda’s stale review December 10, 2021 20:10

Pull request has been modified.

@behzadnouri behzadnouri merged commit e08139f into solana-labs:master Dec 11, 2021
@behzadnouri behzadnouri deleted the slot-meta-last-index branch December 11, 2021 14:47
mergify Bot pushed a commit that referenced this pull request Dec 11, 2021
SlotMeta.last_index may be unknown and current code is using u64::MAX to
indicate that:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L169-L174

This lacks type-safety and can introduce bugs if not always checked for
Several instances of slot_meta.last_index + 1 are also subject to
overflow.

This commit updates the type to Option<u64>. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.

(cherry picked from commit e08139f)
mergify Bot added a commit that referenced this pull request Dec 11, 2021
SlotMeta.last_index may be unknown and current code is using u64::MAX to
indicate that:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L169-L174

This lacks type-safety and can introduce bugs if not always checked for
Several instances of slot_meta.last_index + 1 are also subject to
overflow.

This commit updates the type to Option<u64>. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.

(cherry picked from commit e08139f)

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 3, 2022
SlotMeta.last_index may be unknown and current code is using u64::MAX to
indicate that:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L169-L174

This lacks type-safety and can introduce bugs if not always checked for
Several instances of slot_meta.last_index + 1 are also subject to
overflow.

This commit updates the type to Option<u64>. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.

(cherry picked from commit e08139f)

# Conflicts:
#	core/src/repair_generic_traversal.rs
#	ledger/src/blockstore.rs
#	ledger/src/blockstore_meta.rs
mergify Bot added a commit that referenced this pull request Feb 3, 2022
* uses Option<u64> for SlotMeta.last_index (#21775)

SlotMeta.last_index may be unknown and current code is using u64::MAX to
indicate that:
https://github.com/solana-labs/solana/blob/6c108c8fc/ledger/src/blockstore_meta.rs#L169-L174

This lacks type-safety and can introduce bugs if not always checked for
Several instances of slot_meta.last_index + 1 are also subject to
overflow.

This commit updates the type to Option<u64>. Backward compatibility is
maintained by customizing serde serialize/deserialize implementations.

(cherry picked from commit e08139f)

# Conflicts:
#	core/src/repair_generic_traversal.rs
#	ledger/src/blockstore.rs
#	ledger/src/blockstore_meta.rs

* removes mergify merge conflicts

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants