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

Market actor state #330

Merged
merged 4 commits into from
Apr 7, 2020
Merged

Market actor state #330

merged 4 commits into from
Apr 7, 2020

Conversation

austinabell
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Everything but the actor implementation

Reference issue to close (if applicable)

Closes #320

Other information and links


self.delete_deal(store, deal_id, deal)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Although I don't see it used anywhere, generateStorageDealID is not included here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's only used in the market actor, which is why I think I left it out initially. I added it now anyway though

assert!(deal.start_epoch <= epoch);

let deal_end = if ever_slashed {
assert!(state.slash_epoch.unwrap() <= deal.end_epoch);
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to leave assets in the code?

Copy link
Member

Choose a reason for hiding this comment

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

asserts*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no reason not to I don't think, if it fails it is a bug or something malicious, and would be caught. Not really a good reason to just pass back an error that could be missed

Copy link
Member

Choose a reason for hiding this comment

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

True. Would it make sense to fail with a message then? Or do asserts give you the line number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They do, I can add a message anyway I guess, this isn't something we expect to ever hit though, just a sanity check

@austinabell austinabell merged commit d30d396 into master Apr 7, 2020
@austinabell austinabell deleted the austin/actors/marketstate branch April 7, 2020 20:00
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.

Market Actor State
3 participants