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

Expose max collateral input count (protocol parameter) #2818

Merged

Conversation

sevanspowell
Copy link
Contributor

@sevanspowell sevanspowell commented Aug 11, 2021

Overview

Expose the maximum number of collateral inputs in the wallet API.

  • Add maximumCollateralInputCount field to ApiNetworkParameters type.
  • Add maximum_collateral_input_count field to API.
  • Modify Scenario/API/Shelley/Network integration test to test it can retrieve maxCollateralInputs from the alonzo-genesis.yaml.

Issue Number

ADP-1061

@sevanspowell sevanspowell force-pushed the sevanspowell/adp-1061/expose-max-collateral-inputs branch from 9344b73 to 4e4fd4c Compare August 11, 2021 05:30
Base automatically changed from sevanspowell/adp-958/maximum-collateral-inputs to master August 11, 2021 06:27
@sevanspowell sevanspowell force-pushed the sevanspowell/adp-1061/expose-max-collateral-inputs branch from e7fe03e to 54083fd Compare August 11, 2021 06:56
@sevanspowell
Copy link
Contributor Author

bors try

iohk-bors bot added a commit that referenced this pull request Aug 11, 2021
Copy link
Contributor

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

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

Hi @sevanspowell

Looks good so far. I've made some suggestions regarding the names of things. (See comments.)

specifications/api/swagger.yaml Outdated Show resolved Hide resolved
specifications/api/swagger.yaml Outdated Show resolved Hide resolved
lib/core/src/Cardano/Wallet/Api/Types.hs Outdated Show resolved Hide resolved
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 11, 2021

try

Build failed:

@sevanspowell sevanspowell force-pushed the sevanspowell/adp-1061/expose-max-collateral-inputs branch 2 times, most recently from a9a2c95 to eba0790 Compare August 12, 2021 02:53
@sevanspowell
Copy link
Contributor Author

bors try

@sevanspowell sevanspowell force-pushed the sevanspowell/adp-1061/expose-max-collateral-inputs branch from c3acfbc to b972719 Compare August 16, 2021 06:14
@jonathanknowles jonathanknowles changed the title Expose max collateral inputs Expose max collateral input count (protocol parameter) Aug 16, 2021
Copy link
Contributor

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

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

Looks good to me!

I only have one very small suggestion. (See comment!)

lib/core-integration/src/Test/Integration/Framework/DSL.hs Outdated Show resolved Hide resolved
@sevanspowell sevanspowell force-pushed the sevanspowell/adp-1061/expose-max-collateral-inputs branch from 3ab2d79 to 933f2eb Compare August 16, 2021 07:26
@sevanspowell
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Aug 16, 2021
2818: Expose max collateral input count (protocol parameter) r=sevanspowell a=sevanspowell

### Overview
Expose the maximum number of collateral inputs in the wallet API.

- Add `maximumCollateralInputCount` field to `ApiNetworkParameters` type.
- Add `maximum_collateral_input_count` field to API.
- Modify Scenario/API/Shelley/Network integration test to test it can retrieve `maxCollateralInputs` from the `alonzo-genesis.yaml`.

### Issue Number

ADP-1061


Co-authored-by: Samuel Evans-Powell <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 16, 2021

Build failed:

cardano-wallet-core-integration> /build/cardano-wallet/lib/core-integration/src/Test/Integration/Framework/DSL.hs:683:1: error:
--
  | cardano-wallet-core-integration>     The type signature for ‘maximumCollateralInputCount’
  | cardano-wallet-core-integration>       lacks an accompanying binding
  | cardano-wallet-core-integration>       (The type signature must be given where ‘maximumCollateralInputCount’ is declared)
  | cardano-wallet-core-integration>     \|
  | cardano-wallet-core-integration> 683 \| maximumCollateralInputCount :: ApiEra -> Word16
  | cardano-wallet-core-integration>     \| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  | cardano-wallet-core-integration>
  | cardano-wallet-core            > Benchmark running disabled by --no-run-benchmarks flag.

#expected

