Skip to content

Commit

Permalink
Tidy up nUnresolvedPayments edge cases; allow editing of interval
Browse files Browse the repository at this point in the history
  • Loading branch information
@area
area committed May 29, 2024
1 parent 8dfc183 commit 58d4d6e
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 18 deletions.
46 changes: 31 additions & 15 deletions contracts/extensions/StreamingPayments.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ contract StreamingPayments is ColonyExtensionMeta {
address token,
uint256 amount
);
event PaymentTokenUpdated(address agent, uint256 indexed streamingPaymentId, uint256 amount);
event PaymentTokenUpdated(
address agent,
uint256 indexed streamingPaymentId,
uint256 amount,
uint256 interval
);
event StartTimeSet(address agent, uint256 indexed streamingPaymentId, uint256 startTime);
event EndTimeSet(address agent, uint256 indexed streamingPaymentId, uint256 endTime);
event ClaimWaived(address agent, uint256 indexed streamingPaymentId);
Expand Down Expand Up @@ -258,6 +263,7 @@ contract StreamingPayments is ColonyExtensionMeta {
/// @param _toChildSkillIndex The linking the domainId to the toPot domain
/// @param _id The id of the streaming payment
/// @param _amount The new amount to pay out
/// @param _interval The new interval over which _amount is paid out
// slither-disable-next-line reentrancy-no-eth
function setTokenAmount(
uint256 _fundingPermissionDomainId,
Expand All @@ -267,7 +273,8 @@ contract StreamingPayments is ColonyExtensionMeta {
uint256 _fromChildSkillIndex,
uint256 _toChildSkillIndex,
uint256 _id,
uint256 _amount
uint256 _amount,
uint256 _interval
)
public
validateFundingPermission(
Expand All @@ -276,28 +283,36 @@ contract StreamingPayments is ColonyExtensionMeta {
streamingPayments[_id].domainId
)
{
claim(_permissionDomainId, _childSkillIndex, _fromChildSkillIndex, _toChildSkillIndex, _id);
StreamingPayment storage streamingPayment = streamingPayments[_id];
if (streamingPayment.startTime < block.timestamp) {
claim(_permissionDomainId, _childSkillIndex, _fromChildSkillIndex, _toChildSkillIndex, _id);
// This require checks that the above claim paid out the full amount the recipient is entitled to
// before any changes are made.
require(
streamingPayment.pseudoAmountClaimedFromStart >= getAmountEntitledFromStart(_id),
"streaming-payments-insufficient-funds"
);
}

bool wasResolved = streamingPayment.pseudoAmountClaimedFromStart >=
getAmountClaimableLifetime(_id);

// This require checks that the above claim paid out the full amount the recipient is entitled to
// before any changes are made.
require(
streamingPayment.pseudoAmountClaimedFromStart >= getAmountEntitledFromStart(_id),
"streaming-payments-insufficient-funds"
);
streamingPayment.amount = _amount;
streamingPayment.interval = _interval;

// Update 'claimed' as if we've had this rate since the beginning
streamingPayment.pseudoAmountClaimedFromStart = getAmountEntitledFromStart(_id);

// Note that if we're at this point, the payment prior to our editing _must_ have been resolved
// So there's no way we're going from an unresolved payment to a still-unresolved payment and accidentally
// incrementing this when we shouldn't.
if (getAmountClaimableLifetime(_id) >= streamingPayment.pseudoAmountClaimedFromStart) {
bool isResolved = streamingPayment.pseudoAmountClaimedFromStart >=
getAmountClaimableLifetime(_id);

if (wasResolved && !isResolved) {
nUnresolvedStreamingPayments += 1;
} else if (!wasResolved && isResolved) {
nUnresolvedStreamingPayments -= 1;
}

emit PaymentTokenUpdated(msgSender(), _id, _amount);
emit PaymentTokenUpdated(msgSender(), _id, _amount, _interval);
}

/// @notice Update the startTime, only if the current startTime is in the future
Expand Down Expand Up @@ -371,7 +386,8 @@ contract StreamingPayments is ColonyExtensionMeta {

uint256 newLifetimeClaimable = getAmountClaimableLifetime(_id);

// Unlike when we're setting start time, we need to check if the payment is unresolved here.
// Unlike when we're setting start time, we need to compare to pseudoAmountClaimedFromStart
// in order to determine if the payment is resolved or not.
bool wasResolved = streamingPayment.pseudoAmountClaimedFromStart >= oldLifetimeClaimable;
bool isResolved = streamingPayment.pseudoAmountClaimedFromStart >= newLifetimeClaimable;

Expand Down
3 changes: 2 additions & 1 deletion docs/interfaces/extensions/streamingpayments.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ Update the startTime, only if the current startTime is in the future
|_startTime|uint256|The new startTime to set


### ▸ `setTokenAmount(uint256 _fundingPermissionDomainId, uint256 _fundingChildSkillIndex, uint256 _permissionDomainId, uint256 _childSkillIndex, uint256 _fromChildSkillIndex, uint256 _toChildSkillIndex, uint256 _id, uint256 _amount)`
### ▸ `setTokenAmount(uint256 _fundingPermissionDomainId, uint256 _fundingChildSkillIndex, uint256 _permissionDomainId, uint256 _childSkillIndex, uint256 _fromChildSkillIndex, uint256 _toChildSkillIndex, uint256 _id, uint256 _amount, uint256 _interval)`

Update the token amount to be paid out. Claims existing payout prior to the change

Expand All @@ -238,6 +238,7 @@ Update the token amount to be paid out. Claims existing payout prior to the chan
|_toChildSkillIndex|uint256|The linking the domainId to the toPot domain
|_id|uint256|The id of the streaming payment
|_amount|uint256|The new amount to pay out
|_interval|uint256|The new interval over which _amount is paid out


### ▸ `uninstall()`
Expand Down
104 changes: 102 additions & 2 deletions test/extensions/streaming-payments.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ contract("Streaming Payments", (accounts) => {

// Claim one wad
balancePre = await token.balanceOf(USER1);
const updateArgs = [1, UINT256_MAX, 1, UINT256_MAX, UINT256_MAX, UINT256_MAX, streamingPaymentId, WAD.muln(2)];
const updateArgs = [1, UINT256_MAX, 1, UINT256_MAX, UINT256_MAX, UINT256_MAX, streamingPaymentId, WAD.muln(2), SECONDS_PER_DAY];
await makeTxAtTimestamp(streamingPayments.setTokenAmount, updateArgs, blockTime + SECONDS_PER_DAY, this);
balancePost = await token.balanceOf(USER1);
expect(balancePost.sub(balancePre)).to.eq.BN(WAD.muln(1).subn(1)); // -1 for network fee
Expand All @@ -627,7 +627,7 @@ contract("Streaming Payments", (accounts) => {
await forwardTime(SECONDS_PER_DAY, this);

await checkErrorRevert(
streamingPayments.setTokenAmount(1, UINT256_MAX, 1, UINT256_MAX, UINT256_MAX, UINT256_MAX, streamingPaymentId, WAD.muln(2)),
streamingPayments.setTokenAmount(1, UINT256_MAX, 1, UINT256_MAX, UINT256_MAX, UINT256_MAX, streamingPaymentId, WAD.muln(2), SECONDS_PER_DAY),
"streaming-payments-insufficient-funds",
);
});
Expand Down Expand Up @@ -802,5 +802,105 @@ contract("Streaming Payments", (accounts) => {
n = await streamingPayments.getNUnresolvedStreamingPayments();
expect(n).to.eq.BN(0);
});

it("various edge cases for nUnresolvedPayments are treated correctly when setting token amounts", async () => {
await fundColonyWithTokens(colony, token, WAD.muln(10));

const blockTime = await getBlockTime();

await streamingPayments.create(
1,
UINT256_MAX,
1,
UINT256_MAX,
1,
blockTime + SECONDS_PER_DAY,
blockTime + SECONDS_PER_DAY + 2,
SECONDS_PER_DAY,
USER1,
token.address,
1,
);
const streamingPaymentId = await streamingPayments.getNumStreamingPayments();

let nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(0);

await streamingPayments.setTokenAmount(1, UINT256_MAX, 1, UINT256_MAX, 1, UINT256_MAX, streamingPaymentId, SECONDS_PER_DAY, SECONDS_PER_DAY);
// Should now pay out 1 token

nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(1);

// No change expected
await streamingPayments.setTokenAmount(1, UINT256_MAX, 1, UINT256_MAX, 1, UINT256_MAX, streamingPaymentId, SECONDS_PER_DAY, SECONDS_PER_DAY);
nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(1);

// Reduce payout
await streamingPayments.setTokenAmount(1, UINT256_MAX, 1, UINT256_MAX, 1, UINT256_MAX, streamingPaymentId, 1, SECONDS_PER_DAY);
nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(0);

// Set payout again
await streamingPayments.setTokenAmount(1, UINT256_MAX, 1, UINT256_MAX, 1, UINT256_MAX, streamingPaymentId, SECONDS_PER_DAY, SECONDS_PER_DAY);
nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(1);

// Forward to way past the end of the payment
await forwardTime(SECONDS_PER_DAY * 2, this);

await streamingPayments.claim(1, UINT256_MAX, UINT256_MAX, UINT256_MAX, streamingPaymentId);
nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(0);

await streamingPayments.claim(1, UINT256_MAX, UINT256_MAX, UINT256_MAX, streamingPaymentId);
nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(0);

await streamingPayments.setTokenAmount(1, UINT256_MAX, 1, UINT256_MAX, 1, UINT256_MAX, streamingPaymentId, SECONDS_PER_DAY, SECONDS_PER_DAY);
// Still only pays out one token, but already claimed, so should be resolved
nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(0);

await streamingPayments.setTokenAmount(1, UINT256_MAX, 1, UINT256_MAX, 1, UINT256_MAX, streamingPaymentId, WAD, SECONDS_PER_DAY);
// Would have paid out more, but updating claims only affects future payouts, and this payment was already finished
// and resolved, so no change expected
nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(0);
});

it("various edge cases for nUnresolvedPayments are treated correctly when setting start and end times", async () => {
await fundColonyWithTokens(colony, token, WAD.muln(10));

const blockTime = await getBlockTime();

await streamingPayments.create(
1,
UINT256_MAX,
1,
UINT256_MAX,
1,
blockTime + SECONDS_PER_DAY,
blockTime + SECONDS_PER_DAY + 1,
SECONDS_PER_DAY,
USER1,
token.address,
1,
);

const streamingPaymentId = await streamingPayments.getNumStreamingPayments();

let nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(0);

await streamingPayments.setStartTime(1, UINT256_MAX, streamingPaymentId, blockTime + SECONDS_PER_DAY);
nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(0);

await streamingPayments.setEndTime(1, UINT256_MAX, streamingPaymentId, blockTime + SECONDS_PER_DAY + 1);
nUnresolvedPayments = await streamingPayments.getNUnresolvedStreamingPayments();
expect(nUnresolvedPayments).to.eq.BN(0);
});
});
});

0 comments on commit 58d4d6e

Please sign in to comment.