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

chore: ffi: copy verifier iface, mock & ffi out of storage #11581

Merged
merged 4 commits into from
Aug 9, 2024

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jan 16, 2024

Looking for some feedback on this as a step in further separating miner and chain code. Aside from itests (where everything is very entangled), the verifier is the second most entangled component since it sits within the storage/ffiwrapper package. Just by pulling it out of there it significantly reduces the amount of non-miner code that reaches into storage/ for anything; the only things left are minor and mostly single-use.

This change is mainly duplication, but very targeted, only the verifier comes out of storage/ffiwrapper.

@rvagg rvagg requested a review from magik6k January 16, 2024 12:45
@rvagg rvagg force-pushed the rvagg/non-storage-verifier branch 3 times, most recently from cdb93cd to 6a48939 Compare January 17, 2024 06:47
@rvagg
Copy link
Member Author

rvagg commented Jan 17, 2024

Deleted a couple of comments in this thread as I was trying to figure out why some itests weren't passing - it ended up being a misunderstanding of how we use fx to do interface matching so I wasn't getting the mock verifier in place for those test. Resolved now and all passing.

@rvagg rvagg marked this pull request as ready for review January 17, 2024 07:04
@rvagg rvagg requested a review from a team as a code owner January 17, 2024 07:04
@rvagg rvagg requested a review from snadrus January 17, 2024 07:04
@rvagg
Copy link
Member Author

rvagg commented Jan 22, 2024

Renamed the package to proofs so I could also put in GenerateUnsealedCID which is used by the vm. Unfortunately I can't remove it entirely from storage because it's used for the mock to do SealPreCommit1.

@rvagg rvagg force-pushed the rvagg/non-storage-verifier branch 2 times, most recently from 2e13513 to 56762ef Compare January 22, 2024 02:27
@rvagg rvagg force-pushed the rvagg/non-storage-verifier branch from 56762ef to ba0c691 Compare April 30, 2024 04:11
Copy link
Collaborator

@ribasushi ribasushi left a comment

Choose a reason for hiding this comment

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

@rvagg this entire thing is quite useful, especially now that the repo is a lot lighter. Please simplify further and make it go in 🚀 :ribaapproves:

chain/proofs/ffi/proofs.go Outdated Show resolved Hide resolved
@rvagg rvagg force-pushed the rvagg/non-storage-verifier branch from f3019f3 to b56788f Compare August 6, 2024 08:57
@rvagg rvagg changed the title chore: copy verifier iface, mock & ffi out of storage chore: ffi: copy verifier iface, mock & ffi out of storage Aug 8, 2024
chain/consensus/filcns/filecoin.go Show resolved Hide resolved
@rvagg rvagg enabled auto-merge (squash) August 9, 2024 01:04
@rvagg rvagg merged commit d7b218e into master Aug 9, 2024
85 checks passed
@rvagg rvagg deleted the rvagg/non-storage-verifier branch August 9, 2024 01:04
rjan90 pushed a commit to hanabi1224/lotus that referenced this pull request Aug 12, 2024
…project#11581)

* chore: copy verifier iface, mock & ffi out of storage

one step toward a cleaner separation of miner & chain

* ffi: s/verifier/proofs, incorporate GenerateUnsealedCID

* fix: ffi: use non-ffi version of GenerateUnsealedCID

* doc: proofs: document GenerateUnsealedCID use
rjan90 pushed a commit that referenced this pull request Aug 12, 2024
* chore: copy verifier iface, mock & ffi out of storage

one step toward a cleaner separation of miner & chain

* ffi: s/verifier/proofs, incorporate GenerateUnsealedCID

* fix: ffi: use non-ffi version of GenerateUnsealedCID

* doc: proofs: document GenerateUnsealedCID use
ribasushi pushed a commit to ribasushi/ci-abusing-lotus-fork that referenced this pull request Aug 20, 2024
…project#11581)

* chore: copy verifier iface, mock & ffi out of storage

one step toward a cleaner separation of miner & chain

* ffi: s/verifier/proofs, incorporate GenerateUnsealedCID

* fix: ffi: use non-ffi version of GenerateUnsealedCID

* doc: proofs: document GenerateUnsealedCID use
ribasushi pushed a commit to ribasushi/ci-abusing-lotus-fork that referenced this pull request Aug 20, 2024
…project#11581)

* chore: copy verifier iface, mock & ffi out of storage

one step toward a cleaner separation of miner & chain

* ffi: s/verifier/proofs, incorporate GenerateUnsealedCID

* fix: ffi: use non-ffi version of GenerateUnsealedCID

* doc: proofs: document GenerateUnsealedCID use
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does not require CHANGELOG.md update
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

4 participants