Open hilmarx opened 4 years ago
Vulnerability with Actions []. Out of Gas reverts! If user can combine whitelisted actions arbitrary, they can result in the calls to consume more than 6M gas, which will lead to a revert and a payment for the provider, but without any fees. Note this is a grieving vector, as users still have to pay for minting the claim in the first place.
Resulting Q: Do we need to whitelist conditions?
I think we still do because there could be buggy conditions that eat up all the gas
and cause a revert refund
payable by provider.
Summary:
Identified Problems with Conditions[] Actions[] approach:
Non-trivial to make work with current provider feature, which requires every provider to whitelist conditions and actions individually. ActionConditionsOk core functionality is to ensure that Tx will only get executed if all requirements of the provider are fulfilled. These requirements, that go beyond the actual condition, are mostly action specific (e.g. ERC20 allowance, enough liquidity in Lending protocol, etc). Decoupling those makes it hard for providers to ensure that all their requirements are met before an action is executed.
The problem of whitelisting is more concerned with actions, rather than conditions, because the actions define what kind of basic requirements need to be met in order for the action to be able to be executed. These requirements are mostly those, which ensure that sufficient funds are moved through the action, to compensate the provider with a fee for paying executors.
Resulting Q: Do we need to whitelist conditions? Conditions only define a certain case when to execute a certain action. This case is more relevant for the user, not the executor. The action on the other hand contains a) the logic which defines how much money the provider will receive and b) what requirements must be fulfilled in order for the provider to be sure that the requirements are met for transaction execution. ActionConditionsOk should always be written in such a way, that they are completely sell-sufficient, hence they will ensure providers get compensated adequately for their service of providing ETH prepayments.
Possible Solution:
Providers pay for users to execute an action. Hence they want to be sure that certain requirements (i.e. terms of service) are met beforehand. To ensure composability, the actions will have to be self-sufficient (aka be able to pay for themselves without the presence of any other action). This means, we can still enable
exex()
to accept anActions[ ]
array, however we would have to make each ActionConditionsOkCheck individually, possibly doing some (like ERC20 balance check) more than once.