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

Actors v2 upgrade setup #854

Merged
merged 22 commits into from
Dec 11, 2020
Merged

Actors v2 upgrade setup #854

merged 22 commits into from
Dec 11, 2020

Conversation

austinabell
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Bumps commit for unchanged actors
  • Setup like updating code Cids for v2

This is blocked on finishing and exporting v1 actors, which is not yet but soon ready to be done, but I will just keep putting changes in this PR which are necessary until then

Reference issue to close (if applicable)

Closes #816
Closes #817
Closes #815
Closes #824

Other information and links

@austinabell austinabell added Spec Change Updates to implementation required due to a Filecoin spec change after implementation Status: Needs Review and removed Status: DO NOT MERGE labels Dec 9, 2020
@austinabell austinabell marked this pull request as ready for review December 9, 2020 21:15
@@ -12,7 +12,7 @@ pub use self::message::*;
pub use self::rand_replay::*;
pub use self::stubs::*;
pub use self::tipset::*;
use actor::CHAOS_ACTOR_CODE_ID;
use actor::actorv2::CHAOS_ACTOR_CODE_ID;
Copy link
Member

Choose a reason for hiding this comment

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

Curious how you're passing v1 conformance tests with v2 chaos actor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code Cid is still using the /1/ path. We probably shouldn't even have chaos within actors because it isn't tied to the actor versions. But can change later

#[derive(Debug, PartialEq, Serialize)]
#[serde(rename_all = "PascalCase")]
pub struct MinerInfo {
pub owner: Address,
Copy link
Member

Choose a reason for hiding this comment

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

These need to be serialized using json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops

Copy link
Member

@ec2 ec2 left a comment

Choose a reason for hiding this comment

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

Very funky fresh setup. I focused mostly outside of the VM, so the state manager, mpool, and chain sync.
I looked at the structure of the stuff in the actors and vm code. Didnt look too hard into the correctness because if it passes conformance v1 actors and can still sync to breeze, then it should be all fine.

Tested against storage miner init functionality as well and its all good after that json fix.

@austinabell austinabell merged commit 47f2bd1 into main Dec 11, 2020
@austinabell austinabell deleted the austin/actv2setup branch December 11, 2020 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Spec Change Updates to implementation required due to a Filecoin spec change after implementation Status: Needs Review
Projects
None yet
2 participants