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

Refactor repeated time check logic into a private function in Bootstrap.sol #21

Closed
coderabbitai bot opened this issue Jun 5, 2024 · 0 comments · Fixed by #74
Closed

Refactor repeated time check logic into a private function in Bootstrap.sol #21

coderabbitai bot opened this issue Jun 5, 2024 · 0 comments · Fixed by #74
Assignees

Comments

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jun 5, 2024

In the Bootstrap.sol contract, the logic for checking spawn times and lock times is repeated multiple times. This logic can be refactored into a private function to reduce redundancy and improve maintainability. This issue tracks the suggested optimization for future implementation.

Related pull request: #17
Comment for context: #17 (comment)

MaxMustermann2 added a commit to MaxMustermann2/exocore-contracts that referenced this issue Aug 5, 2024
@MaxMustermann2 MaxMustermann2 linked a pull request Aug 5, 2024 that will close this issue
github-merge-queue bot pushed a commit that referenced this issue Aug 6, 2024
* refactor(bootstrap): optimize checks

Instead of iterating through the validator set, use a mapping to
identify whether the consensus key and the name are unique.

Closes #73 and #18

* fix: replace ++i with i++

closes #68

* refactor: DRY time check

Fixes #21

* fix: validate cons key non-empty

* fix: use `block.timestamp >= lockTime`

The `isLocked` function returns true if `block.timestamp >= lockTime`,
which is exactly what the `_validateSpawnTimeAndOffsetDuration` function
should do.

The spawn time must not be in the past, or exactly at the current block.

The offset duration must be greater than equal to the spawn time,
however, if they are equal, the lock time ends up in the past, and it is
rejected.

* fix: init i = 0 in for loops

* fix: use correct var when emitting event

Use storage var instead of function param

* fix: don't touch test files in this PR

* test: add offset duration > spawn time test

Split from >= case since the errors are different

* refactor: remove duplicated code
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 a pull request may close this issue.

2 participants