-
Notifications
You must be signed in to change notification settings - Fork 91
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: update creditclock to facet #813
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should keep this test because it (at least) sheds some light on the original idea of why we need the CreditClock
facet and how to use it. Pls refactor accordingly.
we can't have this test, please note it takes state vars originally from the CreditClock, this is more like a test redesign that's why i open #812 to rather take time to design a "similar" test but not the same, plus look how the test it invokes 3 times for loops almost doing the same, the test is originally bad designed (to my point), you still think we need it for this issue? |
Then we should add new view methods to the I want to preserve as much context as possible. The whole "credit clock" feature consists of 2 purely described google documents + the original issue description. So it is pretty hard to keep the whole picture of how the |
possible approach could be inside the library function checkStructData() internal pure (CreditClockData memory) {
CreditClockData storage data = creditClockStorage();
return data; this result in foundry test
where as |
Update: We're putting It means that the |
Resolves #772
CreditClock.t.sol
Normal because this one it's being introduced as a lib in this PR
https://github.com/ubiquity/ubiquity-dollar/actions/runs/6447528063/job/17503896857?pr=813
look this call:
[58400] Diamond::setRatePerBlock(0x00000000000000000000000000000000)