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

Miner accepts deal even if there's not enough collateral in miner to service the deal. #5444

Closed
dineshshenoy opened this issue Jan 27, 2021 · 5 comments
Labels
area/markets/client area/markets/storage P2 P2: Should be resolved team/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs

Comments

@dineshshenoy
Copy link

v1.4.1

Current behavior: The miner accepts a deal and starts the data transfer. After the data transfer is complete, it checks and aborts the deal because of insufficient funds.

Desired: It would be helpful to issue a warning or check funds before initiating the data transfer if the miner doesn't have the resources to accept the deal.

Open Questions:

  1. We have a market actor concept when proposing deals. Should we have an equivalent concept for miners when the accept a deal and are waiting for the data transfer (essentially lock the funds?)
  2. Does this open up risks in terms of a DoS attack if funds are locked since some deals maybe aborted?
@dineshshenoy dineshshenoy added P2 P2: Should be resolved area/client/storage labels Jan 27, 2021
@dineshshenoy dineshshenoy added this to the 💹Storage Deal Success milestone Jan 27, 2021
@f8-ptrk
Copy link
Contributor

f8-ptrk commented Jan 29, 2021

Current behavior: The miner accepts a deal and starts the data transfer. After the data transfer is complete, it checks and aborts the deal because of insufficient funds.

all that is off chain logic. the funds are needed when the deal gets published and not when accepted or transferred.

the miner should initiate an AddBalance message to add funds before the publish deals message - and as far as i know it is doing exactly that if there aren't enough funds to lock for a deal in the market actor for the miner

Open Questions:

  1. We have a market actor concept when proposing deals. Should we have an equivalent concept for miners when the accept > a deal and are waiting for the data transfer (essentially lock the funds?)
  2. Does this open up risks in terms of a DoS attack if funds are locked since some deals maybe aborted?
  1. you shouldn't touch any funds a miner rightfully owns and "lock" them. (whatever you mean with 'lock the funds'). the funds will get locked when the publish message gets on chain.
  2. yes it does. an attacker makes enough deals and then transfers the data with snailmail-speed over to the miner. possibly "locking" funds for hours, days - even weeks

but still not sure what you mean with "lock funds"

the proposal is a storage client side, signed "document" and as soon as he sends it out he has no chance to revert it - that's why his funds should be "locked" when this gets send out. but the miner is not "accepting" the deal before the publish message hits the chain/is created. it's more a "i will have a look at your proposal" than acceptance before the publish message is signed

@f8-ptrk
Copy link
Contributor

f8-ptrk commented Jan 29, 2021

a client can always demand the deal to be published before he sends the data. what will happen is that miners will let the client pay for the publish deals message since they do not have the data before paying, right now, ~$6 to get the message on chain.

[edit] does the storage client actually have to lock the funds before (in the sense of hard locked in the market actor) before the publish message is on chain - or is it a software side soft lock that just tells him that the funds are locked? how does the chain know that the client created a proposal and send it out?

[edit2]
https://spec.filecoin.io/_gen/diagrams/systems/filecoin_markets/storage_market/storage_market_flow.svg?1605115265

i don't think anyone should need to lock any funds before the "publish deal on chain" step - and i doubt anyone does right now.

rect1331

looking at it the diagram right now: it explicitly puts the "Deal Accepted" step after the deal was published!

@f8-ptrk
Copy link
Contributor

f8-ptrk commented Jan 29, 2021

so this is not an implementation problem but a specifications problem of how to define when a deal is accepted and what locked funds are. right now, rightfully, the deal isn't "Accepted" and funds aren't "locked" before both parties have a message with their signature in/on it on chain.

if you/we/lotus wants to implement a soft lock on miners market balances before the deal is on chain - feel free to do so, but make it optional.

@raulk raulk added the team/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs label Feb 23, 2021
@raulk raulk changed the title Miner accepts deal even if ther's not enough collateral in miner to service the deal. Miner accepts deal even if there's not enough collateral in miner to service the deal. Mar 9, 2021
@dkkapur dkkapur modified the milestone: 💹Storage Deal Success May 3, 2021
@rjan90
Copy link
Contributor

rjan90 commented Nov 24, 2021

It seems like a fix for this issue has been merged into Lotus, so this issue can probably be closed now? #rengjøring

@Reiers
Copy link

Reiers commented Dec 12, 2021

Merged.

Closing ticket.

@Reiers Reiers closed this as completed Dec 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/markets/client area/markets/storage P2 P2: Should be resolved team/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs
Projects
None yet
Development

No branches or pull requests

7 participants