From ba1b0674aba6c0297d826ca326b57678d3db4901 Mon Sep 17 00:00:00 2001 From: didi Date: Thu, 26 Jun 2025 16:34:40 +0200 Subject: [PATCH 1/8] distinguish between unsettled and claimable balance in pools --- .../gdav1/GeneralDistributionAgreementV1.sol | 2 +- .../agreements/gdav1/SuperfluidPool.sol | 31 +++++++++---- .../gdav1/GeneralDistributionAgreement.t.sol | 46 +++++++++++++++++++ 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/packages/ethereum-contracts/contracts/agreements/gdav1/GeneralDistributionAgreementV1.sol b/packages/ethereum-contracts/contracts/agreements/gdav1/GeneralDistributionAgreementV1.sol index c7c02135ff..a7a7ed69a7 100644 --- a/packages/ethereum-contracts/contracts/agreements/gdav1/GeneralDistributionAgreementV1.sol +++ b/packages/ethereum-contracts/contracts/agreements/gdav1/GeneralDistributionAgreementV1.sol @@ -142,7 +142,7 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi _getPoolMemberData(token, account, ISuperfluidPool(pool)); assert(exist); assert(poolMemberData.pool == pool); - fromPools += ISuperfluidPool(pool).getClaimable(account, uint32(time)); + fromPools += SuperfluidPool(pool).getUnsettledValue(account, uint32(time)); } } rtb += fromPools; diff --git a/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol b/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol index 60d586a76c..4bbb820f4f 100644 --- a/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol +++ b/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol @@ -374,12 +374,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 GDA.isMemberConnected(ISuperfluidPool(address(this)), memberAddr) + ? int256(0) + : getUnsettledValue(memberAddr, time); } /// @inheritdoc ISuperfluidPool @@ -478,10 +475,23 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { _handlePoolMemberNFT(memberAddr, 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); } @@ -511,10 +521,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)); } 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 f7bc8156e7..8b453103a0 100644 --- a/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol +++ b/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol @@ -906,6 +906,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 //////////////////////////////////////////////////////////////////////////*/ From f76ce16b6df8d9415002a9301130aa585da82baf Mon Sep 17 00:00:00 2001 From: didi Date: Thu, 26 Jun 2025 16:43:40 +0200 Subject: [PATCH 2/8] add assertions --- .../test/foundry/FoundrySuperfluidTester.t.sol | 15 +++++++++++++++ .../gdav1/GeneralDistributionAgreement.t.sol | 4 +--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/ethereum-contracts/test/foundry/FoundrySuperfluidTester.t.sol b/packages/ethereum-contracts/test/foundry/FoundrySuperfluidTester.t.sol index b289cace21..7ad994aee7 100644 --- a/packages/ethereum-contracts/test/foundry/FoundrySuperfluidTester.t.sol +++ b/packages/ethereum-contracts/test/foundry/FoundrySuperfluidTester.t.sol @@ -1278,6 +1278,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 8b453103a0..5415d34182 100644 --- a/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol +++ b/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol @@ -995,9 +995,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"); From 9c433c503f23328b393ec553c20bdfc5d5f87c3d Mon Sep 17 00:00:00 2001 From: didi Date: Tue, 15 Jul 2025 18:12:29 +0200 Subject: [PATCH 3/8] use new reader function --- .../contracts/agreements/gdav1/SuperfluidPool.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol b/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol index 612d353594..472bcd2160 100644 --- a/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol +++ b/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol @@ -375,7 +375,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { /// @inheritdoc ISuperfluidPool function getClaimable(address memberAddr, uint32 time) public view override returns (int256) { - return GDA.isMemberConnected(ISuperfluidPool(address(this)), memberAddr) + return superToken.isPoolMemberConnected(GDA, ISuperfluidPool(address(this)), memberAddr) ? int256(0) : getUnsettledValue(memberAddr, time); } From d6687abec81bfa8f19e6ebd3c55b7a6fb59b4d42 Mon Sep 17 00:00:00 2001 From: didi Date: Wed, 16 Jul 2025 17:19:44 +0200 Subject: [PATCH 4/8] small fix --- packages/ethereum-contracts/ops-scripts/deploy-framework.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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(); } From 5dad0c3565b9bb9a334ee0cf021bb089b012e802 Mon Sep 17 00:00:00 2001 From: didi Date: Wed, 16 Jul 2025 17:41:57 +0200 Subject: [PATCH 5/8] updated changelog --- packages/ethereum-contracts/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) 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`. From 882f654f56b65e3381167de4411a1c9e48d558b1 Mon Sep 17 00:00:00 2001 From: "Miao, ZhiCheng" Date: Thu, 17 Jul 2025 11:30:39 +0300 Subject: [PATCH 6/8] limit valid input size for _settle() --- .../agreements/gdav1/SuperfluidPool.sol | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol b/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol index 472bcd2160..5415c4f85e 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,7 +370,7 @@ 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 @@ -427,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); @@ -456,8 +456,9 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { emit MemberUnitsUpdated(superToken, memberAddr, oldUnits, newUnits); } - function _settle(address memberAddr, int256 amount) internal { - 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; } @@ -471,9 +472,10 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { } function _claimAll(address memberAddr, uint32 time) internal returns (int256 amount) { - amount = getClaimable(memberAddr, time); - _settle(memberAddr, amount); - + // For connected pool, claimable amount is zero; hence, we skip. + if (!superToken.isPoolMemberConnected(GDA, ISuperfluidPool(address(this)), memberAddr)) { + amount = _settle(memberAddr, time); + } emit DistributionClaimed(superToken, memberAddr, amount, _membersData[memberAddr].claimedValue); } @@ -485,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)); @@ -502,8 +504,7 @@ 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 settleAmount = getUnsettledValue(memberAddr, time); - _settle(memberAddr, settleAmount); + int256 settleAmount = _settle(memberAddr, time); int128 units = uint256(_getUnits(memberAddr)).toInt256().toInt128(); if (doConnect) { _shiftDisconnectedUnits(Unit.wrap(-units), Value.wrap(settleAmount), Time.wrap(time)); From e27cb4181602fa9df8414d267d99c44484f8bdfe Mon Sep 17 00:00:00 2001 From: "Miao, ZhiCheng" Date: Thu, 17 Jul 2025 11:33:54 +0300 Subject: [PATCH 7/8] remove some silly type convrersions --- .../contracts/agreements/gdav1/SuperfluidPool.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol b/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol index 5415c4f85e..6b7c33a895 100644 --- a/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol +++ b/packages/ethereum-contracts/contracts/agreements/gdav1/SuperfluidPool.sol @@ -375,7 +375,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { /// @inheritdoc ISuperfluidPool function getClaimable(address memberAddr, uint32 time) public view override returns (int256) { - return superToken.isPoolMemberConnected(GDA, ISuperfluidPool(address(this)), memberAddr) + return superToken.isPoolMemberConnected(GDA, this, memberAddr) ? int256(0) : getUnsettledValue(memberAddr, time); } @@ -440,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); } @@ -473,7 +473,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable { function _claimAll(address memberAddr, uint32 time) internal returns (int256 amount) { // For connected pool, claimable amount is zero; hence, we skip. - if (!superToken.isPoolMemberConnected(GDA, ISuperfluidPool(address(this)), memberAddr)) { + if (!superToken.isPoolMemberConnected(GDA, this, memberAddr)) { amount = _settle(memberAddr, time); } emit DistributionClaimed(superToken, memberAddr, amount, _membersData[memberAddr].claimedValue); From 5a0fb64cbe9d038a8f401d993f544c0fb1ebebeb Mon Sep 17 00:00:00 2001 From: pavedroad <138004431+pavedroad@users.noreply.github.com> Date: Thu, 17 Jul 2025 16:46:52 +0800 Subject: [PATCH 8/8] chore: remove redundant words in comment (#2092) Signed-off-by: pavedroad --- .../scheduler/contracts/VestingSchedulerV2.sol | 2 +- .../scheduler/contracts/VestingSchedulerV3.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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(); }