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

rename TestEnableDevelopmentHardForkEras to TestEnableAdvertiseDevelopmentProtVer #4043

Closed
Tracked by #4144 ...
nfrisby opened this issue Jun 13, 2022 · 1 comment · Fixed by #4341
Closed
Tracked by #4144 ...

rename TestEnableDevelopmentHardForkEras to TestEnableAdvertiseDevelopmentProtVer #4043

nfrisby opened this issue Jun 13, 2022 · 1 comment · Fixed by #4341
Assignees

Comments

@nfrisby
Copy link
Contributor

nfrisby commented Jun 13, 2022

Currently, setting TestEnableDevelopmentHardForkEras: true in the node's config file sets this flag in the code:

https://github.com/input-output-hk/cardano-node/blob/d0b0fa10ac83dc1d0d40aaf294cfded1b455a0a4/cardano-node/src/Cardano/Node/Types.hs#L176-L191

As that comment says, this flag controls what the node "advertises". We just had some Slack conversations about whether we should set this flag, and there was plenty of confusion about what it actually does.

For example, at the moment (since we're preparing a Babbage-able release candidate), its direct effect looks like this on the master branch:

https://github.com/input-output-hk/cardano-node/blob/d0b0fa10ac83dc1d0d40aaf294cfded1b455a0a4/cardano-node/src/Cardano/Node/Protocol/Cardano.hs#L205-L216

And if you chase through the code a bit, you see that its ultimate effect is the value of shelleyProtocolVersion.

https://github.com/input-output-hk/ouroboros-network/blob/0a3a61ae80244dde943843fd39ef17cd85260980/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/Config.hs#L44-L47

data instance BlockConfig (ShelleyBlock proto era) = ShelleyConfig {
      -- | The highest protocol version this node supports. It will be stored
      -- the headers of produced blocks.
      shelleyProtocolVersion  :: !SL.ProtVer

So that's what "advertise" means, concretely, for the Shelley ledgers.

This Issue is to rename the flag to something more indicative of its purpose. I think that's low-hanging fruit to make such confusion less likely in the future. (Happy to see bikeshedding here, my TestEnableAdvertiseDevelopmentProtVer is just a conversation starter.)

@LudvikGalois LudvikGalois self-assigned this Aug 10, 2022
@CarlosLopezDeLara CarlosLopezDeLara moved this to 📋 Backlog in Node CLI/API 2022 Aug 11, 2022
@CarlosLopezDeLara CarlosLopezDeLara moved this from 📋 Backlog to 🔖 Ready for the next sprint in Node CLI/API 2022 Aug 11, 2022
@LudvikGalois
Copy link
Contributor

I'll make a PR which renames this, but in current master, this field is unused

LudvikGalois pushed a commit that referenced this issue Aug 16, 2022
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
LudvikGalois pushed a commit that referenced this issue Aug 23, 2022
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
@CarlosLopezDeLara CarlosLopezDeLara moved this from 🔖 Sprint backlog to 👀Code review in Node CLI/API 2022 Aug 23, 2022
LudvikGalois pushed a commit that referenced this issue Aug 25, 2022
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
LudvikGalois pushed a commit that referenced this issue Oct 24, 2022
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
LudvikGalois pushed a commit that referenced this issue Oct 31, 2022
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
LudvikGalois pushed a commit that referenced this issue Oct 31, 2022
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
@CarlosLopezDeLara CarlosLopezDeLara mentioned this issue Nov 3, 2022
4 tasks
newhoggy pushed a commit that referenced this issue Feb 27, 2023
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
@newhoggy newhoggy self-assigned this Feb 28, 2023
newhoggy pushed a commit that referenced this issue Mar 22, 2023
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
newhoggy pushed a commit that referenced this issue Apr 2, 2023
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
newhoggy pushed a commit that referenced this issue Apr 2, 2023
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
newhoggy pushed a commit that referenced this issue Apr 2, 2023
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
newhoggy pushed a commit that referenced this issue Apr 6, 2023
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
newhoggy pushed a commit that referenced this issue Apr 10, 2023
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
newhoggy pushed a commit that referenced this issue Apr 11, 2023
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants