Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated blocks crate to reflect spec changes #86

Merged
merged 7 commits into from
Dec 13, 2019

Conversation

dutterbutter
Copy link
Contributor

@dutterbutter dutterbutter commented Dec 10, 2019

Changes introduced in this PR:

  • Refactored block.rs to reflect spec changes
  • Added TxMeta struct and replaced messages within blockheader to this type
  • Added bls_signature to block header with type Signature
  • Added ElectionPoStVerifyInfo and its required field structs
  • Made runtime public in order to use ChainEpoch type
  • Added Clone, Debug and PartialEq derive to ChainEpoch type in runtime for tipset validation
  • Updated tipset test to reflect struct changes
  • Added bls and secp message types to TxMeta struct

Not exactly sure what ElectionPoStVerifyInfo is responsible for, added in the comment what I assume its related to but since the spec has it included in the block component I have included it there.

Closes #80

* Refactored block.rs to reflect spec changes
* Added TxMeta struct and replaced messages within blockheader to this type
* Added bls_signature to block header with type Signature
* Added ElectionPoStVerifyInfo and its required field structs
* Made runtime public in order to use ChainEpoch type
* Added Clone, Debug and PartialEq derive to ChainEpoch type in runtime for tipset validation
* Updated tipset test to reflect struct changes
* Added bls and secp message types to TxMeta struct:
@dutterbutter
Copy link
Contributor Author

The build will fix itself when #87 comes in

@GregTheGreek
Copy link
Member

@dutterbutter Conflicts

blockchain/src/blocks/block.rs Outdated Show resolved Hide resolved
blockchain/src/blocks/block.rs Outdated Show resolved Hide resolved
* Updated comment
* removed height
blockchain/src/blocks/block.rs Show resolved Hide resolved
Comment on lines +87 to +92
/// ElectionPoStVerifyInfo seems to be connected to VRF
/// see https://github.com/filecoin-project/lotus/blob/master/chain/sync.go#L1099
struct ElectionPoStVerifyInfo {
candidates: PoStCandidate,
randomness: PoStRandomness,
proof: PoStProof,
Copy link
Contributor

Choose a reason for hiding this comment

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

unused, what is the point of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See comment made in PR conversation, basically, it's defined in the spec and so I figured I would include it as a stubbed structure until we understand its purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll leave to others opinions but this seems weird to stub out a type that may change and is not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I am impartial I can remove it if that's the popular opinion.

Copy link
Member

Choose a reason for hiding this comment

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

Id remove it. But also impartial lol

blockchain/src/blocks/block.rs Show resolved Hide resolved
blockchain/src/blocks/block.rs Outdated Show resolved Hide resolved
vm/src/lib.rs Outdated Show resolved Hide resolved
vm/src/runtime/mod.rs Outdated Show resolved Hide resolved
* Stubbed block scoped ChainEpoch type and removed runtime dependency
* made comment on unused structures
@dutterbutter
Copy link
Contributor Author

Will wait to merge until #79 comes in to reflect messages changes

* Resolved conflicts; merged in master
* Updated Block struct to include UnSignedMessage
* Removed ChainEpoch stub for clock::ChainEpoch
* Added new fn for impl ChainEpoch to be used publicly for tipset tests
@dutterbutter
Copy link
Contributor Author

@austinabell @ansermino @GregTheGreek @ec2 updated to reflect #79

@dutterbutter dutterbutter merged commit 2b8ee0e into master Dec 13, 2019
@dutterbutter dutterbutter deleted the dustin/block-refactor branch December 13, 2019 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add and Refactor Block Types
5 participants