[WIP] Added overflow handling for TWAB timestamp #204
Conversation
POOL-1408 Ticket contract should handle overflow cleanly
Two items can overflow:
The key is to let them overflow, but ensure you can detect when they overflow. A branch with partial implementation is here, although it will be out of date: https://github.com/pooltogether/pooltogether-contract-tsunami/tree/pool-1408-ticket-contract-should-handle-overflow I believe the branch solved the timestamp overflow, but not the amount. |
contracts/DrawCalculator.sol
Outdated
| _timestampsWithStartCutoffTimes[i] = ( | ||
| _draws[i].timestamp - _prizeDistributions[i].startTimestampOffset | ||
| ); | ||
| _timestampsWithEndCutoffTimes[i] = uint32( | ||
| _timestampsWithEndCutoffTimes[i] = ( | ||
| _draws[i].timestamp - _prizeDistributions[i].endTimestampOffset | ||
| ); |
There was a problem hiding this comment.
So we don't need these parentheses anymore right?
| @@ -261,8 +261,8 @@ contract Ticket is ControlledToken, ITicket { | |||
| averageBalances[i] = TwabLib.getAverageBalanceBetween( | |||
| _account.twabs, | |||
There was a problem hiding this comment.
Should be stored in a variable.
There was a problem hiding this comment.
which? The _account.twabs ? it's a pointer- wouldn't make a difference
| @@ -2,6 +2,8 @@ | |||
|
|
|||
| pragma solidity 0.8.6; | |||
|
|
|||
| import "hardhat/console.sol"; | |||
| @@ -41,14 +41,6 @@ describe('TwabLib', () => { | |||
| .withArgs([200, 1, 1], [0, timestamp], false); | |||
| }); | |||
|
|
|||
| it('should require the timestamp to always increase', async () => { | |||
There was a problem hiding this comment.
Why has this test been removed?
There was a problem hiding this comment.
Because if the passed timestamp is less than the previous timestamp it assumes that the timestamp has overflowed. That means this test is no longer relevant (I think)
asselstine
force-pushed
the
pool-1408-ticket-contract-should-handle-overflow
branch
from
2fe1bc4
to
2a9d203
Compare
contracts/DrawBeacon.sol
Outdated
| @@ -191,7 +191,7 @@ contract DrawBeacon is IDrawBeacon, Ownable { | |||
| } | |||
|
|
|||
| /// @inheritdoc IDrawBeacon | |||
| function calculateNextBeaconPeriodStartTime(uint256 _currentTime) | |||
| function calculateNextBeaconPeriodStartTime(uint256 __currentTime) | |||
There was a problem hiding this comment.
Is this to avoid shadowing? I've renamed to _time:
v4-core/contracts/DrawBeacon.sol
Line 194 in b039133
So you probably need to rebase master.
asselstine
force-pushed
the
pool-1408-ticket-contract-should-handle-overflow
branch
from
ff7e8b8
to
0832802
Compare
asselstine
force-pushed
the
pool-1408-ticket-contract-should-handle-overflow
branch
from
0832802
to
688d7ad
Compare
asselstine
deleted the
pool-1408-ticket-contract-should-handle-overflow
branch
No description provided.