Basic blockchain types and Tipset methods #28

dutterbutter · 2019-11-21T22:08:20Z

Changes introduced in this PR:

Added Blockheader, Block, Ticket, Tipset types
Added Tipset utility methods (len, is_empty, key, min_ticket etc.)
Started errors.rs for blockchain component

Stubbed functions:

new

new which will be used to assemble blocks into a TipSet.

Todo:

Add component logic to mod.rs

Closes #9, closes #23

* Defined block header and block structures * Defined ticket structure with vrfproof type * Defined tipset structure and wip new_tip_set fn with tests

* Added utility methods to the Tipset type * Updated comments * Linted using clippy and rustfmt

* Fix min_ticket return statement * update comments

GregTheGreek · 2019-11-21T22:15:52Z

Fyi If you write:

Closes #9, #23 this should auto close them on merge

ansermino

I think we should use doc comments for the function documentation (/// instead of //).

Also -- just wanted to point out the use of pub on fields in the BlockHeader struct (and elsewhere), I think this is generally a bad practice, but until we have a better sense of how we are constructing the blocks we can probably leave as is.

blockchain/src/blocks/tipset.rs

ansermino · 2019-11-21T22:56:10Z

Fyi If you write:

Closes #9, #23 this should auto close them on merge

That only closes the first one iirc

dutterbutter · 2019-11-21T23:01:03Z

I think we should use doc comments for the function documentation (/// instead of //).

Also -- just wanted to point out the use of pub on fields in the BlockHeader struct (and elsewhere), I think this is generally a bad practice, but until we have a better sense of how we are constructing the blocks we can probably leave as is.

I added pub to fields that were also exported in go implementation, let me know if you want them removed

austinabell

Looks good, just minor questions/ comments. One thing I would suggest is we default to making functions private over public unless actually needed to be public. Reason for this is to not crowd possible imports and expose functions/parameters that should not be accessed outside of this module or crate

blockchain/src/blocks/block.rs

blockchain/src/blocks/ticket.rs

blockchain/src/blocks/tipset.rs

blockchain/src/blocks/block.rs

blockchain/src/blocks/tipset.rs

* Added Tipset comment * Added clone to TipSetKeys * Referenced Address type from vm * Return proper error rather than integer in min_timestamp

dutterbutter · 2019-11-22T16:27:30Z

@ansermino @austinabell @GregTheGreek changes made as requested

* Added Option to min_timestamp return rather than error

austinabell

:D looks like a good framework with what's available. Would be nice to have some sort of tests though so the logic and function signatures don't have to be assumed to be correct

dutterbutter · 2019-11-22T16:48:53Z

:D looks like a good framework with what's available. Would be nice to have some sort of tests though so the logic and function signatures don't have to be assumed to be correct

Yeah tests will be added shortly after new fn is implemented and we have some agreement on where we want tests to live as per your slack message

blockchain/src/blocks/errors.rs

blockchain/src/blocks/mod.rs

blockchain/src/blocks/ticket.rs

blockchain/src/blocks/tipset.rs

blockchain/src/blocks/block.rs

ec2

Remove extern crate. Other than that, its cosmetics, that are opinionated, so feel free to disregard those.

* Added blockheader comment * Added vrfpi comment * Removed error prefixes

blockchain/src/blocks/tipset.rs

* Moved and renamed fn under impl Tipset

dutterbutter · 2019-11-22T17:22:13Z

@GregTheGreek @ec2 @ansermino changes made as requested

blockchain/src/blocks/tipset.rs

* Removed extern crate * Moved the vfriproof type to top of file

austinabell

new commits look good

ec2 · 2019-11-25T19:19:28Z

blockchain/src/blocks/block.rs

+use cid::Cid;
+use vm::address::Address;
+
+/// BlockHeader defines header of a block in the filecoin blockchain


filecoin -> Filecoin

blockchain/src/blocks/block.rs

ec2 · 2019-11-25T19:22:05Z

blockchain/src/blocks/block.rs

+    /// MINER INFO
+    ///
+    /// miner_address is the address of the miner actor that mined this block
+    miner_address: Address,


What's the rational of what needs to be public or private in this struct?

Based on the above comments, only make the fields public that are currently being used (e.g. height, parents). Once further functionality has been added the remainder of the fields in this struct will become public.

blockchain/src/blocks/tipset.rs

blockchain/src/blocks/ticket.rs

ec2

Minor cosmetics stuffs

ec2

lgtm

dutterbutter added 4 commits November 20, 2019 17:16

Initial structures defined

a6b5b1e

* Defined block header and block structures * Defined ticket structure with vrfproof type * Defined tipset structure and wip new_tip_set fn with tests

Added Tipset Methods

533e74c

* Added utility methods to the Tipset type * Updated comments * Linted using clippy and rustfmt

Fix return type

cac34bd

* Fix min_ticket return statement * update comments

Ran cargo fmt

dc2b8a9

dutterbutter requested review from austinabell, ansermino, ec2 and GregTheGreek November 21, 2019 22:13

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

ansermino reviewed Nov 21, 2019

View reviewed changes

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

austinabell suggested changes Nov 21, 2019

View reviewed changes

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

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

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

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

GregTheGreek requested changes Nov 22, 2019

View reviewed changes

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

austinabell reviewed Nov 22, 2019

View reviewed changes

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

Resolved requested changes

edd8a9f

* Added Tipset comment * Added clone to TipSetKeys * Referenced Address type from vm * Return proper error rather than integer in min_timestamp

dutterbutter added 2 commits November 22, 2019 10:44

Updated return type

cd630f2

* Added Option to min_timestamp return rather than error

ran cargo fmt

a0c8246

austinabell approved these changes Nov 22, 2019

View reviewed changes

ec2 reviewed Nov 22, 2019

View reviewed changes

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

ec2 reviewed Nov 22, 2019

View reviewed changes

blockchain/src/blocks/mod.rs Show resolved Hide resolved

ec2 reviewed Nov 22, 2019

View reviewed changes

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

ec2 reviewed Nov 22, 2019

View reviewed changes

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

ec2 reviewed Nov 22, 2019

View reviewed changes

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

ec2 requested changes Nov 22, 2019

View reviewed changes

Requested changes made

8b17ff8

* Added blockheader comment * Added vrfpi comment * Removed error prefixes

GregTheGreek reviewed Nov 22, 2019

View reviewed changes

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

GregTheGreek reviewed Nov 22, 2019

View reviewed changes

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

Moved new_tip_set under impl Tipset

d9d0708

* Moved and renamed fn under impl Tipset

austinabell reviewed Nov 22, 2019

View reviewed changes

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

updated variable name

c1eff55

dutterbutter requested review from GregTheGreek, ec2 and ansermino November 22, 2019 18:20

GregTheGreek approved these changes Nov 22, 2019

View reviewed changes

austinabell approved these changes Nov 23, 2019

View reviewed changes

dutterbutter added 2 commits November 25, 2019 09:35

Updated comments

249449f

* Removed extern crate * Moved the vfriproof type to top of file

ran cargo fmt

da74bfd

austinabell approved these changes Nov 25, 2019

View reviewed changes

ec2 reviewed Nov 25, 2019

View reviewed changes

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

ec2 reviewed Nov 25, 2019

View reviewed changes

Update comments

ee09687

ec2 reviewed Nov 25, 2019

View reviewed changes

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

Updated min_timestamp return type

4fcb353

ec2 reviewed Nov 25, 2019

View reviewed changes

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

ec2 requested changes Nov 25, 2019

View reviewed changes

Updated vrfproof field name

1298876

ec2 approved these changes Nov 25, 2019

View reviewed changes

dutterbutter merged commit ae84675 into master Nov 25, 2019

dutterbutter deleted the dustin/tipset-structures branch November 25, 2019 19:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic blockchain types and Tipset methods #28

Basic blockchain types and Tipset methods #28

dutterbutter commented Nov 21, 2019 •

edited

GregTheGreek commented Nov 21, 2019 •

edited

ansermino left a comment

ansermino commented Nov 21, 2019

dutterbutter commented Nov 21, 2019

austinabell left a comment

dutterbutter commented Nov 22, 2019

austinabell left a comment

dutterbutter commented Nov 22, 2019 •

edited

ec2 left a comment

dutterbutter commented Nov 22, 2019

austinabell left a comment

ec2 Nov 25, 2019

ec2 Nov 25, 2019

dutterbutter Nov 25, 2019

ec2 left a comment •

edited

ec2 left a comment

Basic blockchain types and Tipset methods #28

Basic blockchain types and Tipset methods #28

Conversation

dutterbutter commented Nov 21, 2019 • edited

GregTheGreek commented Nov 21, 2019 • edited

ansermino left a comment

Choose a reason for hiding this comment

ansermino commented Nov 21, 2019

dutterbutter commented Nov 21, 2019

austinabell left a comment

Choose a reason for hiding this comment

dutterbutter commented Nov 22, 2019

austinabell left a comment

Choose a reason for hiding this comment

dutterbutter commented Nov 22, 2019 • edited

ec2 left a comment

Choose a reason for hiding this comment

dutterbutter commented Nov 22, 2019

austinabell left a comment

Choose a reason for hiding this comment

ec2 Nov 25, 2019

Choose a reason for hiding this comment

ec2 Nov 25, 2019

Choose a reason for hiding this comment

dutterbutter Nov 25, 2019

Choose a reason for hiding this comment

ec2 left a comment • edited

Choose a reason for hiding this comment

ec2 left a comment

Choose a reason for hiding this comment

dutterbutter commented Nov 21, 2019 •

edited

GregTheGreek commented Nov 21, 2019 •

edited

dutterbutter commented Nov 22, 2019 •

edited

ec2 left a comment •

edited