diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index dce469187b..89464102a6 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -441,7 +441,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { if (!disableClaimCheck && schedule.claimValidityDate != 0) revert ScheduleNotClaimed(); - // Ensure that that the claming date is after the cliff/flow date and before the claim validity date + // Ensure that the claming date is after the cliff/flow date and before the claim validity date if (schedule.cliffAndFlowDate > block.timestamp || _lteDateToExecuteCliffAndFlow(schedule) < block.timestamp) revert TimeWindowInvalid(); diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV3.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV3.sol index 4170be4fc2..d8445d1478 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV3.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV3.sol @@ -760,7 +760,7 @@ contract VestingSchedulerV3 is IVestingSchedulerV3, IRelayRecipient { revert ScheduleNotClaimed(); } - // Ensure that that the claming date is after the cliff/flow date and before the claim validity date + // Ensure that the claming date is after the cliff/flow date and before the claim validity date if (schedule.cliffAndFlowDate > block.timestamp || _lteDateToExecuteCliffAndFlow(schedule) < block.timestamp) { revert TimeWindowInvalid(); } diff --git a/packages/ethereum-contracts/CHANGELOG.md b/packages/ethereum-contracts/CHANGELOG.md index 0905546b6a..7747002294 100644 --- a/packages/ethereum-contracts/CHANGELOG.md +++ b/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`. diff --git a/packages/ethereum-contracts/contracts/agreements/gdav1/GeneralDistributionAgreementV1.sol b/packages/ethereum-contracts/contracts/agreements/gdav1/GeneralDistributionAgreementV1.sol index 97aff72c57..2ecb3e7671 100644 --- a/packages/ethereum-contracts/contracts/agreements/gdav1/GeneralDistributionAgreementV1.sol +++ b/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; diff --git a/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol b/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol index b4faa49280..6b7c33a895 100644 --- a/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol +++ b/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol @@ -289,7 +289,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { Value.unwrap( // PDPoolMemberMU(poolIndex, memberData) PDPoolMemberMU(poolIndexDataToPDPoolIndex(_index), _memberDataToPDPoolMember(memberData)).settle( - Time.wrap(uint32(block.timestamp)) + Time.wrap(SafeCast.toUint32(block.timestamp)) ).m._settled_value ) ); @@ -370,17 +370,14 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { returns (int256 claimableBalance, uint256 timestamp) { timestamp = ISuperfluid(superToken.getHost()).getNow(); - return (getClaimable(memberAddr, uint32(timestamp)), timestamp); + return (getClaimable(memberAddr, SafeCast.toUint32(timestamp)), timestamp); } /// @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, this, memberAddr) + ? int256(0) + : getUnsettledValue(memberAddr, time); } /// @inheritdoc ISuperfluidPool @@ -430,7 +427,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { if (superToken.isPool(GDA, memberAddr)) revert SUPERFLUID_POOL_NO_POOL_MEMBERS(); if (memberAddr == address(0)) revert SUPERFLUID_POOL_NO_ZERO_ADDRESS(); - uint32 time = uint32(ISuperfluid(superToken.getHost()).getNow()); + uint32 time = SafeCast.toUint32(ISuperfluid(superToken.getHost()).getNow()); Time t = Time.wrap(time); Unit wrappedUnits = toSemanticMoneyUnit(newUnits); @@ -443,7 +440,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { PDPoolMemberMU memory mu = PDPoolMemberMU(pdPoolIndex, pdPoolMember); // update pool's disconnected units - if (!superToken.isPoolMemberConnected(GDA, ISuperfluidPool(address(this)), memberAddr)) { + if (!superToken.isPoolMemberConnected(GDA, this, memberAddr)) { _shiftDisconnectedUnits(wrappedUnits - mu.m.owned_units, Value.wrap(0), t); } @@ -459,11 +456,26 @@ 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); - assert(GDA.poolSettleClaim(superToken, memberAddr, (amount))); + function _settle(address memberAddr, uint32 time) internal returns (int256 amount) { + amount = getUnsettledValue(memberAddr, time); + 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) { + // For connected pool, claimable amount is zero; hence, we skip. + if (!superToken.isPoolMemberConnected(GDA, this, memberAddr)) { + amount = _settle(memberAddr, time); + } emit DistributionClaimed(superToken, memberAddr, amount, _membersData[memberAddr].claimedValue); } @@ -475,7 +487,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { /// @inheritdoc ISuperfluidPool function claimAll(address memberAddr) public returns (bool) { bool isConnected = superToken.isPoolMemberConnected(GDA, this, memberAddr); - uint32 time = uint32(ISuperfluid(superToken.getHost()).getNow()); + uint32 time = SafeCast.toUint32(ISuperfluid(superToken.getHost()).getNow()); int256 claimedAmount = _claimAll(memberAddr, time); if (!isConnected) { _shiftDisconnectedUnits(Unit.wrap(0), Value.wrap(claimedAmount), Time.wrap(time)); @@ -492,10 +504,10 @@ 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 = _settle(memberAddr, time); 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)); } diff --git a/packages/ethereum-contracts/ops-scripts/deploy-framework.js b/packages/ethereum-contracts/ops-scripts/deploy-framework.js index f53b38a014..df893c09ec 100644 --- a/packages/ethereum-contracts/ops-scripts/deploy-framework.js +++ b/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(); } diff --git a/packages/ethereum-contracts/test/foundry/FoundrySuperfluidTester.t.sol b/packages/ethereum-contracts/test/foundry/FoundrySuperfluidTester.t.sol index 3dddf60eae..2af3bc4e5a 100644 --- a/packages/ethereum-contracts/test/foundry/FoundrySuperfluidTester.t.sol +++ b/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 { diff --git a/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol b/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol index 9a90aef10c..1512a48d2f 100644 --- a/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol +++ b/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");