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

feat(protocol): Move proofTimeTarget to state var and adjust scripts/tests #13769

Merged
merged 7 commits into from
May 16, 2023

Conversation

adaki2004
Copy link
Contributor

proofTImeTarget is now state var. Tests / scripts are adjusted.

Will add setter for them (proofTImeTarget + proofTImeIssued) soon - after lunch.

@adaki2004 adaki2004 requested review from dantaik, d1onys1us, Brechtpd and davidtaikocha and removed request for d1onys1us May 16, 2023 10:10
packages/protocol/contracts/L1/TaikoData.sol Outdated Show resolved Hide resolved
packages/protocol/script/DeployOnL1.s.sol Outdated Show resolved Hide resolved
packages/protocol/script/DetermineNewProofTimeIssued.s.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/L1/TaikoL1.sol Show resolved Hide resolved
@dantaik
Copy link
Contributor

dantaik commented May 16, 2023

Will add setter for them (proofTImeTarget + proofTImeIssued) soon - after lunch.

Maybe we should add them now :)

@adaki2004
Copy link
Contributor Author

Will add setter for them (proofTImeTarget + proofTImeIssued) soon - after lunch.

Maybe we should add them now :)

Sure! I meant 'lunch' not 'launch' :D

@adaki2004
Copy link
Contributor Author

Ok, added a function:
function setProofParams(uint64 newProofTimeTarget, uint64 newProofTimeIssued) external onlyOwner

And a helper script 'DetermineNewProofTimeIssued.s.sol'.

Some notes:

  • In case we want to update the proofTimeTarget AND proofTimeIssued as well, we simply need to run that script with DESIRED_PROOF_TIME_TARGET set properly and it will tell us the new proofTimeIssued value.
  • It might happen that we DO NOT want to update the proofTimeIssued variable. Either because the change is so small for the proofTimeTarget (5-10 secs) OR we think that thr provers will anyhow adjust their behavior to the new proofTimeTarget (aka: if we set it higher, provers will delay their proofs). In such case we need to set the input param newProofTimeIssued = type(uint64).max indicating this.

Copy link
Contributor Author

@adaki2004 adaki2004 left a comment

Choose a reason for hiding this comment

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

Resolved requests.

@dantaik dantaik added this pull request to the merge queue May 16, 2023
Merged via the queue into main with commit 40086b1 May 16, 2023
@dantaik dantaik deleted the create_proofTimes_setter branch May 16, 2023 15:25
@github-actions github-actions bot mentioned this pull request May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants