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

Save and share the state of deployed contracts #52

Closed
sideninja opened this issue Feb 5, 2021 · 11 comments
Closed

Save and share the state of deployed contracts #52

sideninja opened this issue Feb 5, 2021 · 11 comments
Assignees
Labels
Feature A new user feature or a new package API Feedback

Comments

@sideninja
Copy link
Contributor

sideninja commented Feb 5, 2021

Issue To Be Solved

When contracts are deployed using the project deploy we get addresses for each deployed contract in the result. These addresses are then used by any transactions or script that interacts with the deployed contracts, but currently, they need to be inserted in imports by hand.

We should have a way to save the addresses of deployed contracts in a state file.

Suggest A Solution

CLI new command for deployment allows us to deploy contracts with a simple command to any desired network. This greatly improves the ability to iterate and update but it would even further improve DevEx if any program could benefit from that work. I'm thinking of a way to save and share the state of deployed contracts.

One way of achieving this would be to have a json file containing newly deployed contracts mapping to their addresses and this file could then be included into any other program. Some kind of a manifest file as @psiemens said.

I think this could be a simple and generic approach that would work among any programming language. Manifests could contain addresses for different networks and potentially maybe even sdk-s could support parsing such a file.

I think we need to open discussion on this topic and get different feedback to create the best solution.

@sideninja sideninja added Feature A new user feature or a new package API Feedback labels Feb 5, 2021
@psiemens
Copy link
Contributor

psiemens commented Feb 22, 2021

I was working on the Kitty Items repo today and ran into two problems that made me think of this issue.

I updated all the Cadence files to import contracts by filename rather than address, so the files no longer specify placeholders like 0xFUNGIBLETOKEN.

  1. After making that change, I wanted a way to use these same files in the Go tests. However, I could no longer do the simple replace(0xFUNGIBLETOKEN, ...) approach. It'd be great if the CLI were to resolve all of the contract filenames and use the new address replacement logic that's now in the CLI to generate a version of the contracts, transactions and scripts rewritten with emulator addresses. It would then be fairly trivial to then automatically generate Go bindings that could be used in the tests.

  2. Similarly, I used a hacky solution to replace the contract filenames in the kitty-items-js project. If we had something like a manifest file (generated by the CLI), it'd be much easier to just parse that JSON file in the JS app and load all of the address mappings.

@MaxStalker I think your generator code also solves problem 2 above. Is that right?

Seeing as we have partial solutions to these problems in both Go and JS, I wonder if we can create a generalized solution that can easily be replicated in other languages.

Here's a rough example of what I'm thinking 👇

// transferKibble.cdc

import Kibble from "../contracts/Kibble.cdc"

...
// kittyItems.manifest.json

{
  "contracts": {
    "Kibble": {
      "testnet": "0xabcdef"
    }
  },
  "transactions": {
    "transferKibble": {
      "arguments": [
        {"name": "recipient", "type": "Address"},
        {"name": "amount", "type": "UFix64"}
      ],
      "code": "import Kibble from {Kibble} ... ",
    }
  },
  "scripts": {
    // ...
  }
}

@MaxStalker
Copy link
Contributor

I think we need to invite @turbolent as well, since that address would also affect language server ability to resolve it to actual contract.

Also, feels a bit like we are re-inventing ABIs with this approach 😅

@sideninja
Copy link
Contributor Author

sideninja commented Feb 22, 2021

I agree, and I think the manifest idea is great. The contracts field can reflect contract addresses on each network.

Transactions and scripts are not "deployable" so we need to do address replacement and save them somewhere. Saving to manifest is one option the other is to have some kind of build folder.

I think the build folder is worth considering since it feels familiar to developers (think webpack, tsc etc.) and it offers a nice separation of files the same way we had them in the source file. Of course, the build folder doesn't allow any other metadata for which manifest is the correct solution if there is a need for such a thing. Another benefit of the build folder is there is no parsing and logic to implement manifest file (which would potentially be implemented in many places for many programming languages - probably would need to be included in the library).

In either case, I think an important aspect is also to think of changes and how are those reflected in any solution. Redeployment of contracts should of course change the state but also change of scripts and transaction files should. I think we could later add some kind of watch feature. Before that developer should probably rerun the command.

Workflow with this functionality:

  • When a developer deploys contracts, state should be updated with new addresses. When they run flow project deploy all this should happen automatically.
  • When a developer changes script or transaction files there should be a command they need to run. Maybe some kind of build command (but in manifest case not build since it implies there is a build folder) or maybe update.

I think in any case there shouldn't be any manual work with the state since the state would be generated with CLI. By that I mean a developer shouldn't change anything in manifest or build files and should just have a CLI to generate that. That is because I don't think it is a good idea to have some things generated and some things manually inserted (problems with overwriting etc).

I think another thing to keep in mind is that our solution should work with task automation tools (grunt, gulp etc). This could even remove the need for us to implement a watch feature. Maybe this is even more appropriate.

@psiemens I wondering why you specify this in an example:

"arguments": [
      {"name": "recipient", "type": "Address"},
      {"name": "amount", "type": "UFix64"}
],

@joshuahannan
Copy link
Member

This sounds pretty great!

I left some comments on onflow/kitty-items#32 about this too, but I don't think this adequately solves some issues. In order to do it this way, you still have to copy and paste contracts and contract interfaces to your repos to import them, whereas we should have some way of importing them from another repo.

Same with providing an easy to use package for anyone to import the transaction templates from their app in any language. It looks like that's what you're talking about above though, so that is good. 👍

@sideninja
Copy link
Contributor Author

sideninja commented Feb 22, 2021

I think there are two separate issues (both important):

  • Once you deploy your contracts using the awesome new feature flow project deploy how do you execute your custom scripts/transactions without having to copy-paste previously deployed contract addresses in script/transactions imports (what this issue mostly targets)
  • You shouldn't have to copy-paste standard (or any other) contracts from github just for the deployment to work. I think this should be solved but I also think it can be solved separately since it opens more problems (vscode language processor should know how to parse remote imported contracts etc...). I will open an issue for this since I believe it is a major pain point when developing. @joshuahannan find more here: Import and deploy contracts from remote sources #71

@sideninja sideninja changed the title Leveraging CLI contract deployment in other programming languages Save and share the state of deployed contracts Feb 22, 2021
@sideninja sideninja self-assigned this Feb 23, 2021
@bjartek
Copy link
Collaborator

bjartek commented Feb 26, 2021

Sending adresses as arguments to scripts contracts would also be nice to think about here. Could there be an cadence.Address method that does this lookup for you?

@sideninja
Copy link
Contributor Author

Sending adresses as arguments to scripts contracts would also be nice to think about here. Could there be an cadence.Address method that does this lookup for you?

Can you please give some examples of usage so I better understand?

@bjartek
Copy link
Collaborator

bjartek commented Feb 26, 2021

https://github.com/versus-flow/auction-flow-contract/blob/master/examples/demo/main.go#L70

Transaction is signed as the marketplace account and the address of the artist is sent in as an argument.

@MaxStalker
Copy link
Contributor

@bjartek you mean you want to pass address as one of the arguments in the script? 🤔

@bjartek
Copy link
Collaborator

bjartek commented Feb 26, 2021

@MaxStalker yes. Ser the example I posted above.

@sideninja
Copy link
Contributor Author

https://github.com/versus-flow/auction-flow-contract/blob/master/examples/demo/main.go#L70

Transaction is signed as the marketplace account and the address of the artist is sent in as an argument.

I like your implementation, it reads really well. And yes ok I understand what you mean. Passing parameters to scripts is actually something @MaxStalker was working on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature A new user feature or a new package API Feedback
Projects
None yet
Development

No branches or pull requests

6 participants