- Add "maximumCollateralInputs" field to `ApiNetworkParameters` type.
- Add "maximum_collateral_inputs" field to API.
- Modify Scenario/API/Shelley/Network integration test to test it can retrieve
  `maxCollateralInputs` from the `alonzo-genesis.yaml`.
- Make maximumCollateralInputCount expected value depend on era, otherwise
  integration tests only work in Alonzo era, as the maximum collateral input
  count value was only introduced in the Alonzo era.

Co-authored-by: Jonathan Knowles <[email protected]>
@sevanspowell sevanspowell force-pushed the sevanspowell/adp-1061/expose-max-collateral-inputs branch from 933f2eb to 91f8f06 Compare August 16, 2021 07:45
@sevanspowell
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Aug 16, 2021
2818: Expose max collateral input count (protocol parameter) r=sevanspowell a=sevanspowell

### Overview
Expose the maximum number of collateral inputs in the wallet API.

- Add `maximumCollateralInputCount` field to `ApiNetworkParameters` type.
- Add `maximum_collateral_input_count` field to API.
- Modify Scenario/API/Shelley/Network integration test to test it can retrieve `maxCollateralInputs` from the `alonzo-genesis.yaml`.

### Issue Number

ADP-1061


Co-authored-by: Samuel Evans-Powell <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 16, 2021

This PR was included in a batch that successfully built, but then failed to merge into master (it was a non-fast-forward update). It will be automatically retried.

iohk-bors bot added a commit that referenced this pull request Aug 16, 2021
2818: Expose max collateral input count (protocol parameter) r=sevanspowell a=sevanspowell

### Overview
Expose the maximum number of collateral inputs in the wallet API.

- Add `maximumCollateralInputCount` field to `ApiNetworkParameters` type.
- Add `maximum_collateral_input_count` field to API.
- Modify Scenario/API/Shelley/Network integration test to test it can retrieve `maxCollateralInputs` from the `alonzo-genesis.yaml`.

### Issue Number

ADP-1061


Co-authored-by: Samuel Evans-Powell <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 16, 2021

Build failed:

/nix/store/sv61k1l9fapkn1dzlvclqjl5i1sbhfyw-stdenv-darwin/setup: line 90: [: missing `]'

Integration tests passed, but then the above happened 🤨

@sevanspowell
Copy link
Contributor Author

bors retry

iohk-bors bot added a commit that referenced this pull request Aug 17, 2021
2818: Expose max collateral input count (protocol parameter) r=sevanspowell a=sevanspowell

### Overview
Expose the maximum number of collateral inputs in the wallet API.

- Add `maximumCollateralInputCount` field to `ApiNetworkParameters` type.
- Add `maximum_collateral_input_count` field to API.
- Modify Scenario/API/Shelley/Network integration test to test it can retrieve `maxCollateralInputs` from the `alonzo-genesis.yaml`.

### Issue Number

ADP-1061


Co-authored-by: Samuel Evans-Powell <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 17, 2021

Build failed:

  src/Test/Integration/Scenario/CLI/Shelley/Wallets.hs:448:49: 
  1) CLI Specifications, SHELLEY_CLI_WALLETS, WALLETS_GET_01 - Can get a wallet
       Waited longer than 90s to resolve action: "Wallet state = Ready".
[cardano-wallet.network:Warning:50585] [2021-08-17 03:03:37.17 UTC] Connection lost with the node. Network.Socket.recvBuf: resource vanished (Connection reset by peer)
       expected: Ready
        but got: Syncing (Quantity (Percentage (4913 % 5000)))

  To rerun use: --match "/CLI Specifications/SHELLEY_CLI_WALLETS/WALLETS_GET_01 - Can get a wallet/"

Randomized with seed 1319616375

#2855

@sevanspowell
Copy link
Contributor Author

bors retry

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 17, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit 9110cbb into master Aug 17, 2021
@iohk-bors iohk-bors bot deleted the sevanspowell/adp-1061/expose-max-collateral-inputs branch August 17, 2021 05:01
WilliamKingNoel-Bot pushed a commit that referenced this pull request Aug 17, 2021
@sevanspowell
Copy link
Contributor Author

Thanks bors 😅

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Aug 18, 2021

try

Merge conflict.

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.

2 participants