Cadence 1.0 Multi-Staging

jribbink commented 4 months ago

Closes #1559

Description

Adds support for staging multiple contracts at once.

Approach:

Encapsulate staging functionality in "Staging Service"
Staging service can stage multiple contracts at once
Staging validator adapted to allow multiple contracts, returns separate errors per contract
Local dependency tree of all local/remote checkers is built, everything is cached (contracts, checkers, etc). Checking remote contracts is very expensive, so caching is important. Additionally, it's important that all contracts are validated together, because this way staging order is irrelevant (e.g. validation is done using local copies).

For contributor use:

[ ] Targeted PR against master branch
[x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work
[x] Code follows the standards mentioned here
[x] Updated relevant documentation
[ ] Re-reviewed Files changed in the Github PR explorer
[x] Added appropriate labels

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 59.18762% with 211 lines in your changes are missing coverage. Please review.

Project coverage is 43.54%. Comparing base (300d082) to head (718d898).

Files	Patch %	Lines
internal/migrate/stage.go	21.52%	103 Missing and 10 partials :warning:
internal/migrate/staging_validator.go	75.71%	42 Missing and 9 partials :warning:
internal/migrate/staging_service.go	79.06%	15 Missing and 12 partials :warning:
internal/cadence/lint.go	0.00%	10 Missing :warning:
internal/migrate/get_staged_code.go	54.54%	6 Missing and 4 partials :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## feature/stable-cadence #1582 +/- ## ========================================================== + Coverage 41.87% 43.54% +1.66% ========================================================== Files 59 60 +1 Lines 3981 4304 +323 ========================================================== + Hits 1667 1874 +207 - Misses 2079 2181 +102 - Partials 235 249 +14 ``` | [Flag](https://app.codecov.io/gh/onflow/flow-cli/pull/1582/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onflow) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/onflow/flow-cli/pull/1582/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onflow) | `43.54% <59.18%> (+1.66%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onflow#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bjartek commented 4 months ago

Just tried to run this on find repo since I have 45 things to stage there and i got

flow migrate stage-contract --all -n testnet
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
                           ⚠ Upgrade to Cadence 1.0
     The Crescendo network upgrade, including Cadence 1.0, is coming soon.
     You may need to update your existing contracts to support this change.
                     Please visit our migration guide here:
             https://cadence-lang.org/docs/cadence_migration_guide
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
panic: interface conversion: common.Location is common.IdentifierLocation, not common.AddressLocation

goroutine 1 [running]:
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).parseAndCheckAllStaged.func1({0x104c13e20, 0x140025977a0})
    github.com/onflow/flow-cli/internal/migrate/staging_validator.go:266 +0x128
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).forEachDependency.func1({0x104c13de0, 0x140024d97e0})
    github.com/onflow/flow-cli/internal/migrate/staging_validator.go:701 +0xfc
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).forEachDependency.func1({0x104c13de0, 0x14002ac2720})
    github.com/onflow/flow-cli/internal/migrate/staging_validator.go:702 +0x114
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).forEachDependency(0x14000d477c0, {{{0x140014d0400, 0xb}, {0x3e, 0x5b, 0x4c, 0x62, 0x70, 0x64, 0x62, ...}}, ...}, ...)
    github.com/onflow/flow-cli/internal/migrate/staging_validator.go:706 +0xd0
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).parseAndCheckAllStaged(0x14000d477c0)
    github.com/onflow/flow-cli/internal/migrate/staging_validator.go:264 +0x1e0
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).Validate(0x14000d477c0, {0x14000238608, 0x2e, 0x1?})
    github.com/onflow/flow-cli/internal/migrate/staging_validator.go:178 +0x1a8
github.com/onflow/flow-cli/internal/migrate.(*stagingServiceImpl).validateAndStageContracts(0x14000d47800, {0x104c0eb60, 0x1061543e0}, {0x14000238608, 0x2e, 0x4b})
    github.com/onflow/flow-cli/internal/migrate/staging_service.go:101 +0x11c
