Integrate latest ledger, consensus and api for 8.6.0 #385

teodanciu · 2023-10-18T15:28:00Z

Changelog

- description: |
    Updated cardano-ledger, ouroboros-consensus and cardano-api packages 
    Replaced  queryCommitteState with new queryCommitteeMembersState
    Added anchor to committee cold key resignation certificate command

# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  - breaking       # the API has changed in a breaking way
  # - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - improvement    # QoL changes e.g. refactoring
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

Additional context for the PR goes here.

If the PR fixes a particular issue please provide a
link
to the issue.

Checklist

Commit sequence broadly makes sense and commits have useful messages
The change log section in the PR description has been filled in
New tests are added if needed and existing tests are updated. These may include:
- golden tests
- property tests
- round trip tests
- integration tests
  See Running tests for more details
The version bounds in .cabal files are updated
CI passes. See note on CI. The following CI checks are required:
- Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
- Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
- Code builds on Linux, MacOS and Windows for ghc-8.10.7 and ghc-9.2.7
Self-reviewed the diff

s

cardano-cli/src/Cardano/CLI/EraBased/Run/Query.hs

teodanciu · 2023-10-19T13:46:09Z

Force-pushed to remove script added by mistake.

teodanciu · 2023-10-19T17:54:25Z

Force pushed to integrate with cardano-api-27.0

cardano-cli/src/Cardano/CLI/EraBased/Options/Query.hs

teodanciu · 2023-10-24T13:41:04Z

Two golden tests are failing, because of the new options in the query - trying to fix them

cardano-cli/src/Cardano/CLI/EraBased/Options/Query.hs

Jimbo4350

This is looking good, a couple minor comments

Jimbo4350 · 2023-10-25T14:18:08Z

cardano-cli/src/Cardano/CLI/EraBased/Commands/Governance/Committee.hs

@@ -59,6 +61,7 @@ data GovernanceCommitteeCreateColdKeyResignationCertificateCmdArgs era =
  GovernanceCommitteeCreateColdKeyResignationCertificateCmdArgs
    { eon               :: !(ConwayEraOnwards era)
    , vkeyColdKeySource :: !(VerificationKeyOrHashOrFile CommitteeColdKey)
+    , anchor            :: !(Maybe (Ledger.Anchor (Ledger.EraCrypto (ShelleyLedgerEra era))))


Have a look at how the governance action parsers handle anchors. E.g

data GovernanceActionCreateNoConfidenceCmdArgs era = GovernanceActionCreateNoConfidenceCmdArgs { eon :: !(ConwayEraOnwards era) , networkId :: !Ledger.Network , deposit :: !Lovelace , returnStakeAddress :: !AnyStakeIdentifier , proposalUrl :: !ProposalUrl , proposalHashSource :: !ProposalHashSource , governanceActionId :: !TxId , governanceActionIndex :: !Word32 , outFile :: !(File () Out) } deriving Show

In particular the proposalUrl and proposalHashSource fields.

@Jimbo4350 So you prefer to flatten these two fields , url and hash.
But what to do about ProposalUrl and ProposalHashSource which are not accurately describing this Anchor, no - since it's not related to proposal, but to resignation.. Create new types for them?

So you can construct an Anchor as follows: https://github.com/input-output-hk/cardano-cli/blob/e86aa60f4481dcdc035d2173d61090bd284ad089/cardano-cli/src/Cardano/CLI/EraBased/Run/Governance/Actions.hs#L98

Hm, I don't understand exactly what the suggestion is (probably because I'm not familiar with the codebase).
Would you like me to represent Anchor in GovernanceCommitteeCreateColdKeyResignationCertificateCmdArgs as two different fields, similar to the example you gave above, so as :

, resignationlUrl :: !ProposalUrl , resignationHashSource :: !ProposalHashSource

instead of the field that I have added:

, anchor :: !(Maybe (Ledger.Anchor (Ledger.EraCrypto (ShelleyLedgerEra era))))

?

Did i understand correctly?

~~Yes exactly 👍~~ I see what you're saying now.

Both of them will have to be Maybe I suppose, since the field is optional. I hope it won't be too messy, if say one is there and the other one not 🤔

