diff --git a/allowances/README.md b/allowances/README.md index fd9db984a..7941ba53f 100644 --- a/allowances/README.md +++ b/allowances/README.md @@ -1,18 +1,20 @@ # Allowance module -This contract is a registry of transfer allowances that can be used by specific accounts. For this the contract needs to be enabled as a module on the Safe that holds the assets that should be transfered. The registry is written to be used with ERC20 tokens and Ether (represented by the zero address). +This contract is a registry of transfer allowances on a Safe that can be used by specific accounts. For this the contract needs to be enabled as a module on the Safe that holds the assets that should be transferred. The registry is written to be used with ERC20 tokens and Ether (represented by the zero address). -All transfer allowance are specific to a Safe, token and delegate. A delegate is an account that can authorize transfers (via a signature). +All transfer allowances are specific to a Safe, token and delegate. A delegate is an account that can authorize transfers (via a signature). + +Note: This is not about allowances on an ERC20 token contract. ## Setting up allowances The contract is designed as a single point registry. This way not every Safe needs to deploy their own module and it is possible that this module is shared between different Safes. -To set an allowance for a Safe it is first require that the Safe add a **delegate**. For this a Safe transaction needs to be executed that calls `addDelegate`. This method will add the specified **delegate** for `msg.sender`, which is the Safe in case of a Safe transaction. The `addDelegate` method can be called multiple times with the same address for a **delegate** without failure. +To set an allowance for a Safe it is first required that the Safe adds a **delegate**. For this a Safe transaction needs to be executed that calls `addDelegate`. This method will add the specified **delegate** for `msg.sender`, which is the Safe in case of a Safe transaction. The `addDelegate` method can be called multiple times with the same address for a **delegate** without failure. Once a **delegate** has been enabled it is possible to set an allowance for that **delegate**. For this a Safe transaction needs to be executed that calls `setAllowance`. -To delete an allowance for a specific token a Safe transactions needs to be executed that calls `deleteAllowance`. Another way is to remove the **delegate** with a Safe transaction that calls `removeDelegate`, but this will remove all allowances for this delegate. +To delete an allowance for a specific token a Safe transaction needs to be executed that calls `deleteAllowance`. Another way is to remove the **delegate** with a Safe transaction that calls `removeDelegate`, but this will remove all allowances for this delegate. Note: calling `setAllowance` will not change the `spent` value of an allowance. To reset the `spent` value you need to call `resetAllowance`. @@ -50,13 +52,13 @@ The allowance module signatures are EIP-712 based. And uses the following scheme Anyone can execute the transfer as long as they provide the [transfer authorization](#transfer-authorization) of a valid **delegate**. To execute the transfer the `executeAllowanceTransfer` method needs to be called, that checks the signature and calls the Safe to perform the transfer. -When calling the Safe there are two cases that need to be differentiated: Is it an Ether transfer, or is it a token transfer. +When calling the Safe there are two cases that need to be differentiated: an Ether transfer, and a token transfer. - In case of an **Ether transfer** the module instructs the Safe to transfer the specified amount to the receiver (so no data is provided). - In case of a **token transfer** the module instructs the Safe to call the transfer method on the token contract to transfer the specified amount to the receiver (so a value of `0` is provided). ## One time allowances -If the `resetTimeMin` time of an allowance is `0` then the allowance will not automatically renew. This means when the `delegate` has used up the allowance it is necessary "reset" the allowance by performing a Safe transaction that calls `resetAllowance`. +If the `resetTimeMin` time of an allowance is `0` then the allowance will not automatically renew. This means when the `delegate` has used up the allowance it is necessary to "reset" the allowance by performing a Safe transaction that calls `resetAllowance`. ## Recurring allowances If `resetTimeMin` is set to any value greater than `0` the allowance will automatically renew after the specified amount of minutes based on the last reset time. The initial reset time can be specified with `resetBaseMin`. This can be used to set the time of day the allowance is renewed. @@ -76,13 +78,13 @@ It is possible to define a refund that is paid to `tx.origin`. This refund is al The contract has been designed to improve the gas usage when executing a transfer and to make it easy to interact with the contract. Because of this configuration changes (e.g. adding or removing allowance and delegates) are not optimized on gas usage. -Allowances are store in a map called `allowances`. The key to this map is a combination of the Safe, the delegate and the token. Each allowance has the following fields: +Allowances are stored in a map called `allowances`. The key to this map is a combination of the Safe, the delegate and the token. Each allowance has the following fields: - `uint96 amount` - Maximum amount that can be spent - `uint96 spent` - Amount that has already been spent - `uint16 resetTimeMin` - - Time span in **minutes** after which the `spent` amount will automatically reset to `0`. If `resetTimeMin` is `0` the `spent` value will not automaticall reset. + - Time span in **minutes** after which the `spent` amount will automatically reset to `0`. If `resetTimeMin` is `0` the `spent` value will not automatically reset. - `uint32 lastResetMin` - Point in time in **minutes** when the `spent` amount was last reset to `0`. This can be used to specify the point in time (e.g. time of a day) when the reset should happen, as each following reset will be based on last time. - `uint16 nonce` @@ -92,11 +94,11 @@ Some limits have been applied to the different fields to make sure that the over - The maximum allowance is `2^96`. Assuming a token with 18 decimals that would allow an allowance from more than 70 billion, which seems sufficient. - The maximum time span that can be used for the reset is `2**16` minutes. This corresponds to around 45 days, which is more than one month and should be sufficient for most use cases. - The maximum reset time would be the unix epoch of `(2**32) * 60`. This is in the year 10136. -- The maximum amount of transfer a delegate can do is `2**16` (aka `65536`) which should be sufficient, as it is easily possible to register a new delegate. +- The maximum numbers of transfers a delegate can do is `2**16` (aka `65536`) which should be sufficient, as it is easily possible to register a new delegate. To make it easier to allow querying for allowances two more data structures have been added to the contract: `delegates` and `tokens`. -`delegates` stores all the delegates for a specific Safe in a double linked list. For this we also store an entry point to that double linked list in `delegatesStart`. Each delegate is identified by a `uint48` which is the first **6bytes** of the delegate address. Because of this is can theoretically come to collisions. As the index points to a struct containing the `address` of the delgate, the `next` index and the `prev` index, it is possible to verify which address was used to get the index. In case of collisions we recommend to generate a new delegate. +`delegates` stores all the delegates for a specific Safe in a double linked list. For this we also store an entry point to that double linked list in `delegatesStart`. Each delegate is identified by a `uint48` which is the first **6bytes** of the delegate address. This could theoretically cause collisions. Therfore the index points to a struct containing the `address` of the delgate, the `next` index and the `prev` index, so that it is possible to verify which address was used to get the index. In case of collisions we recommend to generate a new delegate. `tokens` is a list that is appended when ever an allowance is set for a token for the first time. The tokens will never be removed from this list. @@ -105,7 +107,7 @@ Using `delegates` and `tokens` it is possible to query all available allowances ## Considerations - If storage rent is introduced for contracts it might be possible (depending on the rent implementation) to perform a grieving attack, as anyone can register allowances. -- The information provided by `delegates` and `tokens` could be rebuild using event and therefore these would not be necessary in the contract (this would make some consistency checks unnecessary and save gas). +- The information provided by `delegates` and `tokens` could be rebuilt using events and therefore these would not be necessary in the contract (this would make some consistency checks unnecessary and save gas). - The bytes assigned to different parts of the `Allowance` struct could be optimized. ## Running tests diff --git a/allowances/contracts/AlowanceModule.sol b/allowances/contracts/AlowanceModule.sol index fd19c3c77..a38ea77a0 100644 --- a/allowances/contracts/AlowanceModule.sol +++ b/allowances/contracts/AlowanceModule.sol @@ -102,7 +102,7 @@ contract AllowanceModule is SignatureDecoder { // Check if we should reset the time. We do this on load to minimize storage read/ writes if (allowance.resetTimeMin > 0 && allowance.lastResetMin <= currentMin - allowance.resetTimeMin) { allowance.spent = 0; - // Resets happen in regular itervals and `lastResetMin` should be aligned to that + // Resets happen in regular intervals and `lastResetMin` should be aligned to that allowance.lastResetMin = currentMin - ((currentMin - allowance.lastResetMin) % allowance.resetTimeMin); } return allowance; @@ -141,7 +141,7 @@ contract AllowanceModule is SignatureDecoder { /// @param safe The Safe whose funds should be used. /// @param token Token contract address. /// @param to Address that should receive the tokens. - /// @param amount Amount to should be transfered. + /// @param amount Amount that should be transferred. /// @param paymentToken Token that should be used to pay for the execution of the transfer. /// @param payment Amount to should be paid for executing the transfer. /// @param delegate Delegate whose allowance should be updated.