orbland / contracts

🔮 Orb and related contracts. Auction + Harberger taxed ownership + invocations.
https://orb.land
MIT License
22 stars 5 forks source link

[LOW] `flagResponse` can be issued for a prior invocation of the Orb #28

Closed odyslam closed 1 year ago

odyslam commented 1 year ago

Description

A holder can flag the response to the question of a previous holder.

This is because all the following checks will pass:

if (block.timestamp - responses[triggerId].timestamp > RESPONSE_FLAGGING_PERIOD) { revert FlaggingPeriodExpired( triggerId, block.timestamp - responses[triggerId].timestamp, RESPONSE_FLAGGING_PERIOD ); }

if (responseFlagged[triggerId] == true) { revert ResponseAlreadyFlagged(triggerId); }



## Suggestion

- Introduce a storage variable `flaggingEnd`
- In `trigger()`, add the following: `flaggingEnd = block.timestamp + RESPONSE_FLAGGING_PERIOD;` 
- In `flagResponse` check that: `block.timestamp < flaggingEnd`
- In `purchase()` and `foreclose()` add: `flaggingEnd = block.timestamp`
lekevicius commented 1 year ago

Wouldn't it be more efficient to just track lastPurchaseTime, updating that only during purchase(), and during flagging check that trigger time is after lastPurchaseTime? Fewer storage writes and probably easier to understand.

odyslam commented 1 year ago

Great suggestion. Will should do that.

odyslam commented 1 year ago

On top of this, we need to check that the response has happened after last auction, as an orb can change hands not only through a purchase but also through foreclose -> auction