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

Query for stakepools in run tx build #3152

Merged
merged 3 commits into from
Sep 6, 2021
Merged

Query for stakepools in run tx build #3152

merged 3 commits into from
Sep 6, 2021

Conversation

newhoggy
Copy link
Contributor

@newhoggy newhoggy commented Sep 1, 2021

No description provided.

DavidEichmann
DavidEichmann reviewed Sep 2, 2021
View reviewed changes
cardano-cli/src/Cardano/CLI/Shelley/Run/Transaction.hs
pparams <- executeQuery era cModeParams localConnInfo pParamsQuery
(eraHistory, systemStart) <- firstExceptT ShelleyTxCmdAcquireFailure
$ newExceptT $ queryEraHistoryAndSystemStart localNodeConnInfo Nothing
(utxo, pparams, eraHistory, systemStart, stakePools) <-
Copy link
Contributor

Choose a reason for hiding this comment

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

@newhoggy I originally suggested changing from ExceptT _ IO _ to IO (Either _ _) in the return type of executeLocalStateQueryExpr, was that meant to avoid some of this pain here?

I'll leave it up to you, but you could also just replace joinEitherM ShelleyTxCmdAcquireFailure id with fmap (join . first ShelleyTxCmdAcquireFailure) then you can avoid having to create joinEither/joinEitherM helper functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets move those helper functions to the cli or refactor as David suggested above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored as suggested.

Jimbo4350
Jimbo4350 previously requested changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Minor changes

@newhoggy newhoggy force-pushed the query-for-stakepools-in-runTxBuild branch from ad0f5a6 to fa3fac9 Compare September 5, 2021 03:59
@newhoggy newhoggy force-pushed the query-for-stakepools-in-runTxBuild branch from fa3fac9 to 03339e7 Compare September 5, 2021 04:20
@newhoggy newhoggy dismissed Jimbo4350’s stale review September 5, 2021 04:21

Addressed comments

@newhoggy newhoggy force-pushed the query-for-stakepools-in-runTxBuild branch from 03339e7 to a4efda1 Compare September 5, 2021 04:35
@newhoggy
Copy link
Contributor Author

newhoggy commented Sep 5, 2021

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 5, 2021

👎 Rejected by too few approved reviews

Jimbo4350
Jimbo4350 previously requested changes Sep 6, 2021
View reviewed changes
cardano-cli/src/Cardano/CLI/Shelley/Run/Query.hs
@@ -794,7 +838,7 @@ executeQuery era cModeP localNodeConnInfo q = do
execQuery :: IO (Either AcquireFailure (Either EraMismatch result))
execQuery = queryNodeLocalState localNodeConnInfo Nothing q

getSbe :: CardanoEraStyle era -> ExceptT ShelleyQueryCmdError IO (ShelleyBasedEra era)
getSbe :: Monad m => CardanoEraStyle era -> ExceptT ShelleyQueryCmdError m (ShelleyBasedEra era)
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 leave this as concrete IO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to back out that change.

There is one place where m /= IO, which is here: https://github.com/input-output-hk/cardano-node/blob/7438366f3c2dbb845bc7a73aee4288be23448b29/cardano-cli/src/Cardano/CLI/Shelley/Run/Query.hs#L691

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the compile error:

src/Cardano/CLI/Shelley/Run/Query.hs:691:18: error:
    • Couldn't match type 'IO'
                     with 'LocalStateQueryExpr
                             (BlockInMode mode) ChainPoint (QueryInMode mode) () IO'
      Expected type: ExceptT
                       ShelleyQueryCmdError
                       (LocalStateQueryExpr
                          (BlockInMode mode) ChainPoint (QueryInMode mode) () IO)
                       (ShelleyBasedEra era)
        Actual type: ExceptT
                       ShelleyQueryCmdError IO (ShelleyBasedEra era)
    • In a stmt of a 'do' block: sbe <- getSbe $ cardanoEraStyle era
      In the expression:
        do sbe <- getSbe $ cardanoEraStyle era
           firstExceptT ShelleyQueryCmdEraMismatch . ExceptT
             $ queryExpr . QueryInEra eInMode . QueryInShelleyBasedEra sbe
                 $ QueryStakePools
      In a case alternative:
          Just eInMode
            -> do sbe <- getSbe $ cardanoEraStyle era
                  firstExceptT ShelleyQueryCmdEraMismatch . ExceptT
                    $ queryExpr . QueryInEra eInMode . QueryInShelleyBasedEra sbe
                        $ QueryStakePools
    • Relevant bindings include
        eInMode :: EraInMode era mode
          (bound at src/Cardano/CLI/Shelley/Run/Query.hs:690:14)
        cMode :: ConsensusMode mode
          (bound at src/Cardano/CLI/Shelley/Run/Query.hs:687:11)
        localNodeConnInfo :: LocalNodeConnectInfo mode
          (bound at src/Cardano/CLI/Shelley/Run/Query.hs:678:7)
        cModeParams :: ConsensusModeParams mode
          (bound at src/Cardano/CLI/Shelley/Run/Query.hs:674:44)
    |
