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.

Sign up for GitHub

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

Disable development eras configuration flag #4030

Merged
merged 2 commits into from
Jun 25, 2022
Merged

Disable development eras configuration flag #4030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6d33fa3
Disable development eras configuration flag so we can hardfork to
Jimbo4350 Jun 9, 2022
2d0f554
Update alonzo ProtVer supported to 7 0
disassembler Jun 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cardano-api/src/Cardano/Api/Script.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,10 +953,10 @@ hashScript (PlutusScript PlutusScriptV1 (PlutusScriptSerialised script)) =
ScriptHash
. Ledger.hashScript @(ShelleyLedgerEra AlonzoEra)
$ Alonzo.PlutusScript Alonzo.PlutusV1 script
-- TODO: Babbage era PV2 only exists in Babbage era onwards!

hashScript (PlutusScript PlutusScriptV2 (PlutusScriptSerialised script)) =
ScriptHash
. Ledger.hashScript @(ShelleyLedgerEra AlonzoEra)
. Ledger.hashScript @(ShelleyLedgerEra BabbageEra)
$ Alonzo.PlutusScript Alonzo.PlutusV2 script

toShelleyScriptHash :: ScriptHash -> Shelley.ScriptHash StandardCrypto
Expand Down
9 changes: 3 additions & 6 deletions cardano-node/src/Cardano/Node/Protocol/Cardano.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ mkSomeConsensusProtocolCardano NodeByronProtocolConfiguration {
npcAlonzoGenesisFileHash
}
NodeHardForkProtocolConfiguration {
npcTestEnableDevelopmentHardForkEras,
-- npcTestEnableDevelopmentHardForkEras,
-- During testing of the Alonzo era, we conditionally declared that we
-- knew about the Alonzo era. We do so only when a config option for
-- testing development/unstable eras is used. This lets us include
Expand Down Expand Up @@ -198,7 +198,7 @@ mkSomeConsensusProtocolCardano NodeByronProtocolConfiguration {
-- version that this node will declare that it understands, when it
-- is in the Alonzo era. That is, it is the version of protocol
-- /after/ Alonzo, i.e. Babbage.
alonzoProtVer = ProtVer 6 0,
alonzoProtVer = ProtVer 7 0,
Copy link

@rdlrt rdlrt Jun 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

why is alonzo protocol version that node sees changed here (yes, there were 2 forks in alonzo era, but changing them abruptly all of a sudden without mapping it against fork is very hacky)?? This breaks existing explorers on public testnet that do check on protocol major version in block header...(now we have blocks with different protocol_major adopted on testnet without a fork submission - randomly based on node's presumption of protocol version, if this is the aim to go on mainnet too - best to add some CIP/docs beforehand - there is already efforts done by @JaredCorduan to reduce the confusion) , this is a deviation that further confuses what's mentioned in CIP 274 from historical view

Copy link

Choose a reason for hiding this comment

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

Yes, this indeed is creating a mess checking block version for specific era

Copy link
Contributor

Choose a reason for hiding this comment

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

This change looks correct to me. We want we the nodes in the Alonzo era that are prepared for the upgrade to signal that they are ready by putting major protocol number 7 in the block header (since 7 corresponds to the next ledger era, Babbage).

Copy link

@rdlrt rdlrt Jun 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The change looks correct for future (looking at version from babbage back to alonzo), but it seems to be at the expense of current (future past) - using vague version in block header that is kinda unrelated to actual protocol_major version , in case of which all the components using block header representation
of protocol_major now need to rename it to node version.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually how it was designed to work from the very beginning, going back to the Shelley ledger spec. See section 13 of https://hydra.iohk.io/job/Cardano/cardano-ledger/shelleyLedgerSpec/latest/download-by-type/doc-pdf/ledger-spec.

The version listed in the block header indicates the latest version of the protocol that the block producer is prepared to upgrade to.

Copy link

@rdlrt rdlrt Jun 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the link, looks like some of us have been understanding the value of this block header wrong the whole time 😢 Can we add a reference to this (if not a short description) to the CIP 274?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a reference to this (if not a short description) to the CIP 274?

That is a most excellent idea!

alonzoMaxTxCapacityOverrides =
TxLimits.mkOverrides TxLimits.noOverridesMeasure
}
Expand All @@ -207,10 +207,7 @@ mkSomeConsensusProtocolCardano NodeByronProtocolConfiguration {
-- version that this node will declare that it understands, when it
-- is in the Babbage era. Since Babbage is currently the last known
-- protocol version then this is also the Babbage protocol version.
Praos.babbageProtVer =
if npcTestEnableDevelopmentHardForkEras
then ProtVer 7 0 -- Advertise we can support Babbage
else ProtVer 6 0, -- Otherwise we only advertise we know about (the second) Alonzo
Praos.babbageProtVer = ProtVer 7 0,
Praos.babbageMaxTxCapacityOverrides =
TxLimits.mkOverrides TxLimits.noOverridesMeasure
}
Expand Down
1 change: 0 additions & 1 deletion scripts/babbage/mkfiles.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ $SED -i "${ROOT}/configuration.yaml" \
echo "TestMaryHardForkAtEpoch: 0" >> "${ROOT}/configuration.yaml"
echo "TestAlonzoHardForkAtEpoch: 0" >> "${ROOT}/configuration.yaml"
echo "TestBabbageHardForkAtEpoch: 0" >> "${ROOT}/configuration.yaml"
echo "TestEnableDevelopmentHardForkEras: True" >> "${ROOT}/configuration.yaml"
echo "TestEnableDevelopmentNetworkProtocols: True" >> "${ROOT}/configuration.yaml"

# Copy the cost mode
Expand Down