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

refactor:ffi: replace ClearLayerData with ClearCache #11352

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

vmx
Copy link
Contributor

@vmx vmx commented Oct 25, 2023

Related Issues

API surface reduction.

Proposed Changes

The ClearLayerData FFI call was accidentally introduced with the Synthetic PoRep. The call does under the hood exactly what ClearCache is doing. This is a first step to remove ClearLayerDatat also from the FFI again, in order to reduce the API surface.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

The `ClearLayerData` FFI call was accidentally introduced with the
Synthetic PoRep. The call does under the hood exactly what `ClearCache`
is doing. This is a first step to remove `ClearLayerData`t also from
the FFI again, in order to reduce the API surface.
@vmx vmx requested a review from a team as a code owner October 25, 2023 14:23
@Stebalien
Copy link
Member

@vmx build appears to be broken.

@vmx
Copy link
Contributor Author

vmx commented Oct 27, 2023

@Stebalien
Copy link
Member

@Stebalien
Copy link
Member

Oh, I see. The "proofs" one converts all the types into cgo types.

@Stebalien
Copy link
Member

We could also fix this in the FFI by making ClearCache take an abi.SectorSize (that's the correct fix, just more annoying).

@Stebalien Stebalien merged commit 21f8f64 into filecoin-project:master Oct 27, 2023
vmx added a commit to filecoin-project/filecoin-ffi that referenced this pull request Nov 3, 2023
The `ClearLayerData` FFI call was accidentally introduced with the
Synthetic PoRep. The call does under the hood exactly what `ClearCache`
is doing. It was already removed from Lotus in [PR 11352], so that
we can remove it here as well (and as a next steps from the Rust
side). This reduces the API surface, which is generally a good idea.

[PR 11352]: filecoin-project/lotus#11352
vmx added a commit to filecoin-project/filecoin-ffi that referenced this pull request Nov 3, 2023
The `ClearLayerData` FFI call was accidentally introduced with the
Synthetic PoRep. The call does under the hood exactly what `ClearCache`
is doing. It was already removed from Lotus in [PR 11352], so that
we can remove it here as well (and as a next steps from the Rust
side). This reduces the API surface, which is generally a good idea.

[PR 11352]: filecoin-project/lotus#11352
vmx added a commit to filecoin-project/filecoin-ffi that referenced this pull request Nov 3, 2023
The `ClearLayerData` FFI call was accidentally introduced with the
Synthetic PoRep. The call does under the hood exactly what `ClearCache`
is doing. It was already removed from Lotus in [PR 11352], so that
we can remove it here as well (and as a next steps from the Rust
side). This reduces the API surface, which is generally a good idea.

[PR 11352]: filecoin-project/lotus#11352
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants