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

Wrong seconds per block in the DAI interest rate model #230

Open
MathisGD opened this issue Dec 4, 2022 · 2 comments · May be fixed by #231
Open

Wrong seconds per block in the DAI interest rate model #230

MathisGD opened this issue Dec 4, 2022 · 2 comments · May be fixed by #231

Comments

@MathisGD
Copy link

MathisGD commented Dec 4, 2022

uint256 private constant SECONDS_PER_BLOCK = 15;

The average number of second between each block used here is 15, while since the Merge, the it is now stable at 12 seconds. This means that the supply rate given by getSupplyRate would be wrong.

Some discussions are open to re-enable deposits to the DSR from cDAI, and it will not be possible to re-use the old DAIInterestRateModelV3 available at 0xfed941d39905b23d6faf02c8301d40bd4834e27f because of this reason.

@MathisGD MathisGD changed the title Wrong second per block in the DAI interest rate model Wrong seconds per block in the DAI interest rate model Dec 4, 2022
@fmorisan
Copy link

Would an easy fix for this just be a simple change like so?

uint256 private constant SECONDS_PER_BLOCK = 12;

fmorisan added a commit to fmorisan/compound-protocol that referenced this issue Dec 22, 2022
fmorisan added a commit to fmorisan/compound-protocol that referenced this issue Dec 22, 2022
@fmorisan fmorisan linked a pull request Dec 22, 2022 that will close this issue
@MathisGD
Copy link
Author

By the way, I just noticed that all the other IRMs assume that the number of second between each block is 15 (2 102 400 blocks per year). But that's not that of an issue, because it is only used to covert the parameters from yearly to per-block at the construction of the contract.

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