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

Compute unsealed sector CID syscall #360

Merged
merged 5 commits into from
Apr 22, 2020
Merged

Conversation

austinabell
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Implements the compute_unsealed_sector_cid syscall in the runtime
    • Interfaces through the filecoin-proofs-api
  • Updates commcid interface to take in references (was no need to move variables before, but assumed they wouldn't be needed after)

Dependent on #357 so I will remove other commits when that comes into mater and open this PR up

Reference issue to close (if applicable)

Closes
#303

Other information and links

vm/Cargo.toml Outdated
@@ -15,3 +15,9 @@ num-derive = "0.3.0"
clock = { path = "../node/clock" }
thiserror = "1.0.11"
crypto = { path = "../crypto" }
commcid = { path = "../utils/commcid" }

[dependencies.filecoin-proofs-api]
Copy link
Member

@ec2 ec2 Apr 20, 2020

Choose a reason for hiding this comment

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

Why the departure from our standard way of declaring dependencies?

Copy link
Member

Choose a reason for hiding this comment

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

Also, don't understand why we need to specify a version if we already specify a rev.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just for safety, and the reason I used this format is just to make updating commit easier and more visible, I'll change to other format tho

commcid = { path = "../../utils/commcid" }

[dependencies.filecoin-proofs-api]
Copy link
Member

Choose a reason for hiding this comment

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

Same as the comment in vm/Cargo.toml

let mut sum = 0u64;
for p in pieces {
sum += p.size.0;
}
Copy link
Member

Choose a reason for hiding this comment

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

let sum: u64 = pieces.iter().map(|p| p.size.0).sum();

Now sum doesn't have to be mut. Not an important change, but a fun one.

@ec2
Copy link
Member

ec2 commented Apr 20, 2020

line 23 in vm/src/piece/mod.rs
pub fn validate(self) -> Result<(), &’static str> {
Can probably take a ref. I couldnt comment on that line so posting here

@austinabell
Copy link
Contributor Author

line 23 in vm/src/piece/mod.rs
pub fn validate(self) -> Result<(), &’static str> {
Can probably take a ref. I couldnt comment on that line so posting here

Nah it's just a u64 so better to just move in. Pretty sure clippy warns on this

@austinabell austinabell merged commit 29b4584 into master Apr 22, 2020
@austinabell austinabell deleted the austin/sys/computeseccid branch April 22, 2020 14:48
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.

None yet

3 participants