cardano-cli/src/Cardano/CLI/EraBased/Options/Query.hs

cardano-cli/src/Cardano/CLI/Json/Friendly.hs

cardano-cli/src/Cardano/CLI/Types/Errors/QueryCmdError.hs

cardano-cli/src/Cardano/CLI/EraBased/Run/Transaction.hs

newhoggy · 2023-10-27T17:26:53Z

cardano-cli/src/Cardano/CLI/EraBased/Run/Transaction.hs

@@ -532,7 +530,6 @@ runTxBuild
    <- hoistEither $ first TxCmdReturnCollateralValidationError $ validateTxReturnCollateral era mReturnCollateral
  dFee <- hoistEither $ first TxCmdTxFeeValidationError $ validateTxFee era dummyFee
  validatedLowerBound <- hoistEither (first TxCmdTxValidityLowerBoundValidationError (validateTxValidityLowerBound era mLowerBound))
-  validatedUpperBound <- hoistEither (first TxCmdTxValidityUpperBoundValidationError (validateTxValidityUpperBound era mUpperBound))


We no longer do validation here because by virtue of using era-sensitive types in the era-based CLI parser the values are already valid.

and adapt to new filter options

Jimbo4350

LGTM! Just get @CarlosLopezDeLara to sign off on the new parser names.

teodanciu force-pushed the td/ledger-consensus-and-api-for-8.6 branch 4 times, most recently from 24b5b44 to 53a9d67 Compare October 19, 2023 11:54

teodanciu changed the title ~~Integrate latest ledger, consensus and api for 8.6.0-pre - SRP~~ Integrate latest ledger, consensus and api for 8.6.0 (based or cardano-api-8.26.0) - SRP Oct 19, 2023

newhoggy reviewed Oct 19, 2023

View reviewed changes

s Outdated Show resolved Hide resolved

newhoggy reviewed Oct 19, 2023

View reviewed changes

cardano-cli/src/Cardano/CLI/EraBased/Run/Query.hs Outdated Show resolved Hide resolved

teodanciu force-pushed the td/ledger-consensus-and-api-for-8.6 branch from 53a9d67 to eaa2c1f Compare October 19, 2023 13:45

teodanciu force-pushed the td/ledger-consensus-and-api-for-8.6 branch from eaa2c1f to 71a1855 Compare October 19, 2023 17:53

teodanciu changed the title ~~Integrate latest ledger, consensus and api for 8.6.0 (based or cardano-api-8.26.0) - SRP~~ Integrate latest ledger, consensus and api for 8.6.0 (based or cardano-api-8.27.0) - SRP Oct 19, 2023

teodanciu force-pushed the td/ledger-consensus-and-api-for-8.6 branch 3 times, most recently from 5749dfd to f95beaa Compare October 23, 2023 12:50

teodanciu changed the title ~~Integrate latest ledger, consensus and api for 8.6.0 (based or cardano-api-8.27.0) - SRP~~ Integrate latest ledger, consensus and api for 8.6.0 (based or cardano-api-8.28.0) - SRP Oct 23, 2023

teodanciu force-pushed the td/ledger-consensus-and-api-for-8.6 branch 3 times, most recently from f3f5745 to 7ac12a9 Compare October 24, 2023 12:54

teodanciu commented Oct 24, 2023

View reviewed changes

cardano-cli/src/Cardano/CLI/EraBased/Options/Query.hs Outdated Show resolved Hide resolved

teodanciu force-pushed the td/ledger-consensus-and-api-for-8.6 branch from 7ac12a9 to 408df38 Compare October 24, 2023 15:09

teodanciu commented Oct 24, 2023

View reviewed changes

cardano-cli/src/Cardano/CLI/EraBased/Options/Query.hs Outdated Show resolved Hide resolved

teodanciu force-pushed the td/ledger-consensus-and-api-for-8.6 branch from 408df38 to 0f3f668 Compare October 24, 2023 16:25

Jimbo4350 reviewed Oct 25, 2023

View reviewed changes

teodanciu force-pushed the td/ledger-consensus-and-api-for-8.6 branch 5 times, most recently from cd50a2b to 2f756fd Compare October 27, 2023 12:03

