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

Ni-PoRep Go/cgo additions #456

Closed
wants to merge 4 commits into from
Closed

Ni-PoRep Go/cgo additions #456

wants to merge 4 commits into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented May 27, 2024

Some additional changes on top of a rebased #453, while I try and wire things up for integration testing.

This has local linking for ref-fvm for filecoin-project/ref-fvm#1999 on top of master for filecoin-project/ref-fvm#2009.

It also has local linking for rust-filecoin-proofs-api for filecoin-project/rust-filecoin-proofs-api#103

--

Additions on top of #453, but this depends on filecoin-project/go-state-types#269 being merged.

Note that his also includes seal_commit_phase2_circuit_proofs in rust/src/proofs/api.rs, that could go into #453.

proofs.go Show resolved Hide resolved
@rvagg rvagg changed the title Ni-PoRep additional bits Ni-PoRep Go/cgo additions Jun 12, 2024
@rvagg rvagg mentioned this pull request Jun 12, 2024
@rvagg rvagg marked this pull request as ready for review June 13, 2024 00:35
@rvagg rvagg requested review from Stebalien and rjan90 June 13, 2024 00:35
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

I can't comment on the proofs side of things, but:

  1. The constants all look correct.
  2. The FFI side of things looks correct.

go.mod Outdated
@@ -5,7 +5,7 @@ go 1.18
require (
github.com/filecoin-project/go-address v1.1.0
github.com/filecoin-project/go-fil-commcid v0.1.0
github.com/filecoin-project/go-state-types v0.13.1
github.com/filecoin-project/go-state-types v0.14.0-dev.0.20240611104512-19a29aa50e4a
Copy link
Member

Choose a reason for hiding this comment

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

I'd cut a release if we're ready.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, it was just waiting for approval but I've given it a tick so @rjan90 can move forward there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated to go-state-types v0.14.0-rc1 in: ca67564

@rvagg
Copy link
Member Author

rvagg commented Jun 13, 2024

Got something funky going on with the gocheck2 .. on aarch it wants it as a GOEXPERIMENT=gocheck2, but Darwin and Linux builders don't recognise that. I'll have to figure that one out before this can be landed.

@Stebalien
Copy link
Member

Specifically:

  • darwin/linux x86 expect the old form.
  • linux aarch64 expect the GOEXPERIMENT version.

???

@Stebalien
Copy link
Member

Ah: the aarch64 runner is "ubuntu-2204:current" while the others are cimg/go:1.20. That latter one likely needs to be upgraded to go 1.21.

@BigLep
Copy link
Member

BigLep commented Jun 13, 2024

@rvagg : I know it's getting late your time. Will you be handling the CI issues discussed above before signing off, or are you expecting @rjan90 to?

Update GST to v0.14.0-rc1
Upgrade the golang executor to use Go 1.21
vmx added a commit that referenced this pull request Jun 13, 2024
This commit adds support for ni-porep.

It's a combination of the indivdual PRs #453, #456 and #458. Thanks everyone
involved working on those.

The changes are for the Go side (actors and FVM) as well as the Rust side
(proofs API).

Changes on the CI are:

 - Makes it match the Rust version in the rust-toolchain.toml
 - Newer Go version (1.21) on all platforms
 - Larger instance for the testsas the Ni-PoRep synthesis phase takes
   more resources.

Closes #453, #456, #458.
vmx added a commit that referenced this pull request Jun 13, 2024
This commit adds support for ni-porep.

It's a combination of the indivdual PRs #453, #456 and #458. Thanks everyone
involved working on those.

The changes are for the Go side (actors and FVM) as well as the Rust side
(proofs API).

Changes on the CI are:

 - Makes it match the Rust version in the rust-toolchain.toml
 - Newer Go version (1.21) on all platforms
 - Larger instance for the testsas the Ni-PoRep synthesis phase takes
   more resources.

Closes #453, #456, #458.
@rjan90
Copy link
Contributor

rjan90 commented Jun 13, 2024

Thanks to @vmx he has addressed the CI-issues above, and also consolidated the three different filecoin-ffi PRs related to NI-PoRep into #459

@BigLep
Copy link
Member

BigLep commented Jun 13, 2024

I'm closing given superseded by #459

@BigLep BigLep closed this Jun 13, 2024
vmx added a commit that referenced this pull request Jun 18, 2024
* feat: add support for Non-interactive PoRep

This commit adds support for ni-porep.

It's a combination of the indivdual PRs #453, #456 and #458. Thanks everyone
involved working on those.

The changes are for the Go side (actors and FVM) as well as the Rust side
(proofs API).

Changes on the CI are:

 - Makes it match the Rust version in the rust-toolchain.toml
 - Newer Go version (1.21) on all platforms
 - Larger instance for the testsas the Ni-PoRep synthesis phase takes
   more resources.

Closes #453, #456, #458.

---------

Co-authored-by: Steven Allen <steven@stebalien.com>
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: nemo <nemo@ellipticresearch.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

5 participants