691 |           sbe <- getSbe $ cardanoEraStyle era
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@newhoggy newhoggy force-pushed the query-for-stakepools-in-runTxBuild branch from a4efda1 to e04bb8c Compare September 6, 2021 12:37
@newhoggy newhoggy dismissed Jimbo4350’s stale review September 6, 2021 12:38

Comments addressed.

@newhoggy
Copy link
Contributor Author

newhoggy commented Sep 6, 2021

bors merge

iohk-bors bot added a commit that referenced this pull request Sep 6, 2021
@iohk-bors @newhoggy
Merge #3152
d407d2c 
3152: Query for stakepools in run tx build r=newhoggy a=newhoggy



Co-authored-by: John Ky <john.ky@iohk.io>
@newhoggy newhoggy force-pushed the query-for-stakepools-in-runTxBuild branch from e04bb8c to 7438366 Compare September 6, 2021 13:04
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 6, 2021

Canceled.

@newhoggy
Copy link
Contributor Author

newhoggy commented Sep 6, 2021

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 6, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit 3ec635b into master Sep 6, 2021
@iohk-bors iohk-bors bot deleted the query-for-stakepools-in-runTxBuild branch September 6, 2021 14:37
newhoggy added a commit that referenced this pull request Sep 7, 2021
@newhoggy
Revert "Merge #3152"
70ad753 
This reverts commit 3ec635b, reversing
changes made to bc729af.
@newhoggy newhoggy mentioned this pull request Sep 7, 2021
Merged
iohk-bors bot added a commit that referenced this pull request Sep 7, 2021
@iohk-bors @newhoggy
Merge #3174
d945d8d 
3174: Revert "Merge #3152" r=Jimbo4350 a=newhoggy

This reverts commit 3ec635b, reversing
changes made to bc729af.

This is reverts changes that pass in the stake pool id.

Co-authored-by: John Ky <john.ky@iohk.io>
newhoggy added a commit that referenced this pull request Sep 7, 2021
@newhoggy
Query for stakepools in run tx build (attempt 2). See #3152 for first…
1e1e3b3 
… attempt.
iohk-bors bot added a commit that referenced this pull request Sep 8, 2021
@iohk-bors @newhoggy
Merge #3176
50699d1 
3176: Query for stakepools in run tx build r=newhoggy a=newhoggy

* Query for stake pool ids to pass into `makeTransactionBodyAutoBalance`.
* Merge multiple separate queries into a single connection.
* New `query stake-pools` command.

This is a second attempt for #3152


Co-authored-by: John Ky <john.ky@iohk.io>
newhoggy added a commit to IntersectMBO/cardano-api that referenced this pull request May 23, 2023
@iohk-bors @newhoggy
Merge #3176
f1a2b5b 
3176: Query for stakepools in run tx build r=newhoggy a=newhoggy

* Query for stake pool ids to pass into `makeTransactionBodyAutoBalance`.
* Merge multiple separate queries into a single connection.
* New `query stake-pools` command.

This is a second attempt for IntersectMBO/cardano-node#3152


Co-authored-by: John Ky <john.ky@iohk.io>
newhoggy added a commit to IntersectMBO/cardano-cli that referenced this pull request May 24, 2023
@iohk-bors @newhoggy
Merge #3176
8d56851 
3176: Query for stakepools in run tx build r=newhoggy a=newhoggy

* Query for stake pool ids to pass into `makeTransactionBodyAutoBalance`.
* Merge multiple separate queries into a single connection.
* New `query stake-pools` command.

This is a second attempt for IntersectMBO/cardano-node#3152


Co-authored-by: John Ky <john.ky@iohk.io>
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

@dcoutts dcoutts Awaiting requested review from dcoutts

@erikd erikd Awaiting requested review from erikd

@JaredCorduan JaredCorduan Awaiting requested review from JaredCorduan

@nc6 nc6 Awaiting requested review from nc6

@DavidEichmann DavidEichmann Awaiting requested review from DavidEichmann

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

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