github.com/onflow/flow-cli/internal/migrate.(*stagingServiceImpl).StageContracts(0x14000d47800, {0x104c0eb60, 0x1061543e0}, {0x14000c31a00?, 0x2e, 0x0?})
    github.com/onflow/flow-cli/internal/migrate/staging_service.go:92 +0x80
github.com/onflow/flow-cli/internal/migrate.stageAll({0x104c000f0, 0x14000d47800}, 0x14000fbcf30, {0x104c359d8?, 0x14000d171a0?})
    github.com/onflow/flow-cli/internal/migrate/stage_contract.go:135 +0xbc
github.com/onflow/flow-cli/internal/migrate.stageContract({0x14000d4b410, 0x0, 0x3}, {{0x0, 0x0}, {0x103ebd395, 0x4}, {0x0, 0x0}, {0x0, ...}, ...}, ...)
    github.com/onflow/flow-cli/internal/migrate/stage_contract.go:92 +0x3cc
github.com/onflow/flow-cli/internal/command.Command.AddToParent.func1(0x14000d51100?, {0x14000d4b410, 0x0, 0x3})
    github.com/onflow/flow-cli/internal/command/command.go:142 +0x4cc
github.com/spf13/cobra.(*Command).execute(0x105fbd4a0, {0x140001b8150, 0x3, 0x3})
    github.com/spf13/cobra@v1.8.0/command.go:987 +0x828
github.com/spf13/cobra.(*Command).ExecuteC(0x1400055d808)
    github.com/spf13/cobra@v1.8.0/command.go:1115 +0x344
github.com/spf13/cobra.(*Command).Execute(...)
    github.com/spf13/cobra@v1.8.0/command.go:1039
main.main()
    github.com/onflow/flow-cli/cmd/flow/main.go:168 +0xd9c

jribbink commented 4 months ago

Just tried to run this on find repo since I have 45 things to stage there and i got

flow migrate stage-contract --all -n testnet
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
                           ⚠ Upgrade to Cadence 1.0
     The Crescendo network upgrade, including Cadence 1.0, is coming soon.
     You may need to update your existing contracts to support this change.
                     Please visit our migration guide here:
             https://cadence-lang.org/docs/cadence_migration_guide
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
panic: interface conversion: common.Location is common.IdentifierLocation, not common.AddressLocation

goroutine 1 [running]:
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).parseAndCheckAllStaged.func1({0x104c13e20, 0x140025977a0})
  github.com/onflow/flow-cli/internal/migrate/staging_validator.go:266 +0x128
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).forEachDependency.func1({0x104c13de0, 0x140024d97e0})
  github.com/onflow/flow-cli/internal/migrate/staging_validator.go:701 +0xfc
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).forEachDependency.func1({0x104c13de0, 0x14002ac2720})
  github.com/onflow/flow-cli/internal/migrate/staging_validator.go:702 +0x114
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).forEachDependency(0x14000d477c0, {{{0x140014d0400, 0xb}, {0x3e, 0x5b, 0x4c, 0x62, 0x70, 0x64, 0x62, ...}}, ...}, ...)
  github.com/onflow/flow-cli/internal/migrate/staging_validator.go:706 +0xd0
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).parseAndCheckAllStaged(0x14000d477c0)
  github.com/onflow/flow-cli/internal/migrate/staging_validator.go:264 +0x1e0
github.com/onflow/flow-cli/internal/migrate.(*stagingValidatorImpl).Validate(0x14000d477c0, {0x14000238608, 0x2e, 0x1?})
  github.com/onflow/flow-cli/internal/migrate/staging_validator.go:178 +0x1a8
