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

Separate storage collateral and pledge collateral #60

Closed
whyrusleeping opened this issue Dec 19, 2018 · 13 comments
Closed

Separate storage collateral and pledge collateral #60

whyrusleeping opened this issue Dec 19, 2018 · 13 comments
Assignees

Comments

@whyrusleeping
Copy link
Member

No description provided.

@pooja pooja transferred this issue from another repository Jan 11, 2019
@pooja pooja added key-todo and removed key-todo labels Apr 4, 2019
@pooja
Copy link
Contributor

pooja commented Apr 5, 2019

@decentralion Is this something you'd have bandwidth to help document? Thanks!

@teamdandelion
Copy link
Contributor

I can help document if I have some more context on what's expected here. What are the relevant sections of the spec? What info do we need to communicate? Etc. cc @whyrusleeping

@pooja
Copy link
Contributor

pooja commented Apr 17, 2019

I'm guessing we want something like this in the spec: https://github.com/filecoin-project/cryptoeconomics/blob/master/docs/filecoin_collateral.md

I'll work with @whyrusleeping to get the relevant context together and post it here!

@whyrusleeping
Copy link
Member Author

At a high level (mostly recap, this is what @decentralion and I have talked about before) we have two types of collateral, pledge collateral and storage collateral. Both are needed per sector committed. The pledge collateral needed per sector is set by the network, the storage collateral per miner is set individually per miner on creation (parameter to the constructor).

Concrete changes that should be made:

  • actors.md:
    • adjust miner constructor to take storage collateral
    • adjust slashing methods (slashconsensusfault, slashstoragefault, arbitratedeal) to slash the correct collateral
      • also write in the 'slashing auction' mechanism here
  • storage-market.md
    • probably want to add a section explaining how collateral works in english

@anorth
Copy link
Member

anorth commented May 29, 2019

