Skip to content

Commit 02b713b

Browse files
committed
fix(contracts): emit correct newExpiration in UserPayment.until (#2268)
receive() computed newExpiration to validate against maxSubscriptionTimeAhead, but the UserPayment event re-derived block.timestamp + paymentExpirationTimeSeconds instead of using it -- wrong (earlier) 'until' whenever an active subscription is extended, diverging from what's actually stored in subscribedAddresses. Fixes #2268
1 parent 2b65191 commit 02b713b

2 files changed

Lines changed: 95 additions & 1 deletion

File tree

‎contracts/src/core/AggregationModePaymentService.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ contract AggregationModePaymentService is Initializable, UUPSUpgradeable, Access
247247
}
248248

249249

250-
emit UserPayment(msg.sender, amount, block.timestamp, block.timestamp + paymentExpirationTimeSeconds);
250+
emit UserPayment(msg.sender, amount, block.timestamp, newExpiration);
251251
}
252252

253253
/**
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// SPDX-License-Identifier: UNLICENSED
2+
pragma solidity ^0.8.12;
3+
4+
import "forge-std/Test.sol";
5+
import {AggregationModePaymentService} from "../src/core/AggregationModePaymentService.sol";
6+
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
7+
8+
contract AggregationModePaymentServiceTest is Test {
9+
AggregationModePaymentService service;
10+
11+
address owner = address(0x1);
12+
address admin = address(0x2);
13+
address fundsRecipient = address(0x3);
14+
address user = address(0x4);
15+
16+
uint256 constant AMOUNT_TO_PAY = 1 ether;
17+
uint256 constant EXPIRATION_SECONDS = 30 days;
18+
uint256 constant SUBSCRIPTION_LIMIT = 100;
19+
uint256 constant MAX_TIME_AHEAD = 365 days;
20+
21+
event UserPayment(address user, uint256 indexed amount, uint256 indexed from, uint256 indexed until);
22+
23+
function setUp() public {
24+
AggregationModePaymentService implementation = new AggregationModePaymentService();
25+
26+
bytes memory initData = abi.encodeWithSelector(
27+
AggregationModePaymentService.initialize.selector,
28+
owner,
29+
admin,
30+
fundsRecipient,
31+
AMOUNT_TO_PAY,
32+
EXPIRATION_SECONDS,
33+
SUBSCRIPTION_LIMIT,
34+
MAX_TIME_AHEAD
35+
);
36+
37+
ERC1967Proxy proxy = new ERC1967Proxy(address(implementation), initData);
38+
service = AggregationModePaymentService(payable(address(proxy)));
39+
40+
vm.deal(user, 10 ether);
41+
}
42+
43+
/// @notice New subscription (no prior active period): `until` should equal
44+
/// `block.timestamp + paymentExpirationTimeSeconds`, i.e. `newExpiration`.
45+
/// This already matches the buggy emission by coincidence, so it does not
46+
/// by itself prove the fix -- see the extension test below for that.
47+
function test_UserPayment_emitsCorrectUntil_onFreshSubscription() public {
48+
uint256 startTime = block.timestamp;
49+
uint256 expectedUntil = startTime + EXPIRATION_SECONDS;
50+
51+
vm.expectEmit(true, true, true, true, address(service));
52+
emit UserPayment(user, AMOUNT_TO_PAY, startTime, expectedUntil);
53+
54+
vm.prank(user);
55+
(bool ok,) = address(service).call{value: AMOUNT_TO_PAY}("");
56+
assertTrue(ok);
57+
58+
assertEq(service.subscribedAddresses(user), expectedUntil);
59+
}
60+
61+
/// @notice Extending an already-active subscription: the emitted `until`
62+
/// must be the *actual* new expiration (extended from the current expiry),
63+
/// not `block.timestamp + paymentExpirationTimeSeconds` computed from the
64+
/// second payment's timestamp. Before the fix, this event carries a wrong
65+
/// (earlier) value than what was actually stored in `subscribedAddresses`.
66+
function test_UserPayment_emitsCorrectUntil_onExtendedSubscription() public {
67+
// First payment: starts a fresh subscription.
68+
vm.prank(user);
69+
(bool ok1,) = address(service).call{value: AMOUNT_TO_PAY}("");
70+
assertTrue(ok1);
71+
72+
uint256 firstExpiration = service.subscribedAddresses(user);
73+
74+
// Move forward, but stay within the still-active subscription window.
75+
vm.warp(block.timestamp + 5 days);
76+
77+
uint256 secondPaymentTime = block.timestamp;
78+
uint256 realNewExpiration = firstExpiration + EXPIRATION_SECONDS;
79+
80+
// The buggy code would instead emit secondPaymentTime + EXPIRATION_SECONDS,
81+
// which is 5 days earlier than the real stored expiration.
82+
assertTrue(realNewExpiration != secondPaymentTime + EXPIRATION_SECONDS);
83+
84+
vm.expectEmit(true, true, true, true, address(service));
85+
emit UserPayment(user, AMOUNT_TO_PAY, secondPaymentTime, realNewExpiration);
86+
87+
vm.prank(user);
88+
(bool ok2,) = address(service).call{value: AMOUNT_TO_PAY}("");
89+
assertTrue(ok2);
90+
91+
// The event must match what's actually persisted in storage.
92+
assertEq(service.subscribedAddresses(user), realNewExpiration);
93+
}
94+
}

0 commit comments

Comments
 (0)