search

solidity-labs-io / safe-time-guard

Other
2 stars 0 forks source link

Cleanup should delte id too? #34

Open Joeysantoro opened 2 hours ago

Joeysantoro commented 2 hours ago

https://github.com/solidity-labs-io/safe-time-guard/blob/a70a7592226a39fb3d11c01a16ac369ff4051b52/src/Timelock.sol#L685

After cleanup on an expired operation, timestamp is still set so

  1. isOperationExpired is still true
  2. the assert statement can be hit on a second call to cleanup
  3. The specific id of the expired operation can no longer be called at all because schedule calls will fail and there is no way to delete the id anymore
ElliotFriedman commented 2 hours ago

The assert statement can be false, however, if a proposal expired, we wanted to keep a record of that, and if we deleted the timestamp, then there would not be a record.