Add possibility to query local node to load protocol parameters - `transaction build` #5054

carbolymer · 2023-04-04T14:58:12Z

This PR adds possibility to query local node for protocol parameters for transaction build command. This is a first part of implementation for #5052.

cardano-cli/src/Cardano/CLI/Shelley/Parsers.hs

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

newhoggy · 2023-04-07T15:52:12Z

Might be good to demonstrate the change does what is intended somehow. For example in tests, or at least running the command manually and verifying results and pasting them in the PR.

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

Jimbo4350 · 2023-04-14T17:33:45Z

cardano-cli/src/Cardano/CLI/Shelley/Commands.hs

@@ -469,6 +470,11 @@ renderGenesisCmd cmd =
 -- Shelley CLI flag/option data types
 --

+data ProtocolParamsSource
+  = ProtocolParamsFromFile !ProtocolParamsFile
+  | ProtocolParamsQueryNode !NetworkId !AnyConsensusModeParams


The issue with this type is, in the situation where we want to query the node for the protocol parameters, we already have NetworkId and AnyConsensusModeParams values available to us.

Jimbo4350 · 2023-04-14T17:36:55Z

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

@@ -323,14 +323,14 @@ runTxBuildCmd
  -> TxMetadataJsonSchema
  -> [ScriptFile]
  -> [MetadataFile]
-  -> Maybe ProtocolParamsFile
+  -> Maybe ProtocolParamsSource


So I think what you want to do here is change Maybe ProtocolParamsFile to ProtocolParamsSource. In the case of Maybe ProtocolParamsFile the Nothing case corresponded to querying the node for the ProtocolParameters value.

Jimbo4350 · 2023-04-14T18:08:35Z

cardano-cli/src/Cardano/CLI/Shelley/Commands.hs

      TxInCount
      TxOutCount
      TxShelleyWitnessCount
      TxByronWitnessCount
  | TxCalculateMinRequiredUTxO
      AnyCardanoEra
-      ProtocolParamsFile
+      ProtocolParamsSource


The problem here is TxCalculateMinRequiredUTxO is an offline cli command i.e does not necessitate having a running node in the background.

Jimbo4350 · 2023-04-14T18:08:54Z

cardano-cli/src/Cardano/CLI/Shelley/Commands.hs

@@ -238,14 +239,14 @@ data TransactionCmd
  | TxCalculateMinFee
      TxBodyFile
      (Maybe NetworkId)
-      ProtocolParamsFile
+      ProtocolParamsSource


Likewise here, this is an offline command.

You can't use ProtocolParamsSource here because it's an offline command i.e you do not have to run a node to use it.

Jimbo4350 · 2023-04-14T18:09:19Z

cardano-cli/src/Cardano/CLI/Shelley/Commands.hs

@@ -185,7 +186,7 @@ data TransactionCmd
      [ScriptFile]
      -- ^ Auxiliary scripts
      [MetadataFile]
-      (Maybe ProtocolParamsFile)
+      (Maybe ProtocolParamsSource)


build-raw is also an offline cli command

You can't use ProtocolParamsSource here because it's an offline command i.e you do not have to run a node to use it.

Jimbo4350 · 2023-04-14T18:15:09Z

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

@@ -361,8 +361,8 @@ runTxBuildCmd
  scripts <- firstExceptT ShelleyTxCmdScriptFileError $
                     mapM (readFileScriptInAnyLang . unScriptFile) scriptFiles
  txAuxScripts <- hoistEither $ first ShelleyTxCmdAuxScriptsValidationError $ validateTxAuxScripts cEra scripts
-  mpparams <- forM mProtocolParamsFile $ \ppf ->
-    firstExceptT ShelleyTxCmdProtocolParamsError (readProtocolParameters ppf)
+  mpparams <- forM mProtocolParamsSource $ \pps ->


This has already been deprecated and we already query the node for the ProtocolParameters: https://github.com/input-output-hk/cardano-node/blob/master/cardano-cli/src/Cardano/CLI/Shelley/Run/Transaction.hs#L651

Good point. mpparams isn't used anywhere further.

github-actions · 2023-06-02T02:02:50Z

This PR is stale because it has been open 45 days with no activity.