github.com/onflow/flow-cli/internal/migrate.(*stagingServiceImpl).validateAndStageContracts(0x14000d47800, {0x104c0eb60, 0x1061543e0}, {0x14000238608, 0x2e, 0x4b})
  github.com/onflow/flow-cli/internal/migrate/staging_service.go:101 +0x11c
github.com/onflow/flow-cli/internal/migrate.(*stagingServiceImpl).StageContracts(0x14000d47800, {0x104c0eb60, 0x1061543e0}, {0x14000c31a00?, 0x2e, 0x0?})
  github.com/onflow/flow-cli/internal/migrate/staging_service.go:92 +0x80
github.com/onflow/flow-cli/internal/migrate.stageAll({0x104c000f0, 0x14000d47800}, 0x14000fbcf30, {0x104c359d8?, 0x14000d171a0?})
  github.com/onflow/flow-cli/internal/migrate/stage_contract.go:135 +0xbc
github.com/onflow/flow-cli/internal/migrate.stageContract({0x14000d4b410, 0x0, 0x3}, {{0x0, 0x0}, {0x103ebd395, 0x4}, {0x0, 0x0}, {0x0, ...}, ...}, ...)
  github.com/onflow/flow-cli/internal/migrate/stage_contract.go:92 +0x3cc
github.com/onflow/flow-cli/internal/command.Command.AddToParent.func1(0x14000d51100?, {0x14000d4b410, 0x0, 0x3})
  github.com/onflow/flow-cli/internal/command/command.go:142 +0x4cc
github.com/spf13/cobra.(*Command).execute(0x105fbd4a0, {0x140001b8150, 0x3, 0x3})
  github.com/spf13/cobra@v1.8.0/command.go:987 +0x828
github.com/spf13/cobra.(*Command).ExecuteC(0x1400055d808)
  github.com/spf13/cobra@v1.8.0/command.go:1115 +0x344
github.com/spf13/cobra.(*Command).Execute(...)
  github.com/spf13/cobra@v1.8.0/command.go:1039
main.main()
  github.com/onflow/flow-cli/cmd/flow/main.go:168 +0xd9c

Awesome, thank you so much for doing this. Looks like an easy fix, glad we could catch it before it became an issue 🙏

bjartek commented 4 months ago

this is the output now

 flow-ms migrate stage-contract --all -n testnet
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
                           ⚠ Upgrade to Cadence 1.0
     The Crescendo network upgrade, including Cadence 1.0, is coming soon.
     You may need to update your existing contracts to support this change.
                     Please visit our migration guide here:
             https://cadence-lang.org/docs/cadence_migration_guide
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
❌ Preliminary validation could not be performed on the following contracts:
  - 35717efbbce11c74.FindMarketDirectOfferEscrow
  - 3e5b4c627064625d.PartyFavorz
  - 35717efbbce11c74.FindLeaseMarketAuctionSoft
  - 35717efbbce11c74.FindMarketAuctionSoft
  - 35717efbbce11c74.FindLeaseMarketSale
  - 35717efbbce11c74.Dandy
  - 35717efbbce11c74.FindForgeOrder
  - 35717efbbce11c74.FindFurnace
  - 35717efbbce11c74.FindMarketDirectOfferSoft
  - 35717efbbce11c74.FindForge
  - 35717efbbce11c74.FindMarketSale
  - 35717efbbce11c74.FindLeaseMarketDirectOfferSoft
  - 35717efbbce11c74.FindThoughts
  - 3e5b4c627064625d.NFGv3
  - 35717efbbce11c74.FindLeaseMarket
  - 35717efbbce11c74.FindVerifier
  - 35717efbbce11c74.FindMarketAuctionEscrow
  - 35717efbbce11c74.FindLostAndFoundWrapper
  - 35717efbbce11c74.FIND
  - 35717efbbce11c74.FindMarketAdmin
  - 35717efbbce11c74.Admin
  - 35717efbbce11c74.FindAirdropper
  - 35717efbbce11c74.NameVoucher
  - 3e5b4c627064625d.Flomies
  - 35717efbbce11c74.FindPack
  - 3e5b4c627064625d.GeneratedExperiences

