-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
revoke deployer key from timelock admin #15710
Conversation
deployment/common/changeset/transfer_to_mcms_with_timelock_test.go
Outdated
Show resolved
Hide resolved
Flakeguard SummaryRan new or updated tests between View Flaky Detector Details | Compare Changes Found Flaky Tests ❌
ArtifactsFor detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json. |
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
Flakeguard SummaryRan new or updated tests between View Flaky Detector Details | Compare Changes Found Flaky Tests ❌
ArtifactsFor detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json. |
deployment/common/changeset/transfer_to_mcms_with_timelock_test.go
Outdated
Show resolved
Hide resolved
deployment/common/changeset/transfer_to_mcms_with_timelock_test.go
Outdated
Show resolved
Hide resolved
f36a558
if err != nil { | ||
return deployment.ChangesetOutput{}, err | ||
} | ||
tl := contracts[cfg.ChainSel].Timelock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth changing to the tl, ok := contracts[cfg.ChainSel]
otherwise if nil
it can throw a nil pointer panic
if err != nil { | ||
return deployment.ChangesetOutput{}, fmt.Errorf("failed to revoke deployer key: %w", err) | ||
} | ||
if _, err := deployment.ConfirmIfNoError(e.Chains[cfg.ChainSel], tx, err); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to allow this to happen also via an mcms proposal? if so maybe we'll need to add conditional logic depending on the config similar to this one:
if cfg.McmsConfig != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not an MCMS proposal, this is done from an EOA since the deployer key has admin role and it renounces its own role
KS-655
Requires
Supports