Jimbo4350 · 2023-06-06T18:32:51Z

cardano-cli/src/Cardano/CLI/Shelley/Commands.hs

@@ -238,14 +239,14 @@ data TransactionCmd
  | TxCalculateMinFee
      TxBodyFile
      (Maybe NetworkId)
-      ProtocolParamsFile
+      ProtocolParamsSource


You can't use ProtocolParamsSource here because it's an offline command i.e you do not have to run a node to use it.

Jimbo4350 · 2023-06-06T18:33:03Z

cardano-cli/src/Cardano/CLI/Shelley/Commands.hs

@@ -185,7 +186,7 @@ data TransactionCmd
      [ScriptFile]
      -- ^ Auxiliary scripts
      [MetadataFile]
-      (Maybe ProtocolParamsFile)
+      (Maybe ProtocolParamsSource)


You can't use ProtocolParamsSource here because it's an offline command i.e you do not have to run a node to use it.

carbolymer linked an issue Apr 7, 2023 that may be closed by this pull request

readProtocolParametersSourceSpec should be able to retrieve the ProtocolParameters by querying local node #5052

Open

1 task

carbolymer force-pushed the mgalazyn/feature/read-protocol-params-from-node branch 3 times, most recently from 62079cf to 3b33373 Compare April 7, 2023 15:18

carbolymer changed the title ~~#5052 Add possibility to query local node to load protocol parameters~~ Add possibility to query local node to load protocol parameters - transaction build Apr 7, 2023

carbolymer marked this pull request as ready for review April 7, 2023 15:21

carbolymer requested review from dcoutts, Jimbo4350, erikd, newhoggy, JaredCorduan and CarlosLopezDeLara as code owners April 7, 2023 15:21

newhoggy reviewed Apr 7, 2023

View reviewed changes

cardano-cli/src/Cardano/CLI/Shelley/Parsers.hs Outdated Show resolved Hide resolved

newhoggy reviewed Apr 7, 2023

View reviewed changes

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

newhoggy reviewed Apr 7, 2023

View reviewed changes

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

carbolymer marked this pull request as draft April 12, 2023 17:24

carbolymer mentioned this pull request Apr 14, 2023

readProtocolParametersSourceSpec should be able to retrieve the ProtocolParameters by querying local node #5052

Open

1 task

carbolymer force-pushed the mgalazyn/feature/read-protocol-params-from-node branch from 3b33373 to 2713502 Compare April 14, 2023 15:51

#5052 Add parser for node params flag

6044cc2

carbolymer force-pushed the mgalazyn/feature/read-protocol-params-from-node branch from 2713502 to 6044cc2 Compare April 14, 2023 15:52

Jimbo4350 reviewed Apr 14, 2023

View reviewed changes

github-actions bot added the Stale label Jun 2, 2023

Jimbo4350 requested changes Jun 6, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add possibility to query local node to load protocol parameters - `transaction build` #5054

Add possibility to query local node to load protocol parameters - `transaction build` #5054

carbolymer commented Apr 4, 2023 •

edited

Loading

newhoggy commented Apr 7, 2023

Jimbo4350 Apr 14, 2023 •

edited

Loading

Jimbo4350 Apr 14, 2023

Jimbo4350 Apr 14, 2023

Jimbo4350 Apr 14, 2023

Jimbo4350 Jun 6, 2023

Jimbo4350 Apr 14, 2023

Jimbo4350 Jun 6, 2023

Jimbo4350 Apr 14, 2023

carbolymer Apr 17, 2023

github-actions bot commented Jun 2, 2023

Jimbo4350 Jun 6, 2023

Jimbo4350 Jun 6, 2023

Add possibility to query local node to load protocol parameters - transaction build #5054

Are you sure you want to change the base?

Add possibility to query local node to load protocol parameters - transaction build #5054

Conversation

carbolymer commented Apr 4, 2023 • edited Loading

newhoggy commented Apr 7, 2023

Jimbo4350 Apr 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add possibility to query local node to load protocol parameters - `transaction build` #5054

Add possibility to query local node to load protocol parameters - `transaction build` #5054

carbolymer commented Apr 4, 2023 •

edited

Loading

Jimbo4350 Apr 14, 2023 •

edited

Loading