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

Jump to bottom

Merge script address build command into address build command #2486

Merged
merged 1 commit into from
Mar 31, 2021
Merged

Merge script address build command into address build command #2486

merged 1 commit into from
Mar 31, 2021

Conversation

newhoggy
Copy link
Contributor

@newhoggy newhoggy commented Mar 15, 2021 •
edited
Loading

Addresses this ticket: #2333

@newhoggy newhoggy force-pushed the CAD-2658-orthogonal-address-build branch 3 times, most recently from e8abc4f to f0d9815 Compare March 15, 2021 04:19
@newhoggy newhoggy changed the title Stuff Merge ccript address build command into build command Mar 15, 2021
@newhoggy newhoggy changed the title Merge ccript address build command into build command Merge script address build command into build command Mar 15, 2021
@newhoggy newhoggy marked this pull request as ready for review March 15, 2021 04:19
@newhoggy newhoggy changed the title Merge script address build command into build command Merge script address build command into address build command Mar 15, 2021
@newhoggy
Copy link
Contributor Author

newhoggy commented Mar 15, 2021 •
edited
Loading

Help text for the address build command:

cabal exec -- cardano-cli address build
Usage: cardano-cli address build (--payment-verification-key STRING |
                                   --payment-verification-key-file FILE |
                                   --script-file FILE)
                                 [--stake-verification-key STRING |
                                   --stake-verification-key-file FILE]
                                 (--mainnet | --testnet-magic NATURAL)
                                 [--out-file FILE]
  Build a Shelley payment address, with optional delegation to a stake address.

Available options:
  --payment-verification-key STRING
                           Payment verification key (Bech32-encoded)
  --payment-verification-key-file FILE
                           Filepath of the payment verification key.
  --script-file FILE       Filepath of the script.
  --stake-verification-key STRING
                           Stake verification key (Bech32 or hex-encoded).
  --stake-verification-key-file FILE
                           Filepath of the staking verification key.
  --mainnet                Use the mainnet magic id.
  --testnet-magic NATURAL  Specify a testnet magic id.
  --out-file FILE          Optional output file. Default is to write to stdout.
  -h,--help                Show this help text

The script-file option is new.

@newhoggy newhoggy force-pushed the CAD-2658-orthogonal-address-build branch 2 times, most recently from 7829e45 to cfad4b2 Compare March 15, 2021 06:09
Jimbo4350
Jimbo4350 reviewed Mar 15, 2021
View reviewed changes
cardano-cli/src/Cardano/CLI/Shelley/Key.hs
@@ -381,6 +383,11 @@ readVerificationKeyOrTextEnvFile asType verKeyOrFile =
VerificationKeyFilePath (VerificationKeyFile fp) ->
readKeyFileTextEnvelope (AsVerificationKey asType) fp

data PaymentSource
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move this to cardano-cli/src/Cardano/CLI/Shelley/Parsers.hs

Copy link
Contributor Author

@newhoggy newhoggy Mar 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately I can't move it to Parsers because AddressCmd definition in Cardano.CLI.Shelley.Commands needs it. Moving the type into Parsers would create a cyclic import dependency between Parsers and Commands.

Jimbo4350
Jimbo4350 approved these changes Mar 15, 2021
View reviewed changes
cardano-cli/src/Cardano/CLI/Shelley/Run/Address.hs Outdated
Just (OutputFile oFp) -> Text.writeFile oFp scriptAddrText
Nothing -> Text.putStr scriptAddrText

runAddressBuildScript scriptFile = runAddressBuild (SourcePaymentScript scriptFile) Nothing
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we also print a deprecation warning here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we have an example of a deprecation warning?

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/input-output-hk/cardano-node/blob/0dac653ca71133abc7a5cc01d31d72aabba3d9d9/cardano-cli/src/Cardano/CLI/Parsers.hs#L75

Copy link
Contributor

@Jimbo4350 Jimbo4350 Mar 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Or something like https://github.com/input-output-hk/cardano-node/blob/94f79cc9d637af569836cd30828681875c2f986a/cardano-api/src/Cardano/Api/Eras.hs#L95

@newhoggy newhoggy force-pushed the CAD-2658-orthogonal-address-build branch 6 times, most recently from 5cd53f8 to 60c6014 Compare March 31, 2021 19:44
@newhoggy newhoggy force-pushed the CAD-2658-orthogonal-address-build branch from 60c6014 to ef804ee Compare March 31, 2021 21:18
@newhoggy
Copy link
Contributor Author

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 31, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit 476db29 into master Mar 31, 2021
@iohk-bors iohk-bors bot deleted the CAD-2658-orthogonal-address-build branch March 31, 2021 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jimbo4350 Jimbo4350 Jimbo4350 approved these changes

@DavidEichmann DavidEichmann Awaiting requested review from DavidEichmann

@dcoutts dcoutts Awaiting requested review from dcoutts

@erikd erikd Awaiting requested review from erikd

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@newhoggy @Jimbo4350