[Copied from #294]
The breach of contract dispute in faults states that the penalty for defaulting on a contract is taken from pledge collateral. It's not explicit about whether the fee is paid to the storage client or burned. I gather this is incorrect and should refer to storage collateral.

In the actor spec, ArbitrateDeal pseudocode mentions both burning pledge collateral and transferring storage collateral to the client. How this storage collateral is represented and where it comes from isn't specified.

On being slashed in mining spec opens a discussion and TODO for disambiguating the two collaterals throughout the spec. My impression from conversations elsewhere suggest we're currently intending to identify both.

Assuming we do follow through with storage collateral, we'll need:

  • a mechanism for determining how much storage collateral is to be posted (requested by client? network constant?). @whyrusleeping's comment above suggest the miner sets a constant value (per byte/year?), that a client can take or leave
  • a mechanism for posting that collateral somewhere (payment channels w/ conditional vouchers? a new account?
  • a mechanism for retrieving or repurposing the collateral when a deal expires successfully
  • ArbitrateDeal to transfer some of that to the client when a dispute is successful
  • Extend to the "slashing auction" model so that this action can be delegated, if desired

Accounting for storage collateral is complicated somewhat by the fact that the details of a successful deal are never posted on chain. How can the network validate sufficient collateral has been posted?

A model and spec for storage collateral is blocking go-filecoin completing deal arbitration and slashing implementation.

@whyrusleeping
Copy link
Member Author

The breach of contract dispute in faults states that the penalty for defaulting on a contract is taken from pledge collateral. It's not explicit about whether the fee is paid to the storage client or burned. I gather this is incorrect and should refer to storage collateral.

heh, unfortunate overlap in word choice here. 'pledged' was not intended to overlap in meaning with 'pledge collateral'. Should change to 'posted' or similar (referring the the collateral the miner already has put up, not specific collateral)

In the actor spec, ArbitrateDeal pseudocode mentions both burning pledge collateral and transferring storage collateral to the client. How this storage collateral is represented and where it comes from isn't specified.

The collateral is simply held by the miner actor. This can be clarified in the spec.

On being slashed in mining spec opens a discussion and TODO for disambiguating the two collaterals throughout the spec. My impression from conversations elsewhere suggest we're currently intending to identify both.

Correct, thus this issue.

a mechanism for determining how much storage collateral is to be posted (requested by client? network constant?). @whyrusleeping's comment above suggest the miner sets a constant value (per byte/year?), that a client can take or leave

The idea is that its a fixed amount per byte, without any sort of time period in the units. A miner would pick an amount that they create their miner actor with, and have that be fixed over time. This has lower implementation complexity than other solutions.

a mechanism for posting that collateral somewhere (payment channels w/ conditional vouchers? a new account?

Since all collateral is held by the miner actor, any funds transferred to the miner actor count as posting collateral (either through a direct payment to the miner actor, or as funds included with another message like CommitSector)

a mechanism for retrieving or repurposing the collateral when a deal expires successfully

A mechanism for retrieving just storage collateral from a 'completed' deal without removing the sector in question is not planned. To retrieve locked collateral a miner must remove the sector. Collateral balances then get updated during SubmitPoSt, and can be retrieved by a call to DePledge (which should probably be renamed to just 'WithdrawCollateral').

ArbitrateDeal to transfer some of that to the client when a dispute is successful

AbitrateDeal does this

Extend to the "slashing auction" model so that this action can be delegated, if desired

The slashing auction and delegated slashing are two separate things. Delegated slashing already works, ArbitrateDeal accepts only a Deal, which may be submitted by anyone. This allows the client to give their deal objects to anyone so those people may slash the miner.

To implement the auction, we need to send an increasing-over-time portion of the funds to the caller of ArbitrateDeal instead of sending it all to the client. This needs to be written.

How can the network validate sufficient collateral has been posted?

Collateral checks are made when the miner commits a sector to the chain. If the miner does not have sufficient collateral, then the call to CommitSector will fail. If the miner is not able to post the sector in the agreed upon time frame (by the deals start date) the client may close their payment channel, and possibly slash the miner. This specific case of ArbitrateDeal would need to be implemented, as the current function only allows the client to slash the miner for removing sectors before the deal is complete (not for never having posted the sector). I filed an issue to discuss this at #306

@anorth
Copy link
Member

anorth commented May 29, 2019

Thanks for those responses. A model of a fixed per-byte amount, attached to sector bytes rather than (unknown) deal bytes is simple and answers many questions. I appreciate the simplicity. Apologies for not thinking fully through that before re-posting these questions.

The collateral is simply held by the miner actor

Is it tracked explicitly in the miner.Collateral/ActiveCollateral state variables, another variable, or implicitly as the wallet balance less these amounts?

Delegated slashing already works, ArbitrateDeal accepts only a Deal, which may be submitted by anyone. This allows the client to give their deal objects to anyone so those people may slash the miner.

Sorry, I should have clarified so that another party "is directly incentivised to pay the gas fees to" slash. Agree the auction model is a distinct issue. I understand that at present the reward is destined fully to the deal client.

@whyrusleeping
Copy link
Member Author

Apologies for not thinking fully through that before re-posting these questions.

It should be more clearly stated in the spec, easy thing to miss.

Is it tracked explicitly in the miner.Collateral/ActiveCollateral state variables, another variable, or implicitly as the wallet balance less these amounts?

Looking at this more closely, this is not quite right in the spec right now. The collateral field seems to be tracking free collateral, but we can more simply use the miners account balance, removing the Collateral field entirely. This simplifies a few things, I will make the change tomorrow.

@anorth
Copy link
Member

anorth commented May 31, 2019

Thanks for improvements in #312.

When you're finishing this off, can I request renaming the ActiveCollateral state variable to better describe it's role? I can't tell whether it's intended as the consensus collateral or storage collateral (or both).

It's also worth highlighting that the consensus collateral requirement may change with time, independent of any miner action, if the general collateral function (CollateralForSectors) depends on time/blockheight/FIL circulation, which I think we need to allow for. This means that a PledgeCollateral state variable is always out of date.

@laser suggested a check for sufficient collateral for an additional sector like
CollateralForSector(miner.SectorSize) < vm.MyBalance() - miner.ActiveCollateral - miner.ConsensusCollateral
but I think it needs to be something like
miner.StorageCollateral + PledgeCollateralForSectors(miner.SectorSize * (miner.ProvingSet.Len() + 1), vm.BlockHeight) <= vm.MyBalance()

This might mean that pledge collateral must always computed, not looked up. If storage collateral is also a simple function of proving set, maybe no state variables are needed? But we do still need some withdrawal delay mechanism like DePledge.

@laser
Copy link
Contributor

laser commented May 31, 2019

@anorth -

It's also worth highlighting that the consensus collateral requirement may change with time, independent of any miner action, if the general collateral function (CollateralForSectors) depends on time/blockheight/FIL circulation, which I think we need to allow for.

The spec, in its current form, shows that the required collateral (in this context) for a sector is a function of that sector's size.

What informs your understanding that the CollateralForSectors function requires arguments beyond sector size? Are you drawing from Pledge Collateral Design Doc (Baseline (Pro-rata) Construction)?

@whyrusleeping
Copy link
Member Author

whyrusleeping commented May 31, 2019 via email

teamdandelion added a commit that referenced this issue Jun 3, 2019
See context in #60.

Thanks to @whyrusleeping for help translating this into spec.
teamdandelion added a commit that referenced this issue Jun 3, 2019
See context in #60.

Thanks to @whyrusleeping for help translating this into spec.
teamdandelion added a commit that referenced this issue Jun 3, 2019
See context in #60.

Thanks to @whyrusleeping for help translating this into spec.
teamdandelion added a commit that referenced this issue Jun 5, 2019
See context in #60.

Thanks to @whyrusleeping for help translating this into spec.
teamdandelion added a commit that referenced this issue Jun 5, 2019
See context in #60.

Thanks to @whyrusleeping for help translating this into spec.
@anorth
Copy link
Member

anorth commented Jun 11, 2019

Note from a discussion in Slack that:

  • we should remove the ActiveCollateral field entirely in favour of dynamic calculations
  • the pledge withdrawal mechanism needs to be updated to handle dynamic requirements

@nicola
Copy link
Contributor

nicola commented Jul 2, 2019

#325 closes this, #371 created for minor details that still need update

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

No branches or pull requests

6 participants