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

Only hashing immutable part of network configuration for dataDir ID #2955

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jangko
Copy link
Contributor

@jangko jangko commented Dec 19, 2024

@mjfh; @agnxsh ; @arnetheduck you can use hidden config --rewrite-datadir-id to reconfigure your existing database if you encounter "Data dir already initialized with other network configuration"

Future hardfork time or in the case of holesky and mainnet, got new depositContractAddress make the network configuration can change when new features added.

Because we need something stable to identify dataDir with network configuration, the new dataDir ID is calculated using only immutable parts of network configuration.

nimbus/config.nim Outdated Show resolved Hide resolved
Comment on lines +175 to +178
if conf.rewriteDatadirId:
writeDataDirId(kvt, calculatedId)
return

Copy link
Contributor

@advaita-saha advaita-saha Dec 19, 2024

Choose a reason for hiding this comment

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

Wouldn't this allow it to write an existing holesky database with sepolia config when the flag is passed? Misconfiguration issue from users POV, but better if prevented

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and how to prevent that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe storing networkId and networkParams separately 🤔, instead of hashing them together

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.

3 participants