Update bootstrapping to include nft and metadata views #4536

joshuahannan · 2023-07-03T17:01:14Z

In onflow/flow-core-contracts#370, we're adding metadata views to the FlowToken smart contract. For this, the contract needs to implement all the metadata views contracts. This PR updates the bootstrapping logic to deploy NonFungibleToken, MetadataViews, ViewResolver, and FungibleTokenMetadataViews and provide those import addresses to the FlowToken smart contract deployment.

joshuahannan · 2023-07-03T18:17:32Z

@ramtinms @janezpodhostnik Would one of you be able to get on a quick call with me so I can explain these changes to you? I'm getting a bunch of errors when running the tests and I'm not sure what is going wrong, so I could use some guidance about how everything fits together.

bluesign · 2023-07-03T19:26:02Z

fvm/bootstrap.go

@@ -411,14 +413,55 @@ func (b *bootstrapExecutor) deployFungibleToken() flow.Address {
 	return fungibleToken
 }

-func (b *bootstrapExecutor) deployFlowToken(service, fungibleToken flow.Address) flow.Address {
+func (b *bootstrapExecutor) deployNonFungibleToken() flow.Address {
+	nonFungibleToken := b.createAccount(b.accountKeys.FungibleTokenAccountPublicKeys)


this can break stuff with on emulator, changes flowtoken address.

does it break it because the addresses are assigned in order? Is there any way around this?

I guess we could create the FlowToken account first, then deploy the other contracts to other accounts, then deploy FlowToken after that to the first account created

creating accounts in advance should make it backwards compatible I guess.

flow-go/fvm/bootstrap.go

Lines 318 to 326 in 7dda552

service := b.createServiceAccount()

fungibleToken := b.deployFungibleToken()

nonFungibleToken := b.deployNonFungibleToken()

b.deployMetadataViews(fungibleToken, nonFungibleToken)

flowToken := b.deployFlowToken(service, fungibleToken, nonFungibleToken)

storageFees := b.deployStorageFees(service, fungibleToken, flowToken)

feeContract := b.deployFlowFees(service, fungibleToken, flowToken, storageFees)

Core contracts have fixed indexes https://github.com/onflow/flow-go/blob/91da6d0b24d1d5a5e156c3616f1179b4951d19a6/fvm/environment/account_creator.go#L15C1-L20C1
better not to disturb them.

you can create all accounts (like serviceAccount ), then reference them in the functions. (by passing as parameter )

something like this:

service := b.createServiceAccount() fungibleToken := b.deployFungibleToken() //create those to not change index flowTokenAccount := b.createAccount(....) flowFeesAccount := b.createAccount(....) nonFungibleToken := b.deployNonFungibleToken() b.deployMetadataViews(fungibleToken, nonFungibleToken) flowToken := b.deployFlowToken(service, flowTokenAccount, fungibleToken, nonFungibleToken) storageFees := b.deployStorageFees(service, fungibleToken, flowToken) feeContract := b.deployFlowFees(service, flowFeesAccount, fungibleToken, flowToken, storageFees)

we can also deploy metadata views and non fungible tokens on the first/service account

@janezpodhostnik actually this is much better, then we would be totally compatible with emulator.

codecov-commenter · 2023-07-04T15:46:45Z

Codecov Report

Merging #4536 (aa528ad) into master (2528190) will decrease coverage by 1.79%.
The diff coverage is 53.49%.

@@            Coverage Diff             @@
##           master    #4536      +/-   ##
==========================================
- Coverage   56.25%   54.47%   -1.79%     
==========================================
  Files         653      914     +261     
  Lines       64699    85238   +20539     
==========================================
+ Hits        36396    46430   +10034     
- Misses      25362    35218    +9856     
- Partials     2941     3590     +649

Flag	Coverage Δ
unittests	`54.47% <53.49%> (-1.79%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cmd/scaffold.go	`14.83% <0.00%> (+0.06%)`	⬆️
engine/access/rpc/backend/backend_transactions.go	`49.01% <0.00%> (+2.86%)`	⬆️
model/flow/identifierList.go	`22.36% <0.00%> (+0.84%)`	⬆️
model/verification/chunkDataPackRequest.go	`43.75% <0.00%> (-6.25%)`	⬇️
module/metrics/herocache.go	`0.00% <0.00%> (ø)`
module/metrics/network.go	`0.00% <0.00%> (ø)`
module/metrics/noop.go	`0.00% <0.00%> (ø)`
network/p2p/inspector/internal/cache/cache.go	`64.70% <ø> (ø)`
network/p2p/middleware/middleware.go	`1.71% <0.00%> (ø)`
network/p2p/p2pbuilder/libp2pNodeBuilder.go	`0.00% <0.00%> (ø)`
... and 29 more

... and 244 files with indirect coverage changes

janezpodhostnik · 2023-07-04T17:36:47Z

Looks like the emulator needs to be upgraded with this version of flow-go then the integration tests need to be updated with that emulator version.

joshuahannan · 2023-07-13T15:58:19Z

I merged the PR in the core contracts repo. What do we need to do to get this PR merged?

janezpodhostnik · 2023-07-14T15:12:17Z

I merged the PR in the core contracts repo. What do we need to do to get this PR merged?

We need another review.

janezpodhostnik · 2023-07-14T18:46:36Z

bors merge
I'm not sure whats up with benchstat, I will investigate if it keeps failing

4536: Update bootstrapping to include nft and metadata views r=janezpodhostnik a=joshuahannan In onflow/flow-core-contracts#370, we're adding metadata views to the `FlowToken` smart contract. For this, the contract needs to implement all the metadata views contracts. This PR updates the bootstrapping logic to deploy `NonFungibleToken`, `MetadataViews`, `ViewResolver`, and `FungibleTokenMetadataViews` and provide those import addresses to the `FlowToken` smart contract deployment. Co-authored-by: Josh Hannan <hannanjoshua19@gmail.com> Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com> Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>

bors · 2023-07-14T19:13:29Z

Build failed:

Integration Tests (make -C integration bft-tests)

janezpodhostnik · 2023-07-14T19:27:17Z

bors merge

4536: Update bootstrapping to include nft and metadata views r=janezpodhostnik a=joshuahannan In onflow/flow-core-contracts#370, we're adding metadata views to the `FlowToken` smart contract. For this, the contract needs to implement all the metadata views contracts. This PR updates the bootstrapping logic to deploy `NonFungibleToken`, `MetadataViews`, `ViewResolver`, and `FungibleTokenMetadataViews` and provide those import addresses to the `FlowToken` smart contract deployment. Co-authored-by: Josh Hannan <hannanjoshua19@gmail.com> Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com> Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>

bors · 2023-07-14T19:37:52Z

Build failed:

Integration Tests (make -C integration mvp-tests)

janezpodhostnik · 2023-07-17T12:41:10Z

bors merge

4536: Update bootstrapping to include nft and metadata views r=janezpodhostnik a=joshuahannan In onflow/flow-core-contracts#370, we're adding metadata views to the `FlowToken` smart contract. For this, the contract needs to implement all the metadata views contracts. This PR updates the bootstrapping logic to deploy `NonFungibleToken`, `MetadataViews`, `ViewResolver`, and `FungibleTokenMetadataViews` and provide those import addresses to the `FlowToken` smart contract deployment. Co-authored-by: Josh Hannan <hannanjoshua19@gmail.com> Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com> Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>

bors · 2023-07-17T13:02:57Z

Build failed:

Unit Tests (network)

janezpodhostnik · 2023-07-17T14:20:07Z

bors merge

4536: Update bootstrapping to include nft and metadata views r=janezpodhostnik a=joshuahannan In onflow/flow-core-contracts#370, we're adding metadata views to the `FlowToken` smart contract. For this, the contract needs to implement all the metadata views contracts. This PR updates the bootstrapping logic to deploy `NonFungibleToken`, `MetadataViews`, `ViewResolver`, and `FungibleTokenMetadataViews` and provide those import addresses to the `FlowToken` smart contract deployment. Co-authored-by: Josh Hannan <hannanjoshua19@gmail.com> Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com> Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>

bors · 2023-07-17T15:25:23Z

Timed out.

janezpodhostnik · 2023-07-17T15:25:55Z

bors merge

4536: Update bootstrapping to include nft and metadata views r=janezpodhostnik a=joshuahannan In onflow/flow-core-contracts#370, we're adding metadata views to the `FlowToken` smart contract. For this, the contract needs to implement all the metadata views contracts. This PR updates the bootstrapping logic to deploy `NonFungibleToken`, `MetadataViews`, `ViewResolver`, and `FungibleTokenMetadataViews` and provide those import addresses to the `FlowToken` smart contract deployment. Co-authored-by: Josh Hannan <hannanjoshua19@gmail.com> Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com> Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>

bors · 2023-07-17T15:31:47Z

Build failed:

Integration Tests (make -C integration collection-tests)

janezpodhostnik · 2023-07-17T16:35:48Z

bors merge

4536: Update bootstrapping to include nft and metadata views r=janezpodhostnik a=joshuahannan In onflow/flow-core-contracts#370, we're adding metadata views to the `FlowToken` smart contract. For this, the contract needs to implement all the metadata views contracts. This PR updates the bootstrapping logic to deploy `NonFungibleToken`, `MetadataViews`, `ViewResolver`, and `FungibleTokenMetadataViews` and provide those import addresses to the `FlowToken` smart contract deployment. Co-authored-by: Josh Hannan <hannanjoshua19@gmail.com> Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com> Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>

bors · 2023-07-17T16:57:33Z

Build failed:

Unit Tests (network)

joshuahannan requested review from ramtinms and janezpodhostnik as code owners July 3, 2023 17:01

bluesign reviewed Jul 3, 2023

View reviewed changes

joshuahannan requested review from gomisha and yhassanzadeh13 as code owners July 3, 2023 20:05

janezpodhostnik mentioned this pull request Jul 4, 2023

Upgrade flow-go to include bootstrapping with NFT and Metadata Views onflow/flow-emulator#443

Merged

6 tasks

joshuahannan and others added 6 commits July 5, 2023 15:58

update bootstrapping to include nft and metadata views

4c2f32c

update dependencies

1583edf

running tests

9af82d3

fix build and deploy NFT on service account

70edc6c

upgrade emulator

3418422

make tidy

e5669ef

janezpodhostnik force-pushed the bootstrapping-views branch from aab111f to e5669ef Compare July 5, 2023 14:17

janezpodhostnik approved these changes Jul 5, 2023

View reviewed changes

ramtinms approved these changes Jul 14, 2023

View reviewed changes

Merge branch 'master' into bootstrapping-views

fd4ffaf

Merge branch 'master' into bootstrapping-views

aa528ad

janezpodhostnik force-pushed the bootstrapping-views branch from d99178a to aa528ad Compare July 18, 2023 15:04

janezpodhostnik merged commit e3411ef into master Jul 18, 2023

janezpodhostnik deleted the bootstrapping-views branch July 18, 2023 16:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update bootstrapping to include nft and metadata views #4536

Update bootstrapping to include nft and metadata views #4536

joshuahannan commented Jul 3, 2023

joshuahannan commented Jul 3, 2023

bluesign Jul 3, 2023

joshuahannan Jul 3, 2023

joshuahannan Jul 3, 2023

bluesign Jul 3, 2023

bluesign Jul 3, 2023 •

edited

Loading

bluesign Jul 3, 2023

janezpodhostnik Jul 3, 2023

bluesign Jul 3, 2023

codecov-commenter commented Jul 4, 2023 •

edited

Loading

janezpodhostnik commented Jul 4, 2023

joshuahannan commented Jul 13, 2023

janezpodhostnik commented Jul 14, 2023

janezpodhostnik commented Jul 14, 2023

bors bot commented Jul 14, 2023

janezpodhostnik commented Jul 14, 2023

bors bot commented Jul 14, 2023

janezpodhostnik commented Jul 17, 2023

bors bot commented Jul 17, 2023

janezpodhostnik commented Jul 17, 2023

bors bot commented Jul 17, 2023

janezpodhostnik commented Jul 17, 2023

bors bot commented Jul 17, 2023

janezpodhostnik commented Jul 17, 2023

bors bot commented Jul 17, 2023

	service := b.createServiceAccount()

	fungibleToken := b.deployFungibleToken()
	nonFungibleToken := b.deployNonFungibleToken()
	b.deployMetadataViews(fungibleToken, nonFungibleToken)
	flowToken := b.deployFlowToken(service, fungibleToken, nonFungibleToken)
	storageFees := b.deployStorageFees(service, fungibleToken, flowToken)
	feeContract := b.deployFlowFees(service, fungibleToken, flowToken, storageFees)

Update bootstrapping to include nft and metadata views #4536

Update bootstrapping to include nft and metadata views #4536

Conversation

joshuahannan commented Jul 3, 2023

joshuahannan commented Jul 3, 2023

bluesign Jul 3, 2023

Choose a reason for hiding this comment

joshuahannan Jul 3, 2023

Choose a reason for hiding this comment

joshuahannan Jul 3, 2023

Choose a reason for hiding this comment

bluesign Jul 3, 2023

Choose a reason for hiding this comment

bluesign Jul 3, 2023 • edited Loading

Choose a reason for hiding this comment

bluesign Jul 3, 2023

Choose a reason for hiding this comment

janezpodhostnik Jul 3, 2023

Choose a reason for hiding this comment

bluesign Jul 3, 2023

Choose a reason for hiding this comment

codecov-commenter commented Jul 4, 2023 • edited Loading

Codecov Report

janezpodhostnik commented Jul 4, 2023

joshuahannan commented Jul 13, 2023

janezpodhostnik commented Jul 14, 2023

janezpodhostnik commented Jul 14, 2023

bors bot commented Jul 14, 2023

janezpodhostnik commented Jul 14, 2023

bors bot commented Jul 14, 2023

janezpodhostnik commented Jul 17, 2023

bors bot commented Jul 17, 2023

janezpodhostnik commented Jul 17, 2023

bors bot commented Jul 17, 2023

janezpodhostnik commented Jul 17, 2023

bors bot commented Jul 17, 2023

janezpodhostnik commented Jul 17, 2023

bors bot commented Jul 17, 2023

bluesign Jul 3, 2023 •

edited

Loading

codecov-commenter commented Jul 4, 2023 •

edited

Loading