newhoggy reviewed Oct 27, 2023

View reviewed changes

cardano-cli/src/Cardano/CLI/EraBased/Run/Transaction.hs Outdated Show resolved Hide resolved

newhoggy reviewed Oct 27, 2023

View reviewed changes

teodanciu marked this pull request as ready for review October 27, 2023 17:27

teodanciu requested review from a team, carbolymer, kevinhammond and disassembler as code owners October 27, 2023 17:27

teodanciu and others added 9 commits October 27, 2023 18:30

Update ledger, consensus and api versions for 8.6

a15d1cf

Adapt to new anchor parameter in ConwayResignCommitteeColdKey

97bbd95

Adapt to ProtocolParametersUpdate changes in cardano-api

0672d09

Adjust to txValidityRange split into lower and upper bound

87e92b1

Add getCommitteeCold\HotCredentialFromVerKeyHashOrFile to Read

022b657

Replace queryCommitteState with new queryCommitteeMembersState

4303fbf

and adapt to new filter options

Parse MemberStatus filter as a flag rather than string argument

5b2d25d

Update golden test files to reflect changes in commands

486d011

Fix CLI support for TxValidityUpperBound

eda3544

teodanciu force-pushed the td/ledger-consensus-and-api-for-8.6 branch from 01e98c0 to eda3544 Compare October 27, 2023 17:30

Write the output of CommitteeMembersState query as json.

10440c0

Jimbo4350 approved these changes Oct 27, 2023

View reviewed changes

Improve command option names based of committee resignation anchor

f1928f9

teodanciu requested a review from CarlosLopezDeLara October 27, 2023 19:25

CarlosLopezDeLara approved these changes Oct 27, 2023

View reviewed changes

CarlosLopezDeLara added this pull request to the merge queue Oct 27, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 27, 2023

CarlosLopezDeLara added this pull request to the merge queue Oct 27, 2023

Merged via the queue into main with commit 5c0897b Oct 27, 2023
19 checks passed

CarlosLopezDeLara deleted the td/ledger-consensus-and-api-for-8.6 branch October 27, 2023 20:02

smelc mentioned this pull request Oct 30, 2023

adjust cc-resignation certificate to also include an optional anchor #384

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate latest ledger, consensus and api for 8.6.0 #385

Integrate latest ledger, consensus and api for 8.6.0 #385

teodanciu commented Oct 18, 2023 •

edited

Loading

teodanciu commented Oct 19, 2023

teodanciu commented Oct 19, 2023

teodanciu commented Oct 24, 2023

Jimbo4350 left a comment

Jimbo4350 Oct 25, 2023 •

edited

Loading

teodanciu Oct 27, 2023 •

edited

Loading

Jimbo4350 Oct 27, 2023

teodanciu Oct 27, 2023

Jimbo4350 Oct 27, 2023 •

edited

Loading

teodanciu Oct 27, 2023 •

edited

Loading

newhoggy Oct 27, 2023

Jimbo4350 left a comment

Integrate latest ledger, consensus and api for 8.6.0 #385

Integrate latest ledger, consensus and api for 8.6.0 #385

Conversation

teodanciu commented Oct 18, 2023 • edited Loading

Changelog

Context

Checklist

teodanciu commented Oct 19, 2023

teodanciu commented Oct 19, 2023

teodanciu commented Oct 24, 2023

Jimbo4350 left a comment

Choose a reason for hiding this comment

Jimbo4350 Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

teodanciu Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

Jimbo4350 Oct 27, 2023

Choose a reason for hiding this comment

teodanciu Oct 27, 2023

Choose a reason for hiding this comment

Jimbo4350 Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

teodanciu Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

newhoggy Oct 27, 2023

Choose a reason for hiding this comment

Jimbo4350 left a comment

Choose a reason for hiding this comment

teodanciu commented Oct 18, 2023 •

edited

Loading

Jimbo4350 Oct 25, 2023 •

edited

Loading

teodanciu Oct 27, 2023 •

edited

Loading

Jimbo4350 Oct 27, 2023 •

edited

Loading

teodanciu Oct 27, 2023 •

edited

Loading