These contracts depend on the following contracts which have not been staged yet:
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT

You may still stage your contract, however it will be unable to be migrated until the missing contracts are staged by their respective owners.  It is important to monitor the status of your contract using the `flow migrate is-validated` command

should we sort and unique the dependent contracts maybe?

jribbink commented 4 months ago

this is the output now

 flow-ms migrate stage-contract --all -n testnet
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
                           ⚠ Upgrade to Cadence 1.0
     The Crescendo network upgrade, including Cadence 1.0, is coming soon.
     You may need to update your existing contracts to support this change.
                     Please visit our migration guide here:
             https://cadence-lang.org/docs/cadence_migration_guide
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
❌ Preliminary validation could not be performed on the following contracts:
  - 35717efbbce11c74.FindMarketDirectOfferEscrow
  - 3e5b4c627064625d.PartyFavorz
  - 35717efbbce11c74.FindLeaseMarketAuctionSoft
  - 35717efbbce11c74.FindMarketAuctionSoft
  - 35717efbbce11c74.FindLeaseMarketSale
  - 35717efbbce11c74.Dandy
  - 35717efbbce11c74.FindForgeOrder
  - 35717efbbce11c74.FindFurnace
  - 35717efbbce11c74.FindMarketDirectOfferSoft
  - 35717efbbce11c74.FindForge
  - 35717efbbce11c74.FindMarketSale
  - 35717efbbce11c74.FindLeaseMarketDirectOfferSoft
  - 35717efbbce11c74.FindThoughts
  - 3e5b4c627064625d.NFGv3
  - 35717efbbce11c74.FindLeaseMarket
  - 35717efbbce11c74.FindVerifier
  - 35717efbbce11c74.FindMarketAuctionEscrow
  - 35717efbbce11c74.FindLostAndFoundWrapper
  - 35717efbbce11c74.FIND
  - 35717efbbce11c74.FindMarketAdmin
  - 35717efbbce11c74.Admin
  - 35717efbbce11c74.FindAirdropper
  - 35717efbbce11c74.NameVoucher
  - 3e5b4c627064625d.Flomies
  - 35717efbbce11c74.FindPack
  - 3e5b4c627064625d.GeneratedExperiences

These contracts depend on the following contracts which have not been staged yet:
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - a983fecbed621163.FiatToken
  - 0afe396ebc8eee65.FLOAT

You may still stage your contract, however it will be unable to be migrated until the missing contracts are staged by their respective owners.  It is important to monitor the status of your contract using the `flow migrate is-validated` command

should we sort and unique the dependent contracts maybe?

Yeah 💯 (I was also planning not to stage contracts if the staged code hasn't changed, just have not added yet)

bjartek commented 4 months ago

It did work though, but it stages each contract in a single transaction. Each tx takes about 300 gas so you should be able to do loads of them in one go if you want to.

The command took 5 minutes to run for me :p

jribbink commented 4 months ago

It did work though, but it stages each contract in a single transaction. Each tx takes about 300 gas so you should be able to do loads of them in one go if you want to.

The command took 5 minutes to run for me :p

Awesome!! Super appreciative that you could run this, it really helps because my manual + unit tests can only go so far... lots of edge cases in this one.

Yeah, doing this in separate transactions is somewhat intentional. I didn't want to run into any headaches related to computation limits, so it seemed more reliable to just stick to individual TX than try to estimate computation & potentially introduce bugs (and since this is what flow project deploy does as well anyways).

I know waiting 5 minutes for the command to run isn't a great experience, but hopefully I can really shave that down by not sending unnecessary TX.

bjartek commented 4 months ago

Fair enough. I dont expect to run it that many times

onflow / flow-cli

Cadence 1.0 Multi-Staging #1582

Description

Codecov Report