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

FungibleToken Factory Manager Setup Transaction #152

Merged
merged 7 commits into from
Sep 1, 2023

Conversation

sisyphusSmiling
Copy link
Collaborator

Closes: #151

@sisyphusSmiling sisyphusSmiling added the enhancement New feature or request label Aug 23, 2023
@sisyphusSmiling sisyphusSmiling requested review from austinkline and a team August 23, 2023 21:47
@sisyphusSmiling sisyphusSmiling self-assigned this Aug 23, 2023
@sisyphusSmiling sisyphusSmiling marked this pull request as ready for review August 23, 2023 22:10
@austinkline
Copy link
Collaborator

Nice to see this one! It looks like we ended up adding some other types for to our flowty-maintained testnet resource
https://f.dnz.dev/0x5617ff347e16bd72/storage/CapabilityFactory_0x294e44e1ec6993c6

Are we missing factories for some of these types? I'll look around at standard contracts to see if the missing combinations are generally in use

@sisyphusSmiling
Copy link
Collaborator Author

Looks like there's a contract FTFactories defining a number of FT-related factory implementations. The addition to the hosted Manager you linked looks to be a {Receiver, Balance} Capability.

I do like consolidating the factories into a single contract, but kinda late into the game to switch up the pattern. What do you think about deploying another contract defining such a Factory?

@austinkline
Copy link
Collaborator

I do like consolidating the factories into a single contract, but kinda late into the game to switch up the pattern. What do you think about deploying another contract defining such a Factory?

Yeah... I didn't even think to consolidate them until I needed something for testing and threw it all together 🙁 I think it's totally fine to stick with how we're doing it and make a new contract. They're tiny, which means it's easy for a reader to understand exactly what it's for

Copy link
Member

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

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

have a test for this somewhere?

@sisyphusSmiling
Copy link
Collaborator Author

sisyphusSmiling commented Aug 25, 2023

No worries @austinkline, I didn't consider it either. I'll add the contract to this PR and can add the new Factory to the hosted Mainnet Manager.

Good callout @joshuahannan I'll add test cases

@codecov-commenter
Copy link

Codecov Report

Merging #152 (6eb2721) into main (dd77f1b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #152   +/-   ##
=======================================
  Coverage   82.50%   82.50%           
=======================================
  Files           4        4           
  Lines         343      343           
=======================================
  Hits          283      283           
  Misses         60       60           

@sisyphusSmiling sisyphusSmiling merged commit 604e872 into main Sep 1, 2023
1 check passed
@sisyphusSmiling sisyphusSmiling deleted the ft-factory-manager-setup branch September 1, 2023 15:52
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.

Deploy a FungibleToken Factory Manager
4 participants