[ETHEREUM-CONTRACTS] distinguish between unsettled and claimable balance in pools

packages/ethereum-contracts/CHANGELOG.md

@@ -11,6 +11,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   - `GDAv1StorageReader` contains getters reading agreement data from the token contract, allowing contracts to get this data without making a call to the GDA contract.
   - `GDAv1StorageWriter` contains functions for writing agreement data to the token contract. This can only be used by the GDA contract itself.
 
+### Fixed
+- `ISuperfluidPool`: `getClaimable` and `getClaimableNow` could previously return non-zero values for connected pools, which was inconsistent with what `claimAll` would actually do in this situation (claim nothing).
+
 ### Breaking
 - PoolMemberNFT pruning: `IPoolMemberNFT` and `PoolMemberNFT` removed, `POOL_MEMBER_NFT()` removed from `ISuperToken`.
 

packages/ethereum-contracts/contracts/agreements/gdav1/GeneralDistributionAgreementV1.sol

@@ -89,7 +89,7 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi
             for (uint256 i = 0; i < slotIds.length; ++i) {
                 address pool = address(uint160(uint256(pidList[i])));
                 _assertPoolConnectivity(token, account, ISuperfluidPool(pool));
-                totalConnectedFromPools += ISuperfluidPool(pool).getClaimable(account, uint32(time));
+                totalConnectedFromPools += SuperfluidPool(pool).getUnsettledValue(account, uint32(time));
             }
         }
         rtb += totalConnectedFromPools;

packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol

@@ -375,12 +375,9 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
 
     /// @inheritdoc ISuperfluidPool
     function getClaimable(address memberAddr, uint32 time) public view override returns (int256) {
-        Time t = Time.wrap(time);
-        PDPoolIndex memory pdPoolIndex = poolIndexDataToPDPoolIndex(_index);
-        PDPoolMember memory pdPoolMember = _memberDataToPDPoolMember(_membersData[memberAddr]);
-        return Value.unwrap(
-            PDPoolMemberMU(pdPoolIndex, pdPoolMember).rtb(t) - Value.wrap(_membersData[memberAddr].claimedValue)
-        );
+        return superToken.isPoolMemberConnected(GDA, ISuperfluidPool(address(this)), memberAddr)
+             ? int256(0)
+             : getUnsettledValue(memberAddr, time);
     }
 
     /// @inheritdoc ISuperfluidPool
@@ -459,10 +456,23 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
         emit MemberUnitsUpdated(superToken, memberAddr, oldUnits, newUnits);
     }
 
