Skip to content
This repository has been archived by the owner on Jan 29, 2024. It is now read-only.

FM-162: FIL actor feature #128

Merged
merged 3 commits into from
Jul 31, 2023
Merged

FM-162: FIL actor feature #128

merged 3 commits into from
Jul 31, 2023

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Jul 31, 2023

Changes

  • Adds a fil-actor feature so that we can depend on ipc-sdk without having to add a dependency on the fvm-utils/fil-actor-runtime crate, which is a modified copy of the builtin-actors Runtime, which was generally not needed so far in a host application like Fendermint or the FVM.

Tests

It still compiles, and disabling the feature only removes some methods that rely on the Runtime, which I do not intend to use. I added a step to the CI build to check the ipc-sdk with no default features, to catch any potential compilation bugs.

Issues

Not at the moment.

@aakoshh aakoshh requested a review from adlrocha July 31, 2023 12:29
Copy link
Contributor

@adlrocha adlrocha left a comment

Choose a reason for hiding this comment

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

This approach LGTM for now. However, as we are deprecating the native actors for now, I think we should take all common types for IPC to an independent repo (or maybe a crate inside IPC-agent/Fendermint?). That way we could archive this repo (if needed). @aakoshh, @cryptoAtwill, WDYT?

Also, @aakoshh, it seems adding the feature in one of the methods break the actor.

@aakoshh
Copy link
Contributor Author

aakoshh commented Jul 31, 2023

I don't have an opinion on the matter because I never used these; I just need the SubnetID at the moment so I have something to put into the genesis JSON.

@aakoshh aakoshh requested a review from adlrocha July 31, 2023 14:06
@adlrocha adlrocha merged commit a4e7c2c into main Jul 31, 2023
3 checks passed
@adlrocha adlrocha deleted the fm-162-fil-actor-feature branch July 31, 2023 17:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants