New Tipset w/ unit tests #56

dutterbutter · 2019-11-29T21:38:59Z

Changes introduced in this pr:

Added logic to tipset fn new which includes updated spec conditional checks and sorting based on ticket size
Added equals tipset_key method
Added placeholder fn cid for generating CID for headers
Added unit tests for tipset methods
Updated tipset comment to reflect spec changes

Todo:

The CBOR encoding and multihash support for blake2b is still required to complete cid fn and then will need to implement a cid check within the new fn

Closes #8

* Added multihash package to retrieve blake2b hash * Added sort_key method for Ticket struct * Added equals method for TipSetKeys struct type * WIP for Tipset new fn * WIP cid method for block type which would return CID of block

* Added new conditional checks as per spec update * WIP sorted and compare logic for ticket size * Fix cid method for blockHeader * WIP new fn tests

* Added unit tests for tipset methods

* Added new tipset logic, includes conditional checks and sorting based on ticket size * Added unit tests for tipset methods * Added cid fn although it is incomplete as we need cbor encoding

* Fixed equals fn check * Linted

blockchain/Cargo.toml

blockchain/src/blocks/block.rs

blockchain/src/blocks/tipset.rs

vm/address/src/lib.rs

blockchain/src/blocks/tipset.rs

* Updated multihash to specific version * Removed extern crate statement * Improved commenting * Updated sort statement * Improved unit test setup * Updated error messages for tipset condition checks

blockchain/src/blocks/block.rs

blockchain/src/blocks/mod.rs

austinabell · 2019-12-02T18:13:27Z

blockchain/src/blocks/tipset.rs

+            // push headers into vec for sorting
+            sorted_headers.push(headers[i].clone());
+            // push header cid into vec for unique check
+            cids.push(headers[i].clone().cid());


so the cids don't have to be in sorted order? What are they used for if they can just be pulled from the headers?

They don't have to be sorted that was my mistake, but they need to be pushed into a vector to be used for an additional condition check to ensure each CID is unique and not duplicated. Currently, this is outlined as a TODO as we require fn cid in block.rs to be completed.

instead of a vector of the cids could just be a hash set? I'm okay with this though since efficiency isn't a priority

I suppose it could be but I would suggest waiting to make that change until the other pieces required are completed.

* Updated cid reference * Removed pub from mod.rs

blockchain/src/blocks/block.rs

blockchain/src/blocks/ticket.rs

blockchain/src/blocks/errors.rs

ec2

Some cosmetics. Shouldn't need a method in Ticket to return clone of vrfproof. Moreover, even if you did, better naming than sort_key.

* Updated error to handle String * Removed unnecessary import * Removed sort_key method for Ticket

dutterbutter · 2019-12-02T22:36:39Z

Requested changes made @ec2 @GregTheGreek

ec2

LGTM. Good stuff bro

GregTheGreek

lgtm

blockchain/src/blocks/ticket.rs

* Removed dead code and unused variable tag

austinabell

nits and clarifications

Ignore the test case comments if you guys don't care, this setup just doesn't seem the best for indicating this does what it should and that if things change with the setup that the tests will still work

austinabell · 2019-12-02T23:12:10Z

blockchain/src/blocks/tipset.rs

+        let data0: Vec<u8> = vec![1, 4, 3, 6, 7, 1, 2];
+        let data1: Vec<u8> = vec![1, 4, 3, 6, 1, 1, 2, 2, 4, 5, 3, 12, 2, 1];
+        let data2: Vec<u8> = vec![1, 4, 3, 6, 1, 1, 2, 2, 4, 5, 3, 12, 2];
+        let cids = key_setup();


why is this even used here when individual cids are passed in individually only?

Need unique cid for cached_cid (individual cid passed in), and the same cid between each header for state_root, message_receipts, and parent_cids which is derived from another call to key_setup

blockchain/src/blocks/tipset.rs

austinabell · 2019-12-02T23:15:19Z

blockchain/src/blocks/tipset.rs

+    const HEIGHT: u64 = 1;
+    const CACHED_BYTES: u8 = 0;
+
+    fn template_key(data: &[u8]) -> Cid {


the point of this is to not rely on indexing the key_setup() so hard to follow what is being tested

blockchain/src/blocks/tipset.rs

dutterbutter · 2019-12-02T23:43:52Z

@austinabell In an effort to try to explain the tests via your comments I have concluded that they're difficult to follow...although I believe they test correctly I am going to restructure and make them far more clear.

blockchain/src/blocks/tipset.rs

dutterbutter added 9 commits November 25, 2019 19:51

Blockchain Interfaces WIP

4aca649

* Added multihash package to retrieve blake2b hash * Added sort_key method for Ticket struct * Added equals method for TipSetKeys struct type * WIP for Tipset new fn * WIP cid method for block type which would return CID of block

New Tipset fn WIP

c8c9934

* Added new conditional checks as per spec update * WIP sorted and compare logic for ticket size * Fix cid method for blockHeader * WIP new fn tests

Added unit tests

dc60488

* Added unit tests for tipset methods

WIP tipset new fn

0a5b9a8

Added unit tests

e09cf5a

* Added unit tests for tipset methods

Added tipset new logic

935734b

* Added new tipset logic, includes conditional checks and sorting based on ticket size * Added unit tests for tipset methods * Added cid fn although it is incomplete as we need cbor encoding

Fixed equals fn

592b48b

* Fixed equals fn check * Linted

Resolved conflicts

84d8ad2

Updated with base

bee09c5

dutterbutter requested review from austinabell, GregTheGreek, ansermino and ec2 November 29, 2019 21:44

dutterbutter added Status: Needs Review Blockchain labels Nov 29, 2019

Remove typo

423c49c

austinabell suggested changes Nov 30, 2019

View reviewed changes

austinabell reviewed Nov 30, 2019

View reviewed changes

blockchain/src/blocks/tipset.rs Outdated Show resolved Hide resolved

blockchain/src/blocks/tipset.rs Outdated Show resolved Hide resolved

Made requested changes

84be3f0

* Updated multihash to specific version * Removed extern crate statement * Improved commenting * Updated sort statement * Improved unit test setup * Updated error messages for tipset condition checks

dutterbutter requested a review from austinabell December 2, 2019 17:38

austinabell suggested changes Dec 2, 2019

View reviewed changes

Made requested changes

4b2e509

* Updated cid reference * Removed pub from mod.rs

ec2 reviewed Dec 2, 2019

View reviewed changes

blockchain/src/blocks/block.rs Outdated Show resolved Hide resolved

ec2 reviewed Dec 2, 2019

View reviewed changes

blockchain/src/blocks/block.rs Outdated Show resolved Hide resolved

ec2 reviewed Dec 2, 2019

View reviewed changes

blockchain/src/blocks/block.rs Outdated Show resolved Hide resolved

GregTheGreek reviewed Dec 2, 2019

View reviewed changes

blockchain/src/blocks/ticket.rs Outdated Show resolved Hide resolved

ec2 reviewed Dec 2, 2019

View reviewed changes

blockchain/src/blocks/errors.rs Outdated Show resolved Hide resolved

ec2 requested changes Dec 2, 2019

View reviewed changes

Requested changes made

8c1cf22

* Updated error to handle String * Removed unnecessary import * Removed sort_key method for Ticket

ec2 approved these changes Dec 2, 2019

View reviewed changes

GregTheGreek approved these changes Dec 2, 2019

View reviewed changes

dutterbutter requested a review from austinabell December 2, 2019 23:03

austinabell suggested changes Dec 2, 2019

View reviewed changes

blockchain/src/blocks/ticket.rs Outdated Show resolved Hide resolved

Requested changes made

c36e37b

* Removed dead code and unused variable tag

dutterbutter requested a review from austinabell December 2, 2019 23:20

austinabell reviewed Dec 2, 2019

View reviewed changes

austinabell suggested changes Dec 2, 2019

View reviewed changes

blockchain/src/blocks/tipset.rs Outdated Show resolved Hide resolved

dutterbutter added 2 commits December 3, 2019 10:18

Remove reverse in sort

e31303b

improved tests

f71cd41

dutterbutter removed the request for review from ansermino December 3, 2019 17:04

austinabell approved these changes Dec 3, 2019

View reviewed changes

dutterbutter merged commit 9fba7d9 into master Dec 3, 2019

dutterbutter deleted the dustin/chain-types branch December 3, 2019 17:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Tipset w/ unit tests #56

New Tipset w/ unit tests #56

dutterbutter commented Nov 29, 2019 •

edited

austinabell Dec 2, 2019

dutterbutter Dec 2, 2019

austinabell Dec 2, 2019

dutterbutter Dec 2, 2019

ec2 left a comment

dutterbutter commented Dec 2, 2019

ec2 left a comment

GregTheGreek left a comment

austinabell left a comment •

edited

austinabell Dec 2, 2019

dutterbutter Dec 3, 2019

austinabell Dec 2, 2019

dutterbutter commented Dec 2, 2019

New Tipset w/ unit tests #56

New Tipset w/ unit tests #56

Conversation

dutterbutter commented Nov 29, 2019 • edited

austinabell Dec 2, 2019

Choose a reason for hiding this comment

dutterbutter Dec 2, 2019

Choose a reason for hiding this comment

austinabell Dec 2, 2019

Choose a reason for hiding this comment

dutterbutter Dec 2, 2019

Choose a reason for hiding this comment

ec2 left a comment

Choose a reason for hiding this comment

dutterbutter commented Dec 2, 2019

ec2 left a comment

Choose a reason for hiding this comment

GregTheGreek left a comment

Choose a reason for hiding this comment

austinabell left a comment • edited

Choose a reason for hiding this comment

austinabell Dec 2, 2019

Choose a reason for hiding this comment

dutterbutter Dec 3, 2019

Choose a reason for hiding this comment

austinabell Dec 2, 2019

Choose a reason for hiding this comment

dutterbutter commented Dec 2, 2019

dutterbutter commented Nov 29, 2019 •

edited

austinabell left a comment •

edited