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

FIP Discussion: In PublishStorageDeals a bad deal doesn't ruin the batch #142

Closed
ZenGround0 opened this issue Aug 16, 2021 · 1 comment
Closed
Labels
FIP0022 Links an existing discussion item to an existing FIP.

Comments

@ZenGround0
Copy link
Contributor

Summary

A single bad deal in the parameters of a PublishStorageDeals message should not invalidate the publishing of all the valid deals in the parameters. Instead the message will succeed and publish only the valid deals

Motivation

Because to some extent deal errors are unavoidable this issue is frequently brought up by storage providers as a major issue. For example filecoin-project/specs-actors#1466. This change will make deal publishing errors less expensive for storage providers and less disruptive for clients.

Design

The simplest approach is to simply drop deals that error and publish the valid ones. The downside is that the receipt no longer contains information about the failed deal. To correct this we can add a field to the PSD return value signaling which of the input deals was successfully published. This goes from:

PublishStorageDealsReturn struct {
    IDs []abi.DealID
}

to

PublishStorageDealsReturn struct {
    IDs []abi.DealID
    Valid bitfield.Bitfield
}

We use a bitfield to keep the return bytes compact. The bitfield is over parameter index. With this new field consumers of the PSDs receipt can maintain exact information on which proposals are matched to which deal ids.

If all proposals fail validation PSD will return an error.

To communicate information about how each deal errored the updated PSD implementation should include log messages communicating the reason each failed proposal is invalid.

Considerations

This should have no impact on security and no impact on incentives other than making it better to use the filecoin storage market.

Note: in the past changing the error handling or batching semantics of actor methods has not required its own fip. In this case I think this change deserves its own fip because PublishStorageDeals is not an internal state machine method but something sent by miner operators and parsed by tooling across the ecosystem.

@kaitlin-beegle
Copy link
Collaborator

Closing due to FIP acceptance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FIP0022 Links an existing discussion item to an existing FIP.
Projects
None yet
Development

No branches or pull requests

2 participants