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

Remaining Audit Fixes & Factory Pattern #750

Merged
merged 30 commits into from
Oct 18, 2023
Merged

Conversation

JakeHartnell
Copy link
Member

@JakeHartnell JakeHartnell commented Sep 14, 2023

Addresses a few of the remaining audit fixes, but more importantly solves a blocker for both Stargaze NFT DAOs as well as potentially future Token Factory DAOs. The Stargaze team requested that NFT creation honor would honor the fairburn mechanism.

This is made possible by the factory pattern: creating Token or NFT DAO tokens by calling into a factory contract that does validation as well as returning required information to implement the voting module.

For example, as a user I want to sell a collection on Stargaze and use that collection and the proceeds from it to create and fund a DAO (with the DAO as the creator of the collection).

This "factory pattern" feature is intended to work with minters such as Stargaze minters or the WIP Augmented Bonding Curve tokens (or potentially other tokens such as LP tokens) which require a more complicated setup process. For these types of projects, the token is created through another contract that manages its issuance, the factory pattern is designed to support this. Factory contracts MUST set TokenFactoryCallback or NftFactoryCallback in the response.

An important note on security: as validation SHOULD happen in the factory contracts, it's important to rely only trusted factory contracts.

Overview of changes:

  • ModuleInstantiateInfo now takes a funds argument so that factories that require funds (such as one using Stargaze fairburn) can be supported.
  • sg721 support has been removed in favor of the factory pattern
  • A new test contract dao-test-custom-factory has been created for testing factory patterns as well as demonstrating how to build a custom factory.
  • test-contracts have been moved inside the contracts/test folder so they can be built with the workspace-optimizer
  • Documentation and comments about this feature have been added
  • Fix inaccurate error message for threshold validation
  • Improve reuse of active threshold logic in dao-voting-cw721-staked, and fix a bug whereby one could update the threshold to be greater than supply

Base automatically changed from audit-fixes to development September 15, 2023 01:57
@JakeHartnell JakeHartnell force-pushed the factory-pattern branch 2 times, most recently from 0b90b8e to c20cce1 Compare September 15, 2023 04:14
# This is the 1st commit message:

Simpler NFT factory test

# The commit message #2 will be skipped:

# f
Context: after speaking with the team, they want all NFTs on the
platform to utilize the fairburn mechanism. In addition, many artists
creating new collections on Stargaze also want to create a minter for
that collection to allow for sales. The factory pattern now allows for both.
@codecov
Copy link

codecov bot commented Sep 16, 2023

Codecov Report

Patch coverage: 55.09% and project coverage change: -0.30% ⚠️

Comparison is base (3518a28) 96.65% compared to head (9b9df6a) 96.35%.
Report is 1 commits behind head on development.

Additional details and impacted files
@@               Coverage Diff               @@
##           development     #750      +/-   ##
===============================================
- Coverage        96.65%   96.35%   -0.30%     
===============================================
  Files              186      203      +17     
  Lines            48253    49499    +1246     
===============================================
+ Hits             46637    47697    +1060     
- Misses            1616     1802     +186     
Files Changed Coverage Δ
...tracts/test/dao-proposal-hook-counter/src/error.rs 0.00% <ø> (ø)
...ontracts/test/dao-proposal-hook-counter/src/msg.rs 66.66% <ø> (ø)
...tracts/test/dao-proposal-hook-counter/src/state.rs 100.00% <ø> (ø)
contracts/test/dao-proposal-sudo/src/contract.rs 68.51% <ø> (ø)
contracts/test/dao-proposal-sudo/src/error.rs 0.00% <ø> (ø)
contracts/test/dao-proposal-sudo/src/msg.rs 40.00% <ø> (ø)
...ontracts/test/dao-test-custom-factory/src/error.rs 0.00% <0.00%> (ø)
...racts/test/dao-voting-cw20-balance/src/contract.rs 92.59% <ø> (ø)
...ontracts/test/dao-voting-cw20-balance/src/error.rs 100.00% <ø> (ø)
contracts/test/dao-voting-cw20-balance/src/msg.rs 50.00% <ø> (ø)
... and 30 more

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

@JakeHartnell JakeHartnell changed the title WIP Factory Pattern Factory Pattern Sep 17, 2023
@JakeHartnell JakeHartnell marked this pull request as ready for review September 17, 2023 01:15
@codecov
Copy link

codecov bot commented Sep 17, 2023

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@JakeHartnell JakeHartnell changed the title Factory Pattern Remaining Audit Fixes & Factory Pattern Sep 17, 2023
…add tests

We now validate supply regardless of what ActiveThreshold is set to.
@JakeHartnell JakeHartnell force-pushed the factory-pattern branch 5 times, most recently from 75c6d56 to 9cd3ee6 Compare October 3, 2023 18:04
- Extends dao-test-custom-factory to mint NFTs
- Validate ActiveThreshold in NFT factory test contract
- Add ModuleInstantiateCallback to nft factory call backs
- Fix transfer ownership in factory test contract
- Add ModuleInstantiateCallback to nft factory callbacks
- Test ownership set correctly in token factory factory
- Test for module instantiate callback in NFT factory
- Include note that custom factory contracts MUST handle validation logic

The most important change here is the that both
`dao-voting-cw721-staked` and `dao-voting-token-staked` implement
ModuleInstantiateCallback now, which allows for more complicated setup possibilities.
Copy link
Member

@NoahSaso NoahSaso left a comment

Choose a reason for hiding this comment

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

NOICE

@JakeHartnell JakeHartnell merged commit 0ba671c into development Oct 18, 2023
9 checks passed
@JakeHartnell JakeHartnell deleted the factory-pattern branch October 18, 2023 03:35
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