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

refactor: remove AddressAdmin config #1220

Merged
merged 6 commits into from
Sep 26, 2024
Merged

refactor: remove AddressAdmin config #1220

merged 6 commits into from
Sep 26, 2024

Conversation

vcastellm
Copy link

@vcastellm vcastellm commented Sep 25, 2024

This PR removes the unused config file parameter AdminAddress as it's not needed. No event is being generated with this address so there's no need to read anything.

The admin address is not a contract address but an EOA that can be mutable.

This is aligned in the config in cdk-erigon with zkevm-node.

@Stefan-Ethernal
Copy link
Collaborator

@vcastellm remember to remove providing the zkevm.address-admin in the Kurtosis CDK, as the Kurtosis deployment fails with the following error (https://github.com/0xPolygonHermez/cdk-erigon/actions/runs/11027906212/job/30627088896?pr=1220):

failed setting config flags from yaml/toml file: failed setting zkevm.address-admin flag with value=0xE34aaF64b29273B7D567FCFc40544c014EEe9970 error=no such flag -zkevm.address-admin

@hexoscott
Copy link
Collaborator

As we're removing a flag that will likely be in use in every node already running cdk-erigon it would be good to add an entry into the DeprecatedFlags map in flags_zkevm.go to notify users that this flag is no longer in use.

@vcastellm
Copy link
Author

As we're removing a flag that will likely be in use in every node already running cdk-erigon it would be good to add an entry into the DeprecatedFlags map in flags_zkevm.go to notify users that this flag is no longer in use.

Let's deprecate it

@vcastellm
Copy link
Author

Applied feedback @hexoscott @Stefan-Ethernal now we only output a warning log stating that the variable is deprecated.

Copy link

sonarcloud bot commented Sep 25, 2024

@V-Staykov V-Staykov merged commit 6c55925 into zkevm Sep 26, 2024
11 checks passed
@V-Staykov V-Staykov deleted the remove-admin branch September 26, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants