From e3b875c10769349712325e3f71b827cc099a4489 Mon Sep 17 00:00:00 2001 From: austinabell Date: Tue, 12 May 2020 11:59:22 -0400 Subject: [PATCH 1/3] Update tipset sorting --- blockchain/blocks/src/header.rs | 17 +++++++++++++++++ blockchain/blocks/src/ticket.rs | 2 +- blockchain/blocks/src/tipset.rs | 15 ++++----------- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/blockchain/blocks/src/header.rs b/blockchain/blocks/src/header.rs index 5c51426637f..e21a12595ed 100644 --- a/blockchain/blocks/src/header.rs +++ b/blockchain/blocks/src/header.rs @@ -18,6 +18,7 @@ use num_bigint::{ BigUint, }; use sha2::Digest; +use std::cmp::Ordering; use std::fmt; use std::time::{SystemTime, UNIX_EPOCH}; use vm::PoStProof; @@ -201,6 +202,22 @@ impl<'de> Deserialize<'de> for BlockHeader { } } +impl Ord for BlockHeader { + fn cmp(&self, other: &Self) -> Ordering { + match self.ticket().cmp(other.ticket()) { + // Only compare cid bytes when tickets are equal + Ordering::Equal => self.cid().to_bytes().cmp(&other.cid().to_bytes()), + order => order, + } + } +} + +impl PartialOrd for BlockHeader { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl BlockHeader { /// Generates a BlockHeader builder as a constructor pub fn builder() -> BlockHeaderBuilder { diff --git a/blockchain/blocks/src/ticket.rs b/blockchain/blocks/src/ticket.rs index d7c1837344c..3d473d6e091 100644 --- a/blockchain/blocks/src/ticket.rs +++ b/blockchain/blocks/src/ticket.rs @@ -9,7 +9,7 @@ use vm::PoStProof; /// A Ticket is a marker of a tick of the blockchain's clock. It is the source /// of randomness for proofs of storage and leader election. It is generated /// by the miner of a block using a VRF and a VDF. -#[derive(Clone, Debug, PartialEq, PartialOrd, Eq, Default)] +#[derive(Clone, Debug, PartialEq, PartialOrd, Eq, Default, Ord)] pub struct Ticket { /// A proof output by running a VRF on the VDFResult of the parent ticket pub vrfproof: VRFProof, diff --git a/blockchain/blocks/src/tipset.rs b/blockchain/blocks/src/tipset.rs index 18d39302a71..88f2916509d 100644 --- a/blockchain/blocks/src/tipset.rs +++ b/blockchain/blocks/src/tipset.rs @@ -88,7 +88,7 @@ impl Tipset { /// A valid tipset contains a non-empty collection of blocks that have distinct miners and all /// specify identical epoch, parents, weight, height, state root, receipt root; /// contentID for headers are supposed to be distinct but until encoding is added will be equal. - pub fn new(headers: Vec) -> Result { + pub fn new(mut headers: Vec) -> Result { verify_blocks(&headers)?; // TODO Have a check the ensures CIDs are distinct @@ -96,14 +96,12 @@ impl Tipset { // sort headers by ticket size // break ticket ties with the header CIDs, which are distinct - let mut sorted_headers = headers; - sorted_headers - .sort_by_key(|header| (header.ticket().vrfproof.clone(), header.cid().to_bytes())); + headers.sort(); // return tipset where sorted headers have smallest ticket size in the 0th index // and the distinct keys Ok(Self { - blocks: sorted_headers, + blocks: headers, key: TipsetKeys { // interim until CID check is in place cids, @@ -178,12 +176,7 @@ impl FullTipset { // sort blocks on creation to allow for more seamless conversions between FullTipset // and Tipset - blocks.sort_by_key(|block| { - ( - block.header.ticket().vrfproof.clone(), - block.header.cid().to_bytes(), - ) - }); + blocks.sort_by(|block1, block2| block1.header().cmp(block2.header())); Ok(Self { blocks }) } /// Returns the first block of the tipset From 757d5bd0df884ca0b293bf71b5aff5b4861e2b69 Mon Sep 17 00:00:00 2001 From: austinabell Date: Tue, 12 May 2020 12:11:09 -0400 Subject: [PATCH 2/3] Switch from verbose test build on CI --- .github/workflows/ci-rust.yml | 4 ++-- Makefile | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci-rust.yml b/.github/workflows/ci-rust.yml index c44ac4a3b7b..8fbaa177d55 100644 --- a/.github/workflows/ci-rust.yml +++ b/.github/workflows/ci-rust.yml @@ -34,10 +34,10 @@ jobs: override: true - name: Cargo Build Tests - run: make test-all-verbose-no-run + run: make test-all-no-run - name: Cargo test all - run: make test-all-verbose + run: make test-all fmt: name: rustfmt diff --git a/Makefile b/Makefile index 5c026b2a0d1..750b794003a 100644 --- a/Makefile +++ b/Makefile @@ -67,8 +67,8 @@ test-all: pull-serialization-tests test-all-verbose: pull-serialization-tests cargo test --verbose --all-features -test-all-verbose-no-run: pull-serialization-tests - cargo test --verbose --all-features --no-run +test-all-no-run: pull-serialization-tests + cargo test --all-features --no-run # Checks if all headers are present and adds if not license: From f456fa468700247ab005358c02509c1ec448e51f Mon Sep 17 00:00:00 2001 From: austinabell Date: Wed, 13 May 2020 08:29:41 -0400 Subject: [PATCH 3/3] Switch to using chained cmp instead of matching --- blockchain/blocks/src/header.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/blockchain/blocks/src/header.rs b/blockchain/blocks/src/header.rs index e21a12595ed..069bce6ed1e 100644 --- a/blockchain/blocks/src/header.rs +++ b/blockchain/blocks/src/header.rs @@ -204,11 +204,10 @@ impl<'de> Deserialize<'de> for BlockHeader { impl Ord for BlockHeader { fn cmp(&self, other: &Self) -> Ordering { - match self.ticket().cmp(other.ticket()) { + self.ticket() + .cmp(other.ticket()) // Only compare cid bytes when tickets are equal - Ordering::Equal => self.cid().to_bytes().cmp(&other.cid().to_bytes()), - order => order, - } + .then_with(|| self.cid().to_bytes().cmp(&other.cid().to_bytes())) } }