Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: response module deadline #58

Closed
wants to merge 2 commits into from
Closed

Conversation

gas1cent
Copy link
Member

No description provided.

@gas1cent gas1cent requested a review from 0xShaito August 27, 2024 14:59
@gas1cent gas1cent self-assigned this Aug 27, 2024
Copy link
Member

@0xJabberwock 0xJabberwock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the following lines found in BondEscalationModule make sense, considering that responseCreatedAt returns a block number and that disputeWindow is expressed in number of seconds as per the interface?

if (block.number > ORACLE.responseCreatedAt(_dispute.responseId) + _params.disputeWindow) {
  revert BondEscalationModule_DisputeWindowOver();
}

A similar construction can be found in the BondedResponseModule as well, although there the tests seem to acknowledge that the disputeWindow is a number of blocks.

Moreover, when looking at the same dispute module, every deadline and disputeWindow is expressed in terms of block timestamps, diverging from what this PR intends to establish in BondedResponseModule.

Comment on lines 30 to +31
// Cannot propose after the deadline
if (block.timestamp >= _params.deadline) revert BondedResponseModule_TooLateToPropose();
if (block.number >= _params.deadline) revert BondedResponseModule_TooLateToPropose();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revise the interface NatSpec for the deadline param.

@gas1cent gas1cent closed this Oct 2, 2024
@gas1cent gas1cent deleted the fix/response-module-deadline branch October 2, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants