Skip to content

🥢 Update ERC20Votes clock to virtual so it can allow for timestamp clock_mode - #1263

Merged
Vectorized merged 2 commits into
Vectorized:mainfrom
giuseppecrj:patch-1
Jan 9, 2025
Merged

Vectorized merged 2 commits into
Vectorized:mainfrom
giuseppecrj:patch-1

Conversation

@giuseppecrj

@giuseppecrj giuseppecrj commented Jan 8, 2025 •

Copy link
Copy Markdown
Contributor

CLOCK_MODE() is virtual, we need clock() to be virtual as well otherwise there's an issue with using mode=timestamp while returning a block.number.

Description

We need clock() to be virtual in case users want timestamp based clock_mode

Checklist

Ensure you completed all of the steps below before submitting your pull request:

  • Ran forge fmt?
  • Ran forge test?

Pull requests with an incomplete checklist will be thrown out.

…d clock votes

CLOCK_MODE() is virtual, we need clock() to be virtual as well otherwise there's an issue with using mode=timestamp while returning a block.number.
@giuseppecrj giuseppecrj changed the title Update ERC20Votes clock to virtual so it can allow for timestamp base… Update ERC20Votes clock to virtual so it can allow for timestamp clock_mode Jan 8, 2025
@giuseppecrj giuseppecrj changed the title Update ERC20Votes clock to virtual so it can allow for timestamp clock_mode ♻️ Update ERC20Votes clock to virtual so it can allow for timestamp clock_mode Jan 8, 2025
@giuseppecrj giuseppecrj changed the title ♻️ Update ERC20Votes clock to virtual so it can allow for timestamp clock_mode 🥢 Update ERC20Votes clock to virtual so it can allow for timestamp clock_mode Jan 8, 2025

@shuhuiluo shuhuiluo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lgtm.

@Vectorized

Copy link
Copy Markdown
Owner

Oh my, how could i miss this. LGTM.

@Vectorized
Vectorized merged commit c7fc8d1 into Vectorized:main Jan 9, 2025
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