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

add lotPrice() back everywhere (unused) for backwards compatibility #1004

Merged
merged 4 commits into from
Nov 17, 2023

Conversation

tbrent
Copy link
Collaborator

@tbrent tbrent commented Oct 31, 2023

I realized while writing down 3.1.0 upgrade instructions that we can't pull lotPrice() off the assets + basketHandler if this is going to be a minor release. I've added it back everywhere with lotPrice() just returning the result of price().

@tbrent tbrent requested a review from pmckelvy1 October 31, 2023 18:29
@pmckelvy1
Copy link
Collaborator

worth adding back all the relevant tests?

@tbrent
Copy link
Collaborator Author

tbrent commented Nov 13, 2023

worth adding back all the relevant tests?

Well the behavior changed, the old tests would fail now. I do think it's worth adding new tests though just to make sure lotPrice() is implemented and that it returns the same thing as price(), will do.

Copy link

openzeppelin-code bot commented Nov 14, 2023

add lotPrice() back everywhere (unused) for backwards compatibility

Generated at commit: e2b9359790094ec63a61e3b23b77db206850d835

🚨 Vulnerabilities Summary

Process Issues Results
Contract Inspector high
note
low
critical
Total
1
35
18
2
56
Dependency Checker Total 0

For more details view the full report in OpenZeppelin Code

@tbrent
Copy link
Collaborator Author

tbrent commented Nov 14, 2023

I've added tests to check that (i) lotPrice() is on all asset interfaces; and (ii) lotPrice() returns the same thing as price()

@pmckelvy1
Copy link
Collaborator

let's add them for RTokenAsset and BasketHandler as well

@tbrent
Copy link
Collaborator Author

tbrent commented Nov 16, 2023

let's add them for RTokenAsset and BasketHandler as well

Good catch, added

@tbrent tbrent merged commit 595eabe into 3.1.0 Nov 17, 2023
8 of 9 checks passed
@tbrent tbrent deleted the 3.1.0-backwards-compatible branch November 17, 2023 19:02
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.

2 participants