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

Update to cardano-api-8.8.0.0 and cardano-cli #5383

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

newhoggy
Copy link
Contributor

@newhoggy newhoggy commented Jul 3, 2023

Description

Note, cardano-api-8.8.0.0 doesn't exist yet, but will be published soon.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. These may include:
    • golden tests
    • property tests
    • roundtrip tests
    • integration tests
      See Runnings tests for more details
  • Any changes are noted in the CHANGELOG.md for affected package
  • 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

Note on CI

If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.

@newhoggy newhoggy changed the title Newhoggy/upgrade to cardano api 8.8.0.0 Update to cardano-api-8.7.0.0 and cardano-cli Jul 3, 2023
@newhoggy newhoggy changed the title Update to cardano-api-8.7.0.0 and cardano-cli Update to cardano-api-8.8.0.0 and cardano-cli Jul 3, 2023
@disassembler disassembler force-pushed the newhoggy/upgrade-to-cardano-api-8.8.0.0 branch from 5741a2b to 196afbe Compare July 3, 2023 19:42
@newhoggy newhoggy force-pushed the newhoggy/upgrade-to-cardano-api-8.8.0.0 branch 2 times, most recently from 80e446a to 8947513 Compare July 7, 2023 12:07
@disassembler disassembler force-pushed the newhoggy/upgrade-to-cardano-api-8.8.0.0 branch from 02c44d4 to ce2d3be Compare July 8, 2023 03:18
@newhoggy newhoggy force-pushed the newhoggy/upgrade-to-cardano-api-8.8.0.0 branch 3 times, most recently from d9a910c to b325a1d Compare July 10, 2023 15:39
@newhoggy newhoggy marked this pull request as ready for review July 10, 2023 15:39
@newhoggy newhoggy requested review from a team as code owners July 10, 2023 15:39
Copy link
Contributor

@bolt12 bolt12 left a comment

Choose a reason for hiding this comment

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

Regarding my changes LGTM!

cabal.project Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Node/Run.hs Outdated Show resolved Hide resolved
@newhoggy newhoggy force-pushed the newhoggy/upgrade-to-cardano-api-8.8.0.0 branch 2 times, most recently from 721aec6 to 142e404 Compare July 10, 2023 22:51
cardano-node/src/Cardano/Node/Startup.hs Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Node/Types.hs Show resolved Hide resolved
cardano-node/src/Cardano/Node/Parsers.hs Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Node/Parsers.hs Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Node/Tracing/Era/Shelley.hs Outdated Show resolved Hide resolved
scripts/conway/test.constitution Outdated Show resolved Hide resolved
scripts/conway/submit-new-constitution.sh Outdated Show resolved Hide resolved
@carbolymer carbolymer force-pushed the newhoggy/upgrade-to-cardano-api-8.8.0.0 branch from 3f2a77e to cd2c174 Compare July 11, 2023 14:31
@newhoggy newhoggy enabled auto-merge July 12, 2023 01:57
cabal.project Outdated
@@ -108,3 +108,39 @@ package snap-server
-- IMPORTANT
-- Do NOT add more source-repository-package stanzas here unless they are strictly
-- temporary! Please read the section in CONTRIBUTING about updating dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are these things not yet in CHaP?

Is it worth waiting until they are?

Copy link
Contributor

Choose a reason for hiding this comment

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

I should probably not have been looking at each commit individually. However, I do also think that the commit history should be cleaned up so that it looks like those SRPs were never added.

@newhoggy newhoggy force-pushed the newhoggy/upgrade-to-cardano-api-8.8.0.0 branch from cd2c174 to 630dd10 Compare July 12, 2023 07:12
@newhoggy newhoggy requested review from erikd and Jimbo4350 July 12, 2023 07:12
@mgmeier
Copy link
Contributor

mgmeier commented Jul 13, 2023

Currently, Hydra CI reports a compilation error for bech32-1.1.2. As a consequence of that error, the workbench shell can't be entered anymore.
On master the version seems to be resolved to bech32-1.1.3, which provides version bounds for its optparse-applicative dependency avoiding the compilation error.

@newhoggy newhoggy force-pushed the newhoggy/upgrade-to-cardano-api-8.8.0.0 branch from 630dd10 to 08ba49b Compare July 13, 2023 11:15
…ano-cli-8.3.0.0

Set initial block forging

Moved UseLedger type to avoid circular imports

- The moving of UseLedger was needed in order to import `Cardano.Node.Protocol (ProtocolInstantiationError)`
- Added new constructors to Startup Tracer

Update block forging configuration on SIGHUP

Block Forging can be enabled/disabled through SIGHUP signal. Sending
such a signal will trigger the node to read the block forging credential
files. Since the credential files are passed via CLI flags one can not
remove them without restarting the node. For this effect, in order for
one to be able to disable block forging, moving/renaming/deleting the
file at the specified path (for the credential flags) and then sending
the SIGHUP signal is what needs to be done. The code will detect that
the specified files do not exist and it will disable block forging,
while tracing the appropriate log messages.

In case one wants to start a block producing node (i.e. passing the
credentials in the respective flags) but doesn't want it to actually
behave as a BP, there's a new `--start-as-non-producing-node` flag that
will run the node with credentials as a normal node. However on SIGHUP
it will read the credential files and start forging.

Refactor code to accomodate the changes

Added EnabledBlockForging type

Fix Shutdown test for new exit codes

Co-authored-by: Armando Santos <armandoifsantos@gmail.com>
Co-authored-by: Jordan Millar <jordan.millar@iohk.io>
Co-authored-by: Marcin Szamotulski <coot@coot.me>
Co-authored-by: Samuel Leathers <samuel.leathers@iohk.io>
@newhoggy newhoggy force-pushed the newhoggy/upgrade-to-cardano-api-8.8.0.0 branch from 08ba49b to eb66a54 Compare July 14, 2023 12:34
Copy link
Contributor

@mgmeier mgmeier left a comment

Choose a reason for hiding this comment

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

Thanks @newhoggy LGTM!

@newhoggy newhoggy added this pull request to the merge queue Jul 14, 2023
Merged via the queue into master with commit 74467ec Jul 14, 2023
@iohk-bors iohk-bors bot deleted the newhoggy/upgrade-to-cardano-api-8.8.0.0 branch July 14, 2023 13:54
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.

7 participants