Skip to content

Commit 7d56c30

Browse files
d10rhellwolfpavedroad
authored
[ETHEREUM-CONTRACTS] distinguish between unsettled and claimable balance in pools (#2084)
* distinguish between unsettled and claimable balance in pools * (piggyback) chore: remove redundant words in comment (#2092) --------- Co-authored-by: Miao, ZhiCheng <zhicheng.miao@gmail.com> Co-authored-by: pavedroad <138004431+pavedroad@users.noreply.github.com>
1 parent 9ecc54b commit 7d56c30

File tree

8 files changed

+97
-23
lines changed

8 files changed

+97
-23
lines changed

‎packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase {
441441
if (!disableClaimCheck && schedule.claimValidityDate != 0)
442442
revert ScheduleNotClaimed();
443443

444-
// Ensure that that the claming date is after the cliff/flow date and before the claim validity date
444+
// Ensure that the claming date is after the cliff/flow date and before the claim validity date
445445
if (schedule.cliffAndFlowDate > block.timestamp ||
446446
_lteDateToExecuteCliffAndFlow(schedule) < block.timestamp)
447447
revert TimeWindowInvalid();

‎packages/automation-contracts/scheduler/contracts/VestingSchedulerV3.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ contract VestingSchedulerV3 is IVestingSchedulerV3, IRelayRecipient {
760760
revert ScheduleNotClaimed();
761761
}
762762

763-
// Ensure that that the claming date is after the cliff/flow date and before the claim validity date
763+
// Ensure that the claming date is after the cliff/flow date and before the claim validity date
764764
if (schedule.cliffAndFlowDate > block.timestamp || _lteDateToExecuteCliffAndFlow(schedule) < block.timestamp) {
765765
revert TimeWindowInvalid();
766766
}

‎packages/ethereum-contracts/CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1111
- `GDAv1StorageReader` contains getters reading agreement data from the token contract, allowing contracts to get this data without making a call to the GDA contract.
1212
- `GDAv1StorageWriter` contains functions for writing agreement data to the token contract. This can only be used by the GDA contract itself.
1313

14+
### Fixed
15+
- `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).
16+
1417
### Breaking
1518
- PoolMemberNFT pruning: `IPoolMemberNFT` and `PoolMemberNFT` removed, `POOL_MEMBER_NFT()` removed from `ISuperToken`.
1619

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ contract GeneralDistributionAgreementV1 is AgreementBase, TokenMonad, IGeneralDi
8989
for (uint256 i = 0; i < slotIds.length; ++i) {
9090
address pool = address(uint160(uint256(pidList[i])));
9191
_assertPoolConnectivity(token, account, ISuperfluidPool(pool));
92-
totalConnectedFromPools += ISuperfluidPool(pool).getClaimable(account, uint32(time));
92+
totalConnectedFromPools += SuperfluidPool(pool).getUnsettledValue(account, uint32(time));
9393
}
9494
}
9595
rtb += totalConnectedFromPools;

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

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
289289
Value.unwrap(
290290
// PDPoolMemberMU(poolIndex, memberData)
291291
PDPoolMemberMU(poolIndexDataToPDPoolIndex(_index), _memberDataToPDPoolMember(memberData)).settle(
292-
Time.wrap(uint32(block.timestamp))
292+
Time.wrap(SafeCast.toUint32(block.timestamp))
293293
).m._settled_value
294294
)
295295
);
@@ -370,17 +370,14 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
370370
returns (int256 claimableBalance, uint256 timestamp)
371371
{
372372
timestamp = ISuperfluid(superToken.getHost()).getNow();
373-
return (getClaimable(memberAddr, uint32(timestamp)), timestamp);
373+
return (getClaimable(memberAddr, SafeCast.toUint32(timestamp)), timestamp);
374374
}
375375

376376
/// @inheritdoc ISuperfluidPool
377377
function getClaimable(address memberAddr, uint32 time) public view override returns (int256) {
378-
Time t = Time.wrap(time);
379-
PDPoolIndex memory pdPoolIndex = poolIndexDataToPDPoolIndex(_index);
380-
PDPoolMember memory pdPoolMember = _memberDataToPDPoolMember(_membersData[memberAddr]);
381-
return Value.unwrap(
382-
PDPoolMemberMU(pdPoolIndex, pdPoolMember).rtb(t) - Value.wrap(_membersData[memberAddr].claimedValue)
383-
);
378+
return superToken.isPoolMemberConnected(GDA, this, memberAddr)
379+
? int256(0)
380+
: getUnsettledValue(memberAddr, time);
384381
}
385382

386383
/// @inheritdoc ISuperfluidPool
@@ -430,7 +427,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
430427
if (superToken.isPool(GDA, memberAddr)) revert SUPERFLUID_POOL_NO_POOL_MEMBERS();
431428
if (memberAddr == address(0)) revert SUPERFLUID_POOL_NO_ZERO_ADDRESS();
432429

433-
uint32 time = uint32(ISuperfluid(superToken.getHost()).getNow());
430+
uint32 time = SafeCast.toUint32(ISuperfluid(superToken.getHost()).getNow());
434431
Time t = Time.wrap(time);
435432
Unit wrappedUnits = toSemanticMoneyUnit(newUnits);
436433

@@ -443,7 +440,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
443440
PDPoolMemberMU memory mu = PDPoolMemberMU(pdPoolIndex, pdPoolMember);
444441

445442
// update pool's disconnected units
446-
if (!superToken.isPoolMemberConnected(GDA, ISuperfluidPool(address(this)), memberAddr)) {
443+
if (!superToken.isPoolMemberConnected(GDA, this, memberAddr)) {
447444
_shiftDisconnectedUnits(wrappedUnits - mu.m.owned_units, Value.wrap(0), t);
448445
}
449446

@@ -459,11 +456,26 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
459456
emit MemberUnitsUpdated(superToken, memberAddr, oldUnits, newUnits);
460457
}
461458

462-
function _claimAll(address memberAddr, uint32 time) internal returns (int256 amount) {
463-
amount = getClaimable(memberAddr, time);
464-
assert(GDA.poolSettleClaim(superToken, memberAddr, (amount)));
459+
function _settle(address memberAddr, uint32 time) internal returns (int256 amount) {
460+
amount = getUnsettledValue(memberAddr, time);
461+
assert(GDA.poolSettleClaim(superToken, memberAddr, amount));
465462
_membersData[memberAddr].claimedValue += amount;
463+
}
466464

465+
function getUnsettledValue(address memberAddr, uint32 time) public view returns (int256) {
466+
Time t = Time.wrap(time);
467+
PDPoolIndex memory pdPoolIndex = poolIndexDataToPDPoolIndex(_index);
468+
PDPoolMember memory pdPoolMember = _memberDataToPDPoolMember(_membersData[memberAddr]);
469+
return Value.unwrap(
470+
PDPoolMemberMU(pdPoolIndex, pdPoolMember).rtb(t) - Value.wrap(_membersData[memberAddr].claimedValue)
471+
);
472+
}
473+
474+
function _claimAll(address memberAddr, uint32 time) internal returns (int256 amount) {
475+
// For connected pool, claimable amount is zero; hence, we skip.
476+
if (!superToken.isPoolMemberConnected(GDA, this, memberAddr)) {
477+
amount = _settle(memberAddr, time);
478+
}
467479
emit DistributionClaimed(superToken, memberAddr, amount, _membersData[memberAddr].claimedValue);
468480
}
469481

@@ -475,7 +487,7 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
475487
/// @inheritdoc ISuperfluidPool
476488
function claimAll(address memberAddr) public returns (bool) {
477489
bool isConnected = superToken.isPoolMemberConnected(GDA, this, memberAddr);
478-
uint32 time = uint32(ISuperfluid(superToken.getHost()).getNow());
490+
uint32 time = SafeCast.toUint32(ISuperfluid(superToken.getHost()).getNow());
479491
int256 claimedAmount = _claimAll(memberAddr, time);
480492
if (!isConnected) {
481493
_shiftDisconnectedUnits(Unit.wrap(0), Value.wrap(claimedAmount), Time.wrap(time));
@@ -492,10 +504,10 @@ contract SuperfluidPool is ISuperfluidPool, BeaconProxiable {
492504

493505
// WARNING for operators: it is undefined behavior if member is already connected or disconnected
494506
function operatorConnectMember(address memberAddr, bool doConnect, uint32 time) external onlyGDA returns (bool) {
495-
int256 claimedAmount = _claimAll(memberAddr, time);
507+
int256 settleAmount = _settle(memberAddr, time);
496508
int128 units = uint256(_getUnits(memberAddr)).toInt256().toInt128();
497509
if (doConnect) {
498-
_shiftDisconnectedUnits(Unit.wrap(-units), Value.wrap(claimedAmount), Time.wrap(time));
510+
_shiftDisconnectedUnits(Unit.wrap(-units), Value.wrap(settleAmount), Time.wrap(time));
499511
} else {
500512
_shiftDisconnectedUnits(Unit.wrap(units), Value.wrap(0), Time.wrap(time));
501513
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -996,7 +996,7 @@ module.exports = eval(`(${S.toString()})({skipArgv: true})`)(async function (
996996
const poolMemberNFTPAddr = await superTokenLogic.POOL_MEMBER_NFT();
997997
let poolMemberNFTLAddr = ZERO_ADDRESS;
998998
if (poolMemberNFTPAddr !== ZERO_ADDRESS) {
999-
const poolMemberNFTContract = await PoolMemberNFT.at(poolMemberNFTPAddr);
999+
const poolMemberNFTContract = await UUPSProxiable.at(poolMemberNFTPAddr);
10001000
poolMemberNFTLAddr = await poolMemberNFTContract.getCodeAddress();
10011001
}
10021002

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,6 +1271,21 @@ contract FoundrySuperfluidTester is Test {
12711271
// _assertRealTimeBalances(ISuperToken(address(poolSuperToken)));
12721272
}
12731273

1274+
function _helperClaimAll(ISuperfluidPool pool_, address caller_, address member_) internal {
1275+
(int256 claimableBefore,) = pool_.getClaimableNow(member_);
1276+
(int256 balanceBefore,,,) = pool_.superToken().realtimeBalanceOfNow(member_);
1277+
1278+
vm.startPrank(caller_);
1279+
pool_.claimAll(member_);
1280+
vm.stopPrank();
1281+
1282+
(int256 claimableAfter,) = pool_.getClaimableNow(member_);
1283+
(int256 balanceAfter,,,) = pool_.superToken().realtimeBalanceOfNow(member_);
1284+
1285+
assertEq(claimableAfter, 0, "GDAv1.t: Member claimable amount should be 0");
1286+
assertEq(balanceAfter, balanceBefore + claimableBefore, "GDAv1.t: Member balance should increase by claimable amount");
1287+
}
1288+
12741289
function _helperConnectPool(address caller_, ISuperToken superToken_, ISuperfluidPool pool_, bool useForwarder_)
12751290
internal
12761291
{

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

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -940,6 +940,52 @@ contract GeneralDistributionAgreementV1IntegrationTest is FoundrySuperfluidTeste
940940
_helperSuperfluidPoolUnitsTransferFrom(freePool, spender, owner, spender, uint256(uint128(transferAmount)));
941941
}
942942

943+
function testGetClaimable(
944+
address member,
945+
uint128 units,
946+
uint64 distributionAmount,
947+
bool useForwarder,
948+
PoolConfig memory config
949+
) public {
950+
vm.assume(member != address(0));
951+
vm.assume(member != address(freePool));
952+
vm.assume(units > 0);
953+
vm.assume(distributionAmount > 0);
954+
vm.assume(units < distributionAmount);
955+
vm.assume(distributionAmount < type(uint128).max);
956+
957+
// Create a pool for testing
958+
ISuperfluidPool pool = _helperCreatePool(superToken, alice, alice, useForwarder, config);
959+
_addAccount(member);
960+
961+
// Step 1: Assign units to disconnected member
962+
_helperUpdateMemberUnits(pool, alice, member, units, _StackVars_UseBools({useForwarder: useForwarder, useGDA: false}));
963+
964+
(int256 balanceBefore,,,) = superToken.realtimeBalanceOfNow(member);
965+
(int256 claimableBefore,) = pool.getClaimableNow(member);
966+
967+
// Distribute
968+
_helperDistributeViaGDA(superToken, alice, alice, pool, distributionAmount, useForwarder);
969+
970+
// Check disconnected member: expect balance unchanged, claimable increased
971+
(int256 balanceAfter1,,,) = superToken.realtimeBalanceOfNow(member);
972+
(int256 claimableAfter1,) = pool.getClaimableNow(member);
973+
974+
assertEq(balanceAfter1, balanceBefore, "Disconnected member balance should not change");
975+
assertTrue(claimableAfter1 > claimableBefore, "Disconnected member claimable amount should increase");
976+
977+
// Step 2: Connect member and distribute again
978+
_helperConnectPool(member, superToken, pool, useForwarder);
979+
_helperDistributeViaGDA(superToken, alice, alice, pool, distributionAmount, useForwarder);
980+
981+
(int256 balanceAfter2,,,) = superToken.realtimeBalanceOfNow(member);
982+
(int256 claimableAfter2,) = pool.getClaimableNow(member);
983+
984+
// Check connected member: balance increased, claimable remains 0
985+
assertTrue(balanceAfter2 > balanceAfter1, "Connected member balance should increase");
986+
assertEq(claimableAfter2, 0, "Connected member claimable amount should be 0");
987+
}
988+
943989
/*//////////////////////////////////////////////////////////////////////////
944990
Assertion Functions
945991
//////////////////////////////////////////////////////////////////////////*/
@@ -983,9 +1029,7 @@ contract GeneralDistributionAgreementV1IntegrationTest is FoundrySuperfluidTeste
9831029
address u4 = TEST_ACCOUNTS[1 + (s.v % N_MEMBERS)];
9841030
emit log_named_string("action", "claimAll");
9851031
emit log_named_address("claim for", u4);
986-
vm.startPrank(user);
987-
assert(freePool.claimAll(u4));
988-
vm.stopPrank();
1032+
_helperClaimAll(freePool, user, u4);
9891033
} else if (action == 3) {
9901034
bool doConnect = s.v % 2 == 0 ? false : true;
9911035
emit log_named_string("action", "doConnectPool");

0 commit comments

Comments
 (0)