-    function _claimAll(address memberAddr, uint32 time) internal returns (int256 amount) {
-        amount = getClaimable(memberAddr, time);
+    function _settle(address memberAddr, int256 amount) internal {
         assert(GDA.poolSettleClaim(superToken, memberAddr, (amount)));
         _membersData[memberAddr].claimedValue += amount;
+    }
+
+    function getUnsettledValue(address memberAddr, uint32 time) public view returns (int256) {
+        Time t = Time.wrap(time);
+        PDPoolIndex memory pdPoolIndex = poolIndexDataToPDPoolIndex(_index);
+        PDPoolMember memory pdPoolMember = _memberDataToPDPoolMember(_membersData[memberAddr]);
+        return Value.unwrap(
+            PDPoolMemberMU(pdPoolIndex, pdPoolMember).rtb(t) - Value.wrap(_membersData[memberAddr].claimedValue)
+        );
+    }
+
+    function _claimAll(address memberAddr, uint32 time) internal returns (int256 amount) {
+        amount = getClaimable(memberAddr, time);
+        _settle(memberAddr, amount);
 
         emit DistributionClaimed(superToken, memberAddr, amount, _membersData[memberAddr].claimedValue);
     }
@@ -492,10 +502,11 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
 
     // WARNING for operators: it is undefined behavior if member is already connected or disconnected
     function operatorConnectMember(address memberAddr, bool doConnect, uint32 time) external onlyGDA returns (bool) {
-        int256 claimedAmount = _claimAll(memberAddr, time);
+        int256 settleAmount = getUnsettledValue(memberAddr, time);
+        _settle(memberAddr, settleAmount);
         int128 units = uint256(_getUnits(memberAddr)).toInt256().toInt128();
         if (doConnect) {
-            _shiftDisconnectedUnits(Unit.wrap(-units), Value.wrap(claimedAmount), Time.wrap(time));
+            _shiftDisconnectedUnits(Unit.wrap(-units), Value.wrap(settleAmount), Time.wrap(time));
         } else {
             _shiftDisconnectedUnits(Unit.wrap(units), Value.wrap(0), Time.wrap(time));
         }

packages/ethereum-contracts/ops-scripts/deploy-framework.js

@@ -996,7 +996,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
                 const poolMemberNFTPAddr = await superTokenLogic.POOL_MEMBER_NFT();
                 let poolMemberNFTLAddr = ZERO_ADDRESS;
                 if (poolMemberNFTPAddr !== ZERO_ADDRESS) {
-                    const poolMemberNFTContract = await PoolMemberNFT.at(poolMemberNFTPAddr);
+                    const poolMemberNFTContract = await UUPSProxiable.at(poolMemberNFTPAddr);
                     poolMemberNFTLAddr = await poolMemberNFTContract.getCodeAddress();
                 }
 

packages/ethereum-contracts/test/foundry/FoundrySuperfluidTester.t.sol

@@ -1271,6 +1271,21 @@ contract FoundrySuperfluidTester is Test {
         // _assertRealTimeBalances(ISuperToken(address(poolSuperToken)));
     }
 
+    function _helperClaimAll(ISuperfluidPool pool_, address caller_, address member_) internal {
+        (int256 claimableBefore,) = pool_.getClaimableNow(member_);
+        (int256 balanceBefore,,,) = pool_.superToken().realtimeBalanceOfNow(member_);
+
+        vm.startPrank(caller_);
+        pool_.claimAll(member_);
+        vm.stopPrank();
+
+        (int256 claimableAfter,) = pool_.getClaimableNow(member_);
+        (int256 balanceAfter,,,) = pool_.superToken().realtimeBalanceOfNow(member_);
+
+        assertEq(claimableAfter, 0, "GDAv1.t: Member claimable amount should be 0");
+        assertEq(balanceAfter, balanceBefore + claimableBefore, "GDAv1.t: Member balance should increase by claimable amount");
+    }
+
     function _helperConnectPool(address caller_, ISuperToken superToken_, ISuperfluidPool pool_, bool useForwarder_)
         internal
     {

packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol

@@ -940,6 +940,52 @@ contract GeneralDistributionAgreementV1IntegrationTest is FoundrySuperfluidTeste
         _helperSuperfluidPoolUnitsTransferFrom(freePool, spender, owner, spender, uint256(uint128(transferAmount)));
     }
 
+    function testGetClaimable(
+        address member,
+        uint128 units,
+        uint64 distributionAmount,
+        bool useForwarder,
+        PoolConfig memory config
+    ) public {
+        vm.assume(member != address(0));
+        vm.assume(member != address(freePool));
+        vm.assume(units > 0);
+        vm.assume(distributionAmount > 0);
+        vm.assume(units < distributionAmount);
+        vm.assume(distributionAmount < type(uint128).max);
+
+        // Create a pool for testing
+        ISuperfluidPool pool = _helperCreatePool(superToken, alice, alice, useForwarder, config);
+        _addAccount(member);
+
+        // Step 1: Assign units to disconnected member
+        _helperUpdateMemberUnits(pool, alice, member, units, _StackVars_UseBools({useForwarder: useForwarder, useGDA: false}));
+
+        (int256 balanceBefore,,,) = superToken.realtimeBalanceOfNow(member);
+        (int256 claimableBefore,) = pool.getClaimableNow(member);
+
+        // Distribute
+        _helperDistributeViaGDA(superToken, alice, alice, pool, distributionAmount, useForwarder);
+
+        // Check disconnected member: expect balance unchanged, claimable increased
+        (int256 balanceAfter1,,,) = superToken.realtimeBalanceOfNow(member);
+        (int256 claimableAfter1,) = pool.getClaimableNow(member);
+
+        assertEq(balanceAfter1, balanceBefore, "Disconnected member balance should not change");
+        assertTrue(claimableAfter1 > claimableBefore, "Disconnected member claimable amount should increase");
+
+        // Step 2: Connect member and distribute again
+        _helperConnectPool(member, superToken, pool, useForwarder);
+        _helperDistributeViaGDA(superToken, alice, alice, pool, distributionAmount, useForwarder);
+
+        (int256 balanceAfter2,,,) = superToken.realtimeBalanceOfNow(member);
+        (int256 claimableAfter2,) = pool.getClaimableNow(member);
+
+        // Check connected member: balance increased, claimable remains 0
+        assertTrue(balanceAfter2 > balanceAfter1, "Connected member balance should increase");
+        assertEq(claimableAfter2, 0, "Connected member claimable amount should be 0");
+    }
+
     /*//////////////////////////////////////////////////////////////////////////
                                     Assertion Functions
     //////////////////////////////////////////////////////////////////////////*/
@@ -983,9 +1029,7 @@ contract GeneralDistributionAgreementV1IntegrationTest is FoundrySuperfluidTeste
                 address u4 = TEST_ACCOUNTS[1 + (s.v % N_MEMBERS)];
                 emit log_named_string("action", "claimAll");
                 emit log_named_address("claim for", u4);
-                vm.startPrank(user);
-                assert(freePool.claimAll(u4));
-                vm.stopPrank();
+                _helperClaimAll(freePool, user, u4);
             } else if (action == 3) {
                 bool doConnect = s.v % 2 == 0 ? false : true;
                 emit log_named_string("action", "doConnectPool");