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

Features: Adds cap to investment drawdowns. #92

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jamesduncombe
Copy link
Contributor

@jamesduncombe jamesduncombe commented Feb 19, 2024

This PR:

  • Adds cap to the creation params for a Crowdfund.
  • Adds isCapped helper method to Crowdfund contract.
  • Bumps Crowdfund version to v3.

⚠️ Caveats...

  • Currently does not handle stupidly small number when creating a capped crowdfund. You can currently create a crowdfund with a 1 wei cap.

Copy link

This pull request has been linked to Shortcut Story #9318: Investment drawdowns: Add cap to smart contract.

@jamesduncombe jamesduncombe added the enhancement New feature or request label Feb 19, 2024
Copy link

codecov bot commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.01%. Comparing base (c0b8727) to head (83732e2).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #92      +/-   ##
==========================================
+ Coverage   79.77%   80.01%   +0.24%     
==========================================
  Files          56       56              
  Lines        1132     1136       +4     
  Branches      178      235      +57     
==========================================
+ Hits          903      909       +6     
+ Misses        229      227       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jamesduncombe jamesduncombe force-pushed the features/sc-9318-investment-drawdowns-add-cap-to-smart-contract branch 3 times, most recently from 22854d8 to 1ed48db Compare February 19, 2024 18:04
@@ -14,7 +14,7 @@ import { abiStructToObj } from "../utils";
chai.use(solidity);
chai.use(smock.matchers);

describe.only("MarketplaceFastDeploymentRequestsFacet", () => {
describe("MarketplaceFastDeploymentRequestsFacet", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably add a check to the CI for this... easy to sneak this in then not test everything by mistake.

@jamesduncombe jamesduncombe force-pushed the features/sc-9318-investment-drawdowns-add-cap-to-smart-contract branch 2 times, most recently from f3830f5 to 675321a Compare February 19, 2024 18:30
}

/// @notice A version identifier for us to track what's deployed.
uint16 public constant VERSION = 2;
uint16 public constant VERSION = 3;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New feature... version bump.

Copy link
Member

Choose a reason for hiding this comment

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

Frontend is going to run into some pain here. We should chat.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Damn, ok. I'll ping you 👍

@jamesduncombe jamesduncombe marked this pull request as ready for review March 4, 2024 13:52
@jamesduncombe jamesduncombe requested a review from hickscorp March 4, 2024 13:53
@jamesduncombe jamesduncombe force-pushed the features/sc-9318-investment-drawdowns-add-cap-to-smart-contract branch from 675321a to 0744576 Compare April 22, 2024 12:56
@jamesduncombe jamesduncombe force-pushed the features/sc-9318-investment-drawdowns-add-cap-to-smart-contract branch from 0744576 to 83732e2 Compare April 22, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants