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

flow-c1 init doesn't work with flow-go's local net #1669

Closed
illia-malachyn opened this issue Jul 8, 2024 · 13 comments · Fixed by #1671
Closed

flow-c1 init doesn't work with flow-go's local net #1669

illia-malachyn opened this issue Jul 8, 2024 · 13 comments · Fixed by #1671
Assignees

Comments

@illia-malachyn
Copy link

illia-malachyn commented Jul 8, 2024

Looks like some contracts are not updated to cadence1. FCL tries to install them on-chain and fails.

Steps to Reproduce

  1. check out the latest flow-go's master
  2. open the local net directory cd flow-go/integration/localnet
  3. start local net make bootstrap && make start
  4. run fcl flow-c1 init
  5. come up with a name for the project
  6. agree to install core contracts and their deps
  7. check FlowServiceAccount in the list
  8. press enter

You should see the error

flow-c1 version Version: v1.21.0-cadence-v1.0.0-preview.28

@illia-malachyn
Copy link
Author

illia-malachyn commented Jul 9, 2024

I assume the local net is not the cause as it fails during initialization. So, you can skip some steps mentioned above

@chasefleming
Copy link
Member

@turbolent do you have any insight into this? It's definitely failing because that contract is not upgraded to C1. It's pulled from the system contracts library.

@turbolent turbolent self-assigned this Jul 11, 2024
@turbolent
Copy link
Member

Looking into, can reproduce the problem with master of flow-go and flow-cli:

$ go run ./cmd/flow init
# github.com/karalabe/usb
In file included from ../../../go/pkg/mod/github.com/karalabe/usb@v0.0.2/libs.go:50:
../../../go/pkg/mod/github.com/karalabe/usb@v0.0.2/libusb/libusb/os/darwin_usb.c:53:29: warning: macro 'ATOMIC_VAR_INIT' has been marked as deprecated [-Wdeprecated-pragma]
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/15.0.0/include/stdatomic.h:54:41: note: macro marked 'deprecated' here
Enter the name of your project

> Test

account:
directoryWithBasePath:  /var/folders/n6/04ql0mr94nq5qj61wz_lcsx40000gn/T/flow-cli-92178280/cadence/contracts
filenameWithBasePath:  /var/folders/n6/04ql0mr94nq5qj61wz_lcsx40000gn/T/flow-cli-92178280/cadence/contracts/Counter.cdc
relativeFilenameWithBasePath:  cadence/contracts/Counter.cdc
account:
account:
✔ Yes
Select any core contracts you would like to install or skip to continue.
Use arrow keys to navigate, space to select, enter to confirm or skip, q to quit:

  [ ] FlowEpoch
  [ ] FlowIDTableStaking
  [ ] FlowClusterQC
  [ ] FlowDKG
> [x] FlowServiceAccount
  [ ] NodeVersionBeacon
  [ ] RandomBeaconHistory
  [ ] FlowStorageFees
  [ ] FlowFees
  [ ] FlowToken
  [ ] FungibleToken
  [ ] NonFungibleToken
  [ ] MetadataViews
  [ ] ViewResolver
  [ ] EVM

🔄 Installing selected core contracts and dependencies...
❌ Command Error: error processing dependency: failed to parse program: Parsing failed:
error: `pub` is no longer a valid access keyword
 --> :7:0
  |
7 | pub contract FlowServiceAccount {
  | ^^^

error: `pub` is no longer a valid access keyword
 --> :9:4
  |
9 |     pub event TransactionFeeUpdated(newFee: UFix64)
  |     ^^^

error: `pub` is no longer a valid access keyword
  --> :11:4
   |
11 |     pub event AccountCreationFeeUpdated(newFee: UFix64)
   |     ^^^

error: `pub` is no longer a valid access keyword
  --> :13:4
   |
13 |     pub event AccountCreatorAdded(accountCreator: Address)
   |     ^^^

error: `pub` is no longer a valid access keyword
  --> :15:4
   |
15 |     pub event AccountCreatorRemoved(accountCreator: Address)
   |     ^^^

error: `pub` is no longer a valid access keyword
  --> :17:4
   |
17 |     pub event IsAccountCreationRestrictedUpdated(isRestricted: Bool)
   |     ^^^

error: `pub` is no longer a valid access keyword
  --> :20:4
   |
20 |     pub var transactionFee: UFix64
   |     ^^^

error: `pub` is no longer a valid access keyword
  --> :23:4
   |
23 |     pub var accountCreationFee: UFix64
   |     ^^^

error: `pub` is no longer a valid access keyword
  --> :29:4
   |
29 |     pub fun initDefaultToken(_ acct: AuthAccount) {
   |     ^^^

error: restricted types have been removed; replace with the concrete type or an equivalent intersection type
  --> :35:35
   |
35 |         acct.link<&FlowToken.Vault{FungibleToken.Receiver}>(
   |                                    ^^^^^^^^^^^^^

@turbolent
Copy link
Member

turbolent commented Jul 11, 2024

I'm new to the flow init feature and the deployment of core contracts.

Why is it prompting the user to deploy any of these contracts? These core contracts are all part of the bootstrapping process, i.e. deployed when the network starts, as the protocol needs them to function.

Also, why are the contracts hard-coded to the Mainnet variants? It looks like that causes the contracts to be pulled from Mainnet – and those contracts are not updated to Cadence 1.0 yet.

It looks like this was added in PR #1517, and the issue for it, #1482, doesn't mention the reason why this should be done.

I'd say the fix is to remove the core contract deployment – unless I'm missing something, it is not only not needed, but won't work.

cc @onflow/flow-cadence-execution

@chasefleming
Copy link
Member

chasefleming commented Jul 12, 2024

init doesn't actually deploy any contracts. It uses Dependency Manager to add them to flow.json for easy setup. That also looks for any dependencies the contract you're installing has and sets those up for you as well to make your life easier. But I think I realized what the issue is! By default when init uses Dependency Manager under the hood it's set to look at mainnet, but now we have different networks with different language versions. I think if we change the default network to be checking contracts and their dependencies to be previewnet it might solve it.

@turbolent
Copy link
Member

I see! Thanks for the explanation 👍

I think if we change the default network to be checking contracts and their dependencies to be previewnet it might solve it.

Yeah, that's what I was wondering about above:

Also, why are the contracts hard-coded to the Mainnet variants? It looks like that causes the contracts to be pulled from Mainnet – and those contracts are not updated to Cadence 1.0 yet.

Maybe we need to change

sc := systemcontracts.SystemContractsForChain(flowGo.Mainnet)

especially for Cadence 1.0, until it's deployed on TN and MN?

@turbolent
Copy link
Member

turbolent commented Jul 12, 2024

init doesn't actually deploy any contracts. It uses Dependency Manager to add them to flow.json for easy setup.

error processing dependency: failed to parse program

Why does it parse the programs? To get all the imports?

@chasefleming
Copy link
Member

chasefleming commented Jul 12, 2024

Yep, I changed that and it appears to get around the issue, but a new issue comes up where it repeatedly asks about the Burner dependency that is a dependency of one of its imports.

I started a draft PR with the network change, but we probably shouldn't merge until we figure out the Burner issue.

And yeah it parses the programs to get the imports of a contract so you don't have to manually copy/paste every contract and its dependencies and then add to your flow.json. Big time saver. More about it on the docs here: https://developers.flow.com/tools/flow-cli/dependency-manager

@chasefleming
Copy link
Member

I think I might have fixed all the additional issues related to the addition of the previewnet network on this feature. One of the main ones being it now needed to know which aliases needed to be asked for depending on what network is was pulling from. Give this branch a try if you can. The second issue being it kept asking for Burner over and over.

@gregsantos @jribbink do you mind reviewing/testing this as well since you are familiar with its behavior? Thanks.

@turbolent turbolent assigned chasefleming and unassigned turbolent Jul 12, 2024
@turbolent
Copy link
Member

Nice! For the Burner contract, we probably can just add it to the SystemContracts in flow-go?

@jribbink
Copy link
Contributor

Nice! For the Burner contract, we probably can just add it to the SystemContracts in flow-go?

I've made a PR 👍 onflow/flow-go#6211

@chasefleming
Copy link
Member

Thanks for adding that @jribbink !

@chasefleming
Copy link
Member

Going to merge #1671 , but heads up there will still be some minor non-ideal issues until we have a flow go update with the new system contracts update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants