Allow flexible SMASH selection wrt ADP-427 #2214

hasufell · 2020-10-05T22:48:45Z

Replaces #2156

I think there are some weird issued with buildkite, because it's from my fork repo.

hasufell · 2020-10-05T22:49:19Z

bors try

iohk-bors · 2020-10-05T23:38:20Z

try

Build failed:

buildkite/cardano-wallet

rvl · 2020-10-06T03:24:52Z

Buildkite won't build branches from forks, as a matter of policy.

hasufell · 2020-10-06T10:14:51Z

bors try

iohk-bors · 2020-10-06T11:34:22Z

try

Build succeeded:

specifications/api/swagger.yaml

piotr-iohk · 2020-10-06T13:28:57Z

specifications/api/swagger.yaml

+      description: |
+        <p align="right">status: <strong>stable</strong></p>
+
+        Overwrite current settings


I think more detailed description would be handy explaining what are the possible values and behavior after updating the settings (like stated in #2163 in "Decision" section, if it is still up to date.)

Since settings should be as general as possible, I don't know how to describe the behavior. That depends on the setting. We can only reasonably document it for the pool_metadata_source. That is already done in the appropriate places:

x-poolMetadataSource: &poolMetadataSource description: | Pool metadata source. This sets the metadata fetching strategy. Possible values are * null -> no fetching * direct -> direct fetching * uri -> use SMASH server oneOf: - type: string enum: - none - direct - title: URI type: string format: uri example: https://smash.cardano-mainnet.iohk.io/ x-settings: &settings description: Settings type: object required: - pool_metadata_source properties: pool_metadata_source: <<: *poolMetadataSource description: How to fetch pool metadata

Ok, perhaps updating to something like:

properties: pool_metadata_source: <<: *poolMetadataSource description: | Select metadata fetching strategy: - `none` - metadata is not fetched at all, - `direct` - metadata is fetched directly from chain, - `uri` - metadata is fetched from the external Stake-Pool Metadata Aggregation Server (SMASH) After update existing metadata will be dropped forcing it to re-sync automatically with the new setting.

lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Wallets.hs

piotr-iohk · 2020-10-06T14:00:09Z

lib/cli/src/Cardano/CLI.hs

+    <> showDefaultWith showT
+    <> help "Pool Metadata fetching strategy. This setting will persist across restarts. Possible values: \
+            \ none, \
+            \ direct, \


What's the difference between "none" and "direct"? 😅

I suppose:

"none" - don't look for metadata at all

"direct" - look for metadata on chain

"uri" - look in the SMASH server

I imagine "none" would be picked rather seldom. (Only case I can think is some maintenance/testing purposes or maybe exchanges running wallet and don't care about metadata)

Perhaps better default value would be "direct"? Which is also a current default...

none was decided as the default in the ticket (ADP-427):

About (1), the command-line option should be removed altogether and the wallet should instead wait for a strategy to be provided. Without strategy, no metadata will be fetched whatsover.

As per: https://input-output-rnd.slack.com/archives/GBT05825V/p1602080513063300

If nothing is provided we should default to the latest setting in use. If there was no previous setting in use, then it should default to "none".

Currently we always have "none" which erases any settings that were set in runtime.

KtorZ

Please make sure to address the few remaining comments before triggering bors 👍

lib/core/src/Cardano/Pool/DB.hs

lib/core/src/Cardano/Pool/DB/Model.hs

lib/core/src/Cardano/Wallet/Primitive/Types.hs

hasufell · 2020-10-07T13:47:18Z

bors r+

2214: Allow flexible SMASH selection wrt ADP-427 r=hasufell a=hasufell Replaces #2156 I think there are some weird issued with buildkite, because it's from my fork repo. Co-authored-by: Julian Ospald <[email protected]>

disassembler · 2020-10-07T14:54:24Z

bors r-

iohk-bors · 2020-10-07T14:54:26Z

Canceled.

disassembler · 2020-10-07T14:54:32Z

bors r+

2214: Allow flexible SMASH selection wrt ADP-427 r=disassembler a=hasufell Replaces #2156 I think there are some weird issued with buildkite, because it's from my fork repo. Co-authored-by: Julian Ospald <[email protected]>

KtorZ · 2020-10-07T15:00:25Z

bors r-

iohk-bors · 2020-10-07T15:00:27Z

Canceled.

KtorZ · 2020-10-07T15:01:14Z

lib/cli/src/Cardano/CLI.hs

-            \Stakepool Metadata Aggregation Server."
+    <> long "pool-metadata-fetching"
+    <> metavar "STRATEGY"
+    <> value FetchNone


@piotr-iohk @hasufell

I suspect the issue is here? There shouldn't be any default value here. Do you concur?

hasufell · 2020-10-07T15:12:56Z

bors r+

2214: Allow flexible SMASH selection wrt ADP-427 r=hasufell a=hasufell Replaces #2156 I think there are some weird issued with buildkite, because it's from my fork repo. Co-authored-by: Julian Ospald <[email protected]>

iohk-bors · 2020-10-07T17:20:36Z

Build failed:

ci/hydra-build:required

  src/Test/Integration/Scenario/API/Shelley/StakePools.hs:530:5: 
  1) API Specifications, SHELLEY_STAKE_POOLS, STAKE_POOLS_JOIN_05 - Can join when stake key already exists
       uncaught exception: RequestException
       DecodeFailure "{\"code\":\"network_unreachable\",\"message\":\"The node backend is unreachable at the moment. Trying again in a bit might work.\"}"

  To rerun use: --match "/API Specifications/SHELLEY_STAKE_POOLS/STAKE_POOLS_JOIN_05 - Can join when stake key already exists/"

#2230

hasufell · 2020-10-07T18:37:13Z

bors r+

2214: Allow flexible SMASH selection wrt ADP-427 r=hasufell a=hasufell Replaces #2156 I think there are some weird issued with buildkite, because it's from my fork repo. Co-authored-by: Julian Ospald <[email protected]>

iohk-bors · 2020-10-07T20:20:26Z

Build failed:

buildkite/cardano-wallet

src/Test/Integration/Scenario/API/Shelley/StakePools.hs:659:9:
--
  | 1) API Specifications, SHELLEY_STAKE_POOLS, STAKE_POOLS_LIST_01 - List stake pools, has non-zero saturation & stake
  | uncaught exception: IOException of type UserError
  | user error (Waited longer than 90s (more than 2 epochs) for an action to resolve. Action: "list pools returns non-empty list". Error condition: Just user error (Quantity {getQuantity = Percentage {getPercentage = 0 % 1}} does not satisfy (> Quantity {getQuantity = Percentage {getPercentage = 0 % 1}})))

Only one stake pool test failed:

STAKE_POOLS_LIST_01 - List stake pools
--
  | has non-zero saturation & stake FAILED [1]
  | pools have the correct retirement information
  | eventually has correct margin, cost and pledge
  | at least one pool eventually produces block
  | contains pool metadata

So this seems different from #2196, where the worker had crashed and no pools at all were shown.

#2224

hasufell · 2020-10-07T20:22:52Z

bors r+

iohk-bors · 2020-10-07T21:15:02Z

Build succeeded:

hasufell · 2020-10-07T21:19:04Z

Build succeeded:

@KtorZ

2235: Fix extra-source-files not containing files used by TH r=hasufell a=hasufell 2239: WIP: Bump version from 2020.9.30 to 2020.10.13 r=KtorZ a=jonathanknowles  Compatible with [`[email protected]`](https://github.com/input-output-hk/jormungandr/releases/tag/v0.9.0) and [`[email protected]`](https://github.com/input-output-hk/cardano-node/releases/tag/1.21.1). ## New Features - Adds the ability for users to select their own SMASH servers for stakepool listings. (#2214) - Adds transaction expiry slots for pending transactions. (#1879) ## Improvements - Adds a 100-wallet scenario to the latency benchmark. (#2223) - Adds an executable `shelley-test-cluster` which starts an integration test cluster that includes faucets. (#2178) - Extends `isOurs` to return the derivation path of an address. (#2219) ## Resolved Issues - Make pool garbage collection handle an unknown current epoch. (#2203) - Fixes incorrect mainnet network parameters returned from API. (#2226) ## Known Issues **_This section is a work in progress._** - Wallet restoration status reported incorrectly on mainnet. ([ADP-483](https://jira.iohk.io/browse/ADP-483)) ## Documentation  Cardano (cardano-node) | ITN (Jörmungandr) --- | --- [API Documentation](https://input-output-hk.github.io/cardano-wallet/api/v2020-10-13) | [API Documentation](https://input-output-hk.github.io/cardano-wallet/api/v2020-10-13) [CLI Manual](https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-command-line-interface/f71ecb7ece3deaec5bf60f4a4beea1d3950e0d46) | [CLI Manual](https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-command-line-interface-jormungandr/f71ecb7ece3deaec5bf60f4a4beea1d3950e0d46) [Docker Manual](https://github.com/input-output-hk/cardano-wallet/wiki/Docker/f71ecb7ece3deaec5bf60f4a4beea1d3950e0d46) | [Docker Manual](https://github.com/input-output-hk/cardano-wallet/wiki/Docker-jormungandr/f71ecb7ece3deaec5bf60f4a4beea1d3950e0d46) ## Installation Instructions ### Cardano (cardano-node) 1. Install [`[email protected]`](https://github.com/input-output-hk/cardano-node/releases/tag/1.21.1). 2. Download the provided `cardano-wallet` for your platform, and uncompress it in a directory that is on your `$PATH`, e.g. `/usr/local/bin`. Or `%PATH%` on Windows. 4. Start `cardano-wallet --help` and see available parameters. #### Docker Pull from DockerHub and verify the version matches 2020.10.13. ``` $ docker pull inputoutput/cardano-wallet:2020.10.13 $ docker run --rm inputoutput/cardano-wallet:2020.10.13 version ``` ### ITN (jormungandr) 1. Install [`[email protected]`](https://github.com/input-output-hk/jormungandr/releases/tag/v0.9.0). 2. Download the provided `cardano-wallet-jormungandr` for your platform, and uncompress it in a directory that is on your `$PATH`, e.g. `/usr/local/bin`. Or `%PATH%` on Windows. 3. (optional) Install the bash/zsh auto-completion script according to the [jormungandr cli manual](https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-Command-Line-Interface/{{JORM_CLI_WIKI_COMMIT}}) 4. Start `cardano-wallet --help` and see available parameters. #### Docker Pull from DockerHub and verify the version matches 2020.10.13 ``` $ docker pull inputoutput/cardano-wallet:2020.10.13-jormungandr $ docker run --rm inputoutput/cardano-wallet:2020.10.13-jormungandr version ``` ## Signatures  Name | Role | Approval --- | --- | ---: Matthias Benkort @KtorZ | Technical Team Lead | ⌛ Piotr Stachyra @piotr-iohk | QA Engineer | ⌛ Tatyana Valkevych @tatyanavych | Release Manager | ⌛ Co-authored-by: Julian Ospald <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: IOHK <[email protected]>

hasufell requested a review from KtorZ October 5, 2020 22:49

iohk-bors bot added a commit that referenced this pull request Oct 5, 2020

Try #2214:

8596cbb

hasufell force-pushed the hasufell/ADP-427/flexible-smash branch from 076f525 to d969dfa Compare October 6, 2020 08:55

iohk-bors bot added a commit that referenced this pull request Oct 6, 2020

Try #2214:

40f45b4

hasufell requested review from jonathanknowles, rvl and Anviking October 6, 2020 12:03

piotr-iohk reviewed Oct 6, 2020

View reviewed changes

specifications/api/swagger.yaml Show resolved Hide resolved

hasufell force-pushed the hasufell/ADP-427/flexible-smash branch from d969dfa to 4bf109c Compare October 6, 2020 13:20

piotr-iohk reviewed Oct 6, 2020

View reviewed changes

KtorZ removed request for jonathanknowles, Anviking and rvl October 6, 2020 15:00

KtorZ approved these changes Oct 6, 2020

View reviewed changes

lib/core/src/Cardano/Pool/DB.hs Show resolved Hide resolved

lib/core/src/Cardano/Pool/DB/Model.hs Show resolved Hide resolved

lib/core/src/Cardano/Wallet/Primitive/Types.hs Outdated Show resolved Hide resolved

hasufell force-pushed the hasufell/ADP-427/flexible-smash branch from 4bf109c to 50b9f9e Compare October 7, 2020 13:34

KtorZ reviewed Oct 7, 2020

View reviewed changes

Allow flexible SMASH selection wrt ADP-427

be92684

hasufell force-pushed the hasufell/ADP-427/flexible-smash branch from 50b9f9e to be92684 Compare October 7, 2020 15:11

iohk-bors bot merged commit 1e3f4b8 into master Oct 7, 2020

iohk-bors bot deleted the hasufell/ADP-427/flexible-smash branch October 7, 2020 21:15

hasufell linked an issue Oct 8, 2020 that may be closed by this pull request

Allow flexible SMASH selection without restarting the wallet #2163

Closed

This was referenced Oct 8, 2020

Flaky STAKE_POOLS_LIST_01 - List stake pools, has non-zero saturation & stake #2224

Closed

Flaky STAKE_POOLS_JOIN_05 - Can join when stake key already exists #2230

Closed

[Duplicate] Flaky STAKE_POOLS_JOIN_05 - Can join when stake key already exists #2231

Closed

jonathanknowles mentioned this pull request Oct 13, 2020

Bump version from 2020.9.30 to 2020.10.13 #2239

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow flexible SMASH selection wrt ADP-427 #2214

Allow flexible SMASH selection wrt ADP-427 #2214

hasufell commented Oct 5, 2020

hasufell commented Oct 5, 2020

iohk-bors bot commented Oct 5, 2020

rvl commented Oct 6, 2020

hasufell commented Oct 6, 2020

iohk-bors bot commented Oct 6, 2020

piotr-iohk Oct 6, 2020

hasufell Oct 7, 2020

piotr-iohk Oct 7, 2020

piotr-iohk Oct 6, 2020

piotr-iohk Oct 6, 2020

hasufell Oct 7, 2020

piotr-iohk Oct 7, 2020

KtorZ left a comment

hasufell commented Oct 7, 2020

disassembler commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020

disassembler commented Oct 7, 2020

KtorZ commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020

KtorZ Oct 7, 2020

hasufell Oct 7, 2020

piotr-iohk Oct 7, 2020

hasufell commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020 •

edited by Anviking

Loading

hasufell commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020 •

edited by Anviking

Loading

hasufell commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020

hasufell commented Oct 7, 2020

Allow flexible SMASH selection wrt ADP-427 #2214

Allow flexible SMASH selection wrt ADP-427 #2214

Conversation

hasufell commented Oct 5, 2020

hasufell commented Oct 5, 2020

iohk-bors bot commented Oct 5, 2020

try

rvl commented Oct 6, 2020

hasufell commented Oct 6, 2020

iohk-bors bot commented Oct 6, 2020

try

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

KtorZ left a comment

Choose a reason for hiding this comment

hasufell commented Oct 7, 2020

disassembler commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020

disassembler commented Oct 7, 2020

KtorZ commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hasufell commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020 • edited by Anviking Loading

hasufell commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020 • edited by Anviking Loading

hasufell commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020

hasufell commented Oct 7, 2020

iohk-bors bot commented Oct 7, 2020 •

edited by Anviking

Loading

iohk-bors bot commented Oct 7, 2020 •

edited by Anviking

Loading