search

codex-storage / nim-codex

Decentralized Durability Engine
https://codex.storage
Apache License 2.0
67 stars 25 forks source link

Use "infinite allowance" for collateral deposits #598

Open emizzle opened 1 year ago

emizzle commented 1 year ago

See https://github.com/codex-storage/nim-codex/issues/550#issuecomment-1754481085 for more context.

In an ideal world, it would be best to avoid usage of increaseAllowance so that the potential for using ERC20 stablecoins (eg DAI) that may not have increaseAllowance in their contract is left open, and also to avoid the need to wrap a token (so that it would have increaseAllowance).

To prevent reliance on increaseAllowance, the following changes to Availabilities were proposed:

  1. Change maxCollateral per slot to maxCollateral total across all slots in the Availability
  2. When an Availability is created, approve the maxCollateral. Modify fillSlot so that we don't need to approve the collateral amount.
  3. Add an Availability property maxCollateralPerByte that specifies the maximum amount of collateral per byte a provider is willing to commit to a request. This can be used to prioritise slots in the slot queue.
  4. Unrelated to avoiding increaseAllowance, but was discussed at the same time, when an Availability is created, deposit the maxCollateral into the Marketplace contract. This would act as a commitment from the provider that they are willing to participate.
  5. Future: allow deletion of Availabilities that are not in use (rest endpoint). Any unused collateral can be withdrawn.
  6. Future: allow for a mechanism in which maxCollateral can be topped up for an Availability.
emizzle commented 6 months ago

As an alternative to increaseAllowance, when an availability is created (or edited/patched), we approve infinite token allowance for the entire availability. Infinite token approval is a known attack vector, however, there is a plan to include a separate address for token payouts/profit, which means the storage provider node would be funded separately, and therefore any profits earned by the node would not be vulnerable from an attack.

On PUT/PATCH, if token approval is not infinity, change it to infinite.

AuHau commented 6 months ago

https://github.com/codex-storage/codex-contracts-eth/issues/71 should be implemented first.

emizzle commented 4 weeks ago

I'm not sure that approving the totalCollateral when an Availability is added/edited will be enough to get rid of increaseAllowance. Instead, there could be an approve call with totalCollateral of all Availabilities minus collateral that's already been spent in active requests.

Imo, this is safer than using infinite approval.