search

FilOzone / pdp

Smart contracts and utilities for Proof of Data Possession
Other
2 stars 2 forks source link

Fix grinding in PDP Service #46

Closed ZenGround0 closed 1 month ago

ZenGround0 commented 1 month ago

This PR fixes the lack of grinding protection in PDPService. This PR started out as a spike exploring a couple possibility. See below for the historical context

--history-- This is a spike to see how we like changing the PDPService directly to disable grinding.

The PDP Service itself only issues challenges against the proof set data that existed before the challenge was generated. All removals are enqueued and processed after proof so that they cannot be used to selectively grind. All adds are enqueued and committed to the proving set after proof.

This is the obvious way to do it. @nicola suggested putting this outside the main service since in cases where the application specifies addition and removal very tightly (i.e. up to ordering and epoch of add and remove) there is no grinding concern.

ZenGround0 commented 1 month ago

I've decided this is the right way forward so I'll convert this spike into a real PR shortly

anorth commented 1 month ago

A big issue I see with this approach is that if a provider is somehow unable to prove a challenged leaf, for example, they've lost the data, then the entire proof set is permanently borked.

We need to allow the provider to admit defeat and move on somehow. The prior code does this by allowing the SP to add or remove roots to create a challenge they can meet, i.e. grinding, but in such a way that might fail the application level requirements. I agree we do need to address grinding more directly though and wish that had been considered in the protocol design earlier.

ZenGround0 commented 1 month ago

We need to allow the provider to admit defeat and move on somehow.

Great point I hadn't uncovered yet. Right now I'm thinking "faultProvingPeriod" which allows processing all proving period operations including removals and resampling and registering a fault.

ZenGround0 commented 1 month ago

@anorth your comments got me experimenting with a better factoring which I'm convinced is better than before. We now have two methods on the interface in place of the one completeProvingPeriod.

  1. The old provePossess except it does not sample challenges
  2. A new nextProvingPeriod which updates the proving set and resamples challenges.

For liveness nextProvingPeriod can be called without proving first. The application sitting on the receiver can make note of this and fault.

It might be cleaner to force an original call to nextProvingPeriod before the first provePossession instead of having the first add setup the first sampling.

I can see the case that its a nice optimization to call nextProvingPeriod at the end of provePossession. But I think its better for everyone to keep them decoupled.

ZenGround0 commented 1 month ago

push a few more changes in here after we talked in person.

FYI going off of @magik6k 's suggestion we'll keep initial sampling on addRoots. I think its a small bit worse but I'm practicing listening to users and if it gets too annoying later it will be simple to change.