Introduce a `Slot` type #7997

bkchr · 2021-01-27T20:56:12Z

Instead of having some type definition that only was used in half of the
code or directly using u64, this adds a new unit type wrapper Slot.
This makes it especially easy for the outside api to know what type is
expected/returned.

polkadot companion: paritytech/polkadot#2345

Instead of having some type definition that only was used in half of the code or directly using `u64`, this adds a new unit type wrapper `Slot`. This makes it especially easy for the outside api to know what type is expected/returned.

andresilva

I started suggesting all of the naming changes related to slot_number -> slot. But this is cumbersome to do on github, do you mind if I just them locally and push a commit for this?

andresilva · 2021-01-28T13:07:23Z

test-utils/runtime/src/lib.rs


 				fn generate_key_ownership_proof(
-					_slot_number: sp_consensus_babe::SlotNumber,
+					_slot_number: sp_consensus_babe::Slot,


Suggested change

_slot_number: sp_consensus_babe::Slot,

_slot: sp_consensus_babe::Slot,

andresilva · 2021-01-28T13:07:43Z

test-utils/runtime/src/lib.rs


 				fn generate_key_ownership_proof(
-					_slot_number: sp_consensus_babe::SlotNumber,
+					_slot_number: sp_consensus_babe::Slot,


Suggested change

_slot_number: sp_consensus_babe::Slot,

_slot: sp_consensus_babe::Slot,

andresilva · 2021-01-28T13:09:38Z

primitives/consensus/babe/src/lib.rs

+	pub start_slot: Slot,
 	/// The duration of this epoch.
-	pub duration: SlotNumber,
+	pub duration: Slot,


Suggested change

pub duration: Slot,

pub duration: u64,

We define epoch_length above as u64 as well.

andresilva · 2021-01-28T13:10:29Z

primitives/consensus/babe/src/lib.rs

 pub fn make_transcript(
 	randomness: &Randomness,
-	slot_number: u64,
+	slot_number: Slot,


Suggested change

slot_number: Slot,

slot: Slot,

andresilva · 2021-01-28T13:10:37Z

primitives/consensus/babe/src/lib.rs

 ) -> Transcript {
 	let mut transcript = Transcript::new(&BABE_ENGINE_ID);
-	transcript.append_u64(b"slot number", slot_number);
+	transcript.append_u64(b"slot number", *slot_number);


Suggested change

transcript.append_u64(b"slot number", *slot_number);

transcript.append_u64(b"slot number", *slot);

andresilva · 2021-01-28T13:10:57Z

primitives/consensus/babe/src/lib.rs

 		label: &BABE_ENGINE_ID,
 		items: vec![
-			("slot number", VRFTranscriptValue::U64(slot_number)),
+			("slot number", VRFTranscriptValue::U64(*slot_number)),


Suggested change

("slot number", VRFTranscriptValue::U64(*slot_number)),

("slot number", VRFTranscriptValue::U64(*slot)),

andresilva · 2021-01-28T13:13:34Z

client/consensus/aura/src/digests.rs

 	}

-	fn aura_pre_digest(slot_num: u64) -> Self {
+	fn aura_pre_digest(slot_num: Slot) -> Self {


Suggested change

fn aura_pre_digest(slot_num: Slot) -> Self {

fn aura_pre_digest(slot: Slot) -> Self {

andresilva · 2021-01-28T13:13:45Z

client/consensus/aura/src/digests.rs


-	fn aura_pre_digest(slot_num: u64) -> Self {
+	fn aura_pre_digest(slot_num: Slot) -> Self {
 		DigestItem::PreRuntime(AURA_ENGINE_ID, slot_num.encode())


Suggested change

DigestItem::PreRuntime(AURA_ENGINE_ID, slot_num.encode())

DigestItem::PreRuntime(AURA_ENGINE_ID, slot.encode())

andresilva · 2021-01-28T13:13:54Z

client/consensus/aura/src/digests.rs


 	/// Construct a digest item which contains the slot number
-	fn aura_pre_digest(slot_num: u64) -> Self;
+	fn aura_pre_digest(slot_num: Slot) -> Self;


Suggested change

fn aura_pre_digest(slot_num: Slot) -> Self;

fn aura_pre_digest(slot: Slot) -> Self;

client/consensus/babe/src/tests.rs

paritytech/substrate#7997

andresilva

Change LGTM, I pushed a commit with some extra renaming. Will give another extra look before merging to double check tiny changes.

primitives/consensus/slots/src/lib.rs

octol

Looking good! I guess a natural next step could be to also create a type for the slot durations?

andresilva

Thanks!

bkchr · 2021-01-28T19:44:19Z

bot merge

ghost · 2021-01-28T19:44:22Z

Trying merge.

* Companion for #7997 paritytech/substrate#7997 * rename slot_number to slot * rename SlotNumber type in overseer docs * "Update Substrate" Co-authored-by: André Silva <[email protected]> Co-authored-by: parity-processbot <>

Introduce a Slot type

4a44983

Instead of having some type definition that only was used in half of the code or directly using `u64`, this adds a new unit type wrapper `Slot`. This makes it especially easy for the outside api to know what type is expected/returned.

bkchr added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jan 27, 2021

bkchr requested review from andresilva and octol January 27, 2021 20:56

paritytech-ci added the D2-breaksapi label Jan 27, 2021

github-actions bot added the A7-needspolkadotpr label Jan 27, 2021

andresilva suggested changes Jan 28, 2021

View reviewed changes

Change epoch duratioC

cefaffe

bkchr added a commit to paritytech/polkadot that referenced this pull request Jan 28, 2021

Companion for #7997

41bd264

paritytech/substrate#7997

bkchr mentioned this pull request Jan 28, 2021

Companion for #7997 paritytech/polkadot#2345

Merged

rename all instances of slot number to slot

57af1a3

andresilva approved these changes Jan 28, 2021

View reviewed changes

primitives/consensus/slots/src/lib.rs Outdated Show resolved Hide resolved

github-actions bot removed the A7-needspolkadotpr label Jan 28, 2021

octol approved these changes Jan 28, 2021

View reviewed changes

jacogr mentioned this pull request Jan 28, 2021

Add Slot type polkadot-js/api#3120

Merged

Make the constructor private

26611cf

andresilva approved these changes Jan 28, 2021

View reviewed changes

ghost merged commit 64df2f9 into master Jan 28, 2021

ghost deleted the bkchr-slot-type branch January 28, 2021 19:44

This pull request was closed.

	_slot_number: sp_consensus_babe::Slot,
	_slot: sp_consensus_babe::Slot,

	transcript.append_u64(b"slot number", *slot_number);
	transcript.append_u64(b"slot number", *slot);

	("slot number", VRFTranscriptValue::U64(*slot_number)),
	("slot number", VRFTranscriptValue::U64(*slot)),

	fn aura_pre_digest(slot_num: Slot) -> Self {
	fn aura_pre_digest(slot: Slot) -> Self {

	DigestItem::PreRuntime(AURA_ENGINE_ID, slot_num.encode())
	DigestItem::PreRuntime(AURA_ENGINE_ID, slot.encode())

	fn aura_pre_digest(slot_num: Slot) -> Self;
	fn aura_pre_digest(slot: Slot) -> Self;

Introduce a Slot type #7997

Introduce a Slot type #7997

Uh oh!

Conversation

bkchr commented Jan 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andresilva left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andresilva left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

octol left a comment

Choose a reason for hiding this comment

Uh oh!

andresilva left a comment

Choose a reason for hiding this comment

Uh oh!

bkchr commented Jan 28, 2021

Uh oh!

ghost commented Jan 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Introduce a `Slot` type #7997

Introduce a `Slot` type #7997

bkchr commented Jan 27, 2021 •

edited

Loading