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

fix(contract preaudit updates): make changes, fixes, upgrade as requested by contracts team #9

Merged
merged 25 commits into from
Feb 6, 2023

Conversation

cmwhited
Copy link
Contributor

@cmwhited cmwhited commented Feb 1, 2023

Description

Updates to the Subscriptions.sol contract based off Findings here

@cmwhited cmwhited marked this pull request as ready for review February 1, 2023 21:09
@cmwhited cmwhited mentioned this pull request Feb 1, 2023
Copy link
Contributor

@tmigone tmigone left a comment

Choose a reason for hiding this comment

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

🚀 left some comments!

contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
int256 len = Prelude.max(
0,
int64(sub.end) - Prelude.max(int64(currentBlock), int64(sub.start))
function unlocked(Subscription memory sub) private view returns (uint128) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as locked I'd change visibility to public (this one IS being used by the contract however).

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above if we make it public we can consider changing the input parameters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see 7a3d93e

contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
@cmwhited cmwhited requested a review from tmigone February 2, 2023 19:08
Copy link
Member

@Theodus Theodus left a comment

Choose a reason for hiding this comment

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

Great stuff! Just some nits I found while catching myself up on the progress

contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
contract/contracts/Subscriptions.sol Outdated Show resolved Hide resolved
@cmwhited cmwhited merged commit 3dde60e into main Feb 6, 2023
@cmwhited cmwhited deleted the chris.whited/contract-preaudit-updates branch February 6, 2023 18:58
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