Merge pull request #66 from CirclesUBI/20240124-global-allowance

20240124 global allowance

specifications/TCIP007-global-allowance.md

@@ -1,80 +1,15 @@
-# (Reflections on) Global Allowance -- MUST BE REWRITTEN
+# Global Allowance
 
-Every Circle node implements the ERC20 interface. As part of the ERC20 interface
-allowance enables external contracts to interact with the token balance of owners.
+## Simple specification
 
-In Circles under normal behaviour of the protocol an avatar accumulates balances
-in the Circle nodes of avatars they have trusted. It therefore becomes cumbersome
-to manually set an allowance for each Circle node that one might have a balance on.
+Global Allowance solves for the "ancillary extension" of the Circles protocol.
 
-A critical part of the Circles protocol pertains to the path transfer of Circles
-over the flow graph induced by the trust graph.
-The conservative implementation requires the avatar to initiate the path transfer,
-but this limits how the protocol can interact with new methods that have been developed, in particular with an intent-based architecture.
+Circles should allow other contracts and protocols to easily operate on the Circle balances (personal and group) a person has. For many use-cases the known {ERC20-transferFrom} suffices by setting the allowance through approval.
 
-In an intent-based architecture the owner can sign final constraints they request
-to hold after the execution of a transaction, while leaving open the details of what transaction might achieve the requested end-state for them.
+However a person may have many balances across the graph, so rather than setting the approval for every node of the graph, we can extend the concept from the local ERC20 contracts to a global allowance that is valid for all ERC20 contracts across the graph.
 
-With the proposal of a global allowance we want to enable the Circles protocol
-to be compatible with such extensions, whether an intent-based path solver network
-for Circles directly, or compatibility with other external protocols.
+Upon extending the concept, we must choose how to integrate the two values (local and global) of the allowance variable (per ERC20 Circles contract). There are two obvious paths: additive or overriding.
 
-## Specification
+Overriding most respects the callers intent, but still leaves open the question: what is the variable that decides the override. A first proposal would be that a non-zero local allowance overrides any global allowance. However, this also has a sharp edge: once the local allowance is spent, the global allowance could take effect, which would violate most developer expectations.
 
-With `global allowance` the `Graph` contract holds an additional allowance
-value for each (`owner`, `spender`) pair which then apply to all Circle node ERC20
-contracts registered in this graph for the balance of said `owner`.
-
-When trying to generalize the allowance across all Circle node contracts we need
-to resolve the duplicated state, as there is an allowance kept globally and locally for each Circle node.
-
-We propose that upon spending the allowance with `transferFrom` in a Circle node
-the global an local allowance are additive. The spending should be continuous so a call will succeed if a sufficient combined allowance is present.
-Thirdly the global allowance is depleted first, afterwards the local allowance is spent.
-
-The global allowance should leave the local ERC20 behaviour of allowances intact.
-Because we want to the spending to be continuous, the `allowance(owner, spender)`
-call should return the summed global and local allowance for that contract.
-However, we inevitably have slight breakages by introducing a dual state:
-upon calling `approve(spender, value)` on the Circle node, the contract should
-set the local allowance value. By calling `approve(spender, value)` on the graph
-contract, it should set the global value of the allowance.
-
-Allowance is often extended with `decreaseAllowance` and `increaseAllowance`
-which should simply still act on the local allowance value. We do not recommend to
-implement a global `decreaseAllowance` and `increaseAllowance` on the graph contract, but this self-evidently be opted for.
-
-## Expected usage
-
-We expect the global allowance to be used for people to opt-in to novel protocol
-extensions such as the intent-architecture as explained. We therefore expect the default global allowance to be set to the maximum value of `uint256`, effectively infinity.
-
-As desribed above, the obvious extension of the allowance to a global scope is
-by duplicating the state value of a global and local allowance. However, upon
-examining the possible implementations it is apparent the required logic requires
-a higher branching complexity to spend across global and local allowance values.
-
-We can therefore consider the global allowance to be a boolean value, rather than
-an integer allowance amount. By separating the types, there is no longer
-a duplication of state across the local and global contract state; the global
-allowance now means an unlimited spending ability for the allowed protocol,
-while enabled.
-
-This reduces complexity of understanding the global allowance for consumers
-of the interface. It resolves questions on the ambiguous behaviour of
-`allowance()`.
-
-Most importantly it actually matches the intended behaviour when setting the
-(global) allowance to `max_uint256` for a trusted protocol contract.
-
-We therefore propose instead this binary implementation for global allowance.
-
-## Time scales for disabling a global allowance
-
-Opting for the binary implementation of the global allowance, additionally
-opens the opportunity to delay the disallowance of a protocol contract at the global level, with the same time delay as untrusting an avatar has.
-
-We implemented such a time-delay to allow synchronization between the execution
-of protocol contracts (in particular a solver network), and the state changes in
-the contract state. For an integer global allowance such a time delay would be
-difficult to sensibly construct.
+To resolve this we propose to track the timestamp when an allowance, local or global is set, and the most recent value overrides the allowance. If both local and global allowance is set in the same block (resulting in the same timestamp), then the local allowance overrides the global allowance.

src/circles/GroupCircle.sol

@@ -11,8 +11,6 @@ import "../graph/IGraph.sol";
 contract GroupCircle is MasterCopyNonUpgradable, TemporalDiscount, IGroupCircleNode {
     // State variables
 
-    IGraph public graph;
-
     // todo: we probably want group to have an interface so that we can call hooks on it
     IGroup public group;
 
@@ -41,10 +39,7 @@ contract GroupCircle is MasterCopyNonUpgradable, TemporalDiscount, IGroupCircleN
         _;
     }
 
-    constructor() {
-        // block setup on the master copy deployment of the group circle
-        graph = IGraph(address(1));
-    }
+    constructor() TemporalDiscount() {}
 
     // External functions
 

src/circles/TemporalDiscount.sol

@@ -2,6 +2,7 @@
 pragma solidity >=0.8.13;
 
 import "../lib/Math64x64.sol";
+import "../graph/IGraph.sol";
 import "./IERC20.sol";
 
 contract TemporalDiscount is IERC20 {
@@ -59,6 +60,8 @@ contract TemporalDiscount is IERC20 {
 
     // State variables
 
+    IGraph public graph;
+
     /**
      * Creation time stores the time this time circle node was created
      */
@@ -89,7 +92,14 @@ contract TemporalDiscount is IERC20 {
      */
     uint256 private totalSupplyTime;
 
-    mapping(address => mapping(address => uint256)) private allowances;
+    mapping(address => mapping(address => uint256)) public allowances;
+
+    /**
+     * Allowance timestamps track when local allowances are set, as to compare
+     * with the timestamp of the global allowance, so that the most recent setting
+     * takes precedence.
+     */
+    mapping(address => mapping(address => uint256)) public allowanceTimestamps;
 
     // Events
 
@@ -106,6 +116,13 @@ contract TemporalDiscount is IERC20 {
 
     event Approval(address indexed owner, address indexed spender, uint256 amount);
 
+    // Constructor
+
+    constructor() {
+        // block the mastercopy from getting called setup on
+        graph = IGraph(address(1));
+    }
+
     // External functions
 
     function transfer(address _to, uint256 _amount) external returns (bool) {
@@ -114,8 +131,15 @@ contract TemporalDiscount is IERC20 {
     }
 
     function transferFrom(address _from, address _to, uint256 _amount) external returns (bool) {
-        uint256 spentAllowance = allowances[_from][msg.sender] - _amount;
-        _approve(_from, msg.sender, spentAllowance);
+        if (localOverGlobalAllowance(_from, msg.sender)) {
+            // spend the local allowance for spender
+            uint256 remainingAllowance = allowances[_from][msg.sender] - _amount;
+            allowances[_from][msg.sender] = remainingAllowance;
+        } else {
+            // or spend from the global allowance instead
+            graph.spendGlobalAllowance(_from, msg.sender, _amount);
+        }
+
         _transfer(_from, _to, _amount);
         return true;
     }
@@ -126,7 +150,13 @@ contract TemporalDiscount is IERC20 {
     }
 
     function allowance(address _owner, address _spender) external view returns (uint256 remaining) {
-        return allowances[_owner][_spender];
+        if (localOverGlobalAllowance(_owner, _spender)) {
+            remaining = allowances[_owner][_spender];
+        } else {
+            remaining = graph.globalAllowances(_owner, _spender);
+        }
+
+        return remaining;
     }
 
     function increaseAllowance(address _spender, uint256 _incrementAmount) external returns (bool) {
@@ -187,9 +217,11 @@ contract TemporalDiscount is IERC20 {
     }
 
     function _approve(address _owner, address _spender, uint256 _amount) internal {
-        require(address(_spender) != address(0), "Spender for approval must not be zero address.");
+        require(_spender != address(0), "Spender for approval must not be zero address.");
 
         allowances[_owner][_spender] = _amount;
+        allowanceTimestamps[_owner][_spender] = block.timestamp;
+
         emit Approval(_owner, _spender, _amount);
     }
 
@@ -338,4 +370,19 @@ contract TemporalDiscount is IERC20 {
         // return the discounted balance
         discountedBalance_ = Math64x64.mulu(reduction64x64, _balance);
     }
+
+    // Private functions
+
+    function localOverGlobalAllowance(address _from, address _spender) private view returns (bool) {
+        uint256 globalAllowanceTimestamp = graph.globalAllowanceTimestamps(_from, _spender);
+        uint256 localAllowanceRTimestamp = allowanceTimestamps[_from][_spender];
+
+        if (globalAllowanceTimestamp > localAllowanceRTimestamp) {
+            // global allowance takes precedence if it strictly set more recently
+            return false;
+        } else {
+            // default to local allowance
+            return true;
+        }
+    }
 }

src/circles/TimeCircle.sol

@@ -35,8 +35,6 @@ contract TimeCircle is MasterCopyNonUpgradable, TemporalDiscount, IAvatarCircleN
 
     // State variables
 
-    IGraph public graph;
-
     address public avatar;
 
     bool public stopped;
@@ -78,10 +76,7 @@ contract TimeCircle is MasterCopyNonUpgradable, TemporalDiscount, IAvatarCircleN
 
     // Constructor
 
-    constructor() {
-        // block the mastercopy from getting called setup on
-        graph = IGraph(address(1));
-    }
+    constructor() TemporalDiscount() {}
 
     // External functions
 
@@ -114,6 +109,13 @@ contract TimeCircle is MasterCopyNonUpgradable, TemporalDiscount, IAvatarCircleN
         lastIssued = block.timestamp;
     }
 
+    /**
+     * Path transfer is only accessible by the graph contract
+     * to move circles along the flow graph induced from the balances
+     * and trust relations.
+     * Graph operators can also act as a core extension
+     * over the authorized flow subgraph to access pathTransfer.
+     */
     function pathTransfer(address _from, address _to, uint256 _amount) external onlyGraph {
         _transfer(_from, _to, _amount);
     }

src/graph/Graph.sol

@@ -151,6 +151,16 @@ contract Graph is ProxyFactory, IGraph {
      */
     mapping(address => mapping(address => uint256)) public globalAllowances;
 
+    /**
+     * Global allowance timestamps tracks the timestamp when the global allowance was set.
+     * If the global allowance timestamp is more recent than a local (at an ERC20 CircleNode)
+     * timestamp of when the local allowance was last set, then the global allowance overrides
+     * the local allowance value.
+     * When the timestamps of local and global allowance would be equal (eg. set in the same block)
+     * then the local allowance overrides the global allowance value.
+     */
+    mapping(address => mapping(address => uint256)) public globalAllowanceTimestamps;
+
     // Events
 
     event RegisterAvatar(address indexed avatar, address circleNode);
@@ -314,6 +324,44 @@ contract Graph is ProxyFactory, IGraph {
         emit RevokedGraphOperator(msg.sender, _operator, earliestExpiry);
     }
 
+    /**
+     * Approve sets the global allowance for all Circle balances held by the caller.
+     * The global allowance overrides any local allowances set (in the individual ERC20 contracts)
+     * if it is called after an allowance for the same spender was set locally.
+     * Conversely, if after setting a global allowance a local allowance value is set
+     * in the ERC20 Circle contract, then that allowance overrides this global allowance.
+     * If both local and global allowances are set in the same block, then the local allowance
+     * overrides this global allowance.
+     */
+    function approve(address _spender, uint256 _amount) external returns (bool) {
+        require(_spender != address(0), "Spender for global approval must not be zero address.");
+
+        globalAllowances[msg.sender][_spender] = _amount;
+        // update the timestamp to know whether global or local allowance takes priority
+        globalAllowanceTimestamps[msg.sender][_spender] = block.timestamp;
+
+        emit GlobalApproval(msg.sender, _spender, _amount);
+
+        return true;
+    }
+
+    function spendGlobalAllowance(address _entity, address _spender, uint256 _amount) external {
+        // only a registered personal or group Circle contract can call this function
+        // to spend from the global allowance
+        require(
+            address(avatarCircleNodesIterable[ICircleNode(msg.sender)]) != address(0)
+                || address(groupCircleNodesIterable[ICircleNode(msg.sender)]) != address(0),
+            "Only a registered Circle node can call to spend global allowance."
+        );
+
+        // note that any registered Circle node can spend from the global allowance
+        // of the _entity, so msg.sender is not used for the state update
+        uint256 remainingGlobalAllowance = globalAllowances[_entity][_spender] - _amount;
+        globalAllowances[_entity][_spender] = remainingGlobalAllowance;
+
+        // note to not update the timestamp from the global allowance as it gets spent.
+    }
+
     function migrateCircles(address _owner, uint256 _amount, IAvatarCircleNode _circle)
         external
         onlyAncestorMigrator

src/graph/IGraph.sol

@@ -4,6 +4,10 @@ pragma solidity >=0.8.13;
 import "./ICircleNode.sol";
 
 interface IGraph {
+    function spendGlobalAllowance(address entity, address spender, uint256 amount) external;
+    function globalAllowances(address entity, address spender) external view returns (uint256);
+    function globalAllowanceTimestamps(address entity, address spender) external view returns (uint256 timestamp);
+
     function avatarToCircle(address avatar) external view returns (IAvatarCircleNode);
 
     function checkAllAreTrustedCircleNodes(address group, ICircleNode[] calldata circles, bool includeGroups)