Skip to content

op-deployer: add intent-config-type#12970

Merged
mslipper merged 14 commits intodevelopfrom
ss/intent-config-type
Nov 27, 2024
Merged

op-deployer: add intent-config-type#12970
mslipper merged 14 commits intodevelopfrom
ss/intent-config-type

Conversation

@bitwiseguy
Copy link
Copy Markdown
Contributor

@bitwiseguy bitwiseguy commented Nov 19, 2024

Creates the concept of intent-config-type. Based on the selection, some fields are auto-populated and others are expected to be populated by the user prior to running op-deployer apply. Valid options are:

  • test used by current test suite, deterministically generates addresses/keys used in testing
  • standard (default, uses standard values as defined in superchain-registry where possible)
  • custom requires all params to be populated by user, errors during apply if any don't get set
  • strict same as "standard", but sets additional params
  • standard-overrides
  • strict-overrides

Design Details

  • op-deployer init
    • intent.SetInitValues: generates initial intent (with many empty fields)
    • user expected to fill out empty/zero-value fields before running op-deployer apply
  • op-deployer apply
    • pipeline stage: init
      • intent.ValidateIntentConfigType: validates that all fields have been populated, and that values meet expectation (e.g. standard values are used if intent-config-type=standard

Open questions

Which fields should thestandard-overrides/strict-overrides settings allow to be overridden? The way this is currently implemented:

  • standard-overrides:
    • op-deployer init: sets intent.toml standard values
    • op-deployer apply: validates that all values are populated (i.e. non zero value) but doesn't require specific values
  • strict-overrides:
    • op-deployer init: sets intent.toml strict values
    • op-deployer apply: validates that all values are populated (i.e. non zero value) but doesn't require specific values

Meta

Closes https://github.com/ethereum-optimism/platforms-team/issues/305

@bitwiseguy bitwiseguy added the M-do-not-merge Meta: Do not merge label Nov 19, 2024
@bitwiseguy bitwiseguy force-pushed the ss/intent-config-type branch from a7d91eb to 9d0e556 Compare November 22, 2024 18:17
@bitwiseguy bitwiseguy changed the title op-deployer: add config-intent-type op-deployer: add intent-config-type Nov 22, 2024
@bitwiseguy bitwiseguy marked this pull request as ready for review November 22, 2024 21:16
@bitwiseguy bitwiseguy requested review from a team, axelKingsley, mds1 and mslipper and removed request for axelKingsley November 22, 2024 21:16
@bitwiseguy bitwiseguy removed the M-do-not-merge Meta: Do not merge label Nov 22, 2024
Comment thread op-deployer/pkg/deployer/state/intent.go Outdated
Comment thread op-deployer/pkg/deployer/flags.go
Copy link
Copy Markdown
Contributor

@mslipper mslipper left a comment

Choose a reason for hiding this comment

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

A few issues I'd like to address before merge. Happy to pair next week to resolve them.

Comment thread op-deployer/pkg/deployer/state/intent.go Outdated
Comment thread op-deployer/pkg/deployer/state/intent.go Outdated
Comment thread op-deployer/pkg/deployer/state/intent.go Outdated
Comment thread op-deployer/pkg/deployer/state/intent.go Outdated
Comment thread op-deployer/pkg/deployer/state/intent.go Outdated
Comment thread op-deployer/pkg/deployer/state/intent.go Outdated
Comment thread op-deployer/pkg/deployer/state/intent.go Outdated
Comment thread op-deployer/pkg/deployer/state/chain_intent.go Outdated
@mslipper mslipper enabled auto-merge November 27, 2024 16:10
@bitwiseguy bitwiseguy force-pushed the ss/intent-config-type branch from fc440e4 to 3af7850 Compare November 27, 2024 16:11
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 27, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 124 lines in your changes missing coverage. Please review.

Project coverage is 42.81%. Comparing base (9109958) to head (55376b2).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
op-deployer/pkg/deployer/state/intent.go 49.15% 80 Missing and 10 partials ⚠️
op-deployer/pkg/deployer/standard/standard.go 32.00% 17 Missing ⚠️
op-deployer/pkg/deployer/init.go 0.00% 6 Missing ⚠️
op-deployer/pkg/deployer/state/chain_intent.go 82.35% 4 Missing and 2 partials ⚠️
op-deployer/pkg/deployer/apply.go 0.00% 2 Missing and 1 partial ⚠️
op-deployer/pkg/deployer/pipeline/init.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12970      +/-   ##
===========================================
- Coverage    44.46%   42.81%   -1.66%     
===========================================
  Files          798      743      -55     
  Lines        71682    67175    -4507     
===========================================
- Hits         31876    28762    -3114     
+ Misses       37219    35990    -1229     
+ Partials      2587     2423     -164     
Flag Coverage Δ
cannon-go-tests-32 ?
cannon-go-tests-64 ?

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

Files with missing lines Coverage Δ
op-deployer/pkg/deployer/artifacts/locator.go 82.60% <100.00%> (+0.97%) ⬆️
op-deployer/pkg/deployer/flags.go 62.50% <ø> (ø)
op-deployer/pkg/deployer/pipeline/init.go 32.74% <0.00%> (ø)
op-deployer/pkg/deployer/apply.go 49.33% <0.00%> (-0.67%) ⬇️
op-deployer/pkg/deployer/init.go 0.00% <0.00%> (ø)
op-deployer/pkg/deployer/state/chain_intent.go 82.35% <82.35%> (ø)
op-deployer/pkg/deployer/standard/standard.go 49.59% <32.00%> (+0.09%) ⬆️
op-deployer/pkg/deployer/state/intent.go 46.69% <49.15%> (+46.69%) ⬆️

... and 66 files with indirect coverage changes

@mslipper mslipper added this pull request to the merge queue Nov 27, 2024
Merged via the queue into develop with commit c8f4b3a Nov 27, 2024
@mslipper mslipper deleted the ss/intent-config-